Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New block: Post comments link #29564

Merged
merged 14 commits into from
Apr 19, 2021
Merged

Conversation

carolinan
Copy link
Contributor

@carolinan carolinan commented Mar 4, 2021

Description

Part of #22724
Adds a new post comments link block.

The block displays the number of comments, and links to the comments area on the single view.

-It tries to link to #comments even if the single view does not have a comment(s) block.

How has this been tested?

I have added the block inside the post content, and viewed the post on the front:
With and without comments, with and without comments enabled.

I have added the block to the index.html file of TT1 Blocks, to make sure that it inherits the context and displays correctly in a query loop.

Screenshots

TT1 Blocks, front:
image
The title is showing because the theme does not have a screen-reader-text class.

Types of changes

New block.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

@carolinan carolinan added New Block Suggestion for a new block [Feature] Full Site Editing labels Mar 4, 2021
@carolinan carolinan requested a review from aristath March 4, 2021 13:31
Add the generated fixtures for post-comments-link.
@carolinan carolinan marked this pull request as ready for review March 5, 2021 00:42
@youknowriad youknowriad added the Needs Design Feedback Needs general design feedback. label Mar 31, 2021
@youknowriad youknowriad added the [Type] Experimental Experimental feature or API. label Mar 31, 2021
@scruffian
Copy link
Contributor

This looking good. I think it would be good to have an option to customize the text - I might like it to say "Leave a comment" rather than show the number.

It might also be nice to add an option to display an icon next to the link.

@jasmussen
Copy link
Contributor

This is what I see:

Screenshot 2021-04-05 at 09 19 18

We need a new icon. But I think that's probably something that can wait, and not something that should block this PR. I'd also echo Ben's comment above about a custom phrasing, but also suggest that's something that can wait (I'd like the same customizability for the pagination blocks, for example).

One thing I noticed is that the block isn't inert:

focusable

I could swear there was precedence in some existing block for making links like that inert using a Disabled component, but I can't find it now, and I'm noticing also Latest Posts and others are interactive. So probably that should also not be a blocker.

So I guess this just needs a code review. But would be good to ticket followups. Thank you!

@carolinan
Copy link
Contributor Author

If the disabled component is used we won't be able to also have an editable text.

@jasmussen
Copy link
Contributor

If the disabled component is used we won't be able to also have an editable text.

Good point, though we could just put a textfield in the sidebar. While normally putting it directly in the canvas is the best strategy, I have a vague instinct that the sidebar might be where people intuit a rename to be. But this is not a strong opinion, and as noted, it could definitely be a followup.

@carolinan
Copy link
Contributor Author

The use of links inside the editors may need a more comprehensive review so that the behavior is the same in different blocks.
And if there already are best practices, they need to be documented.
The problem with having the link clickable here is that in the site editor, the linked page is opened inside the iframe.
It can also be confusing with visible links that are not clickable (but can be opened with right click), but it seems like the least bad option.

@jasmussen
Copy link
Contributor

Agreed. We've been talking about a "navigation cursor" you can select from the tools drop-down, which would navigate to link destinations inside the editor, when links are clicked. Sometimes referred to as "browse mode".

Set text color to false in block.json,
Updated comment count logic, update index.php
@carolinan carolinan requested a review from aristath April 14, 2021 10:35
@carolinan carolinan requested a review from aristath April 16, 2021 05:16
@carolinan carolinan merged commit d2ed38e into WordPress:trunk Apr 19, 2021
@github-actions github-actions bot added this to the Gutenberg 10.5 milestone Apr 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Design Feedback Needs general design feedback. New Block Suggestion for a new block [Type] Experimental Experimental feature or API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants