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.

Sign up for GitHub

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

Jump to bottom

Post Excerpt: improve preview on the editor site #50167

Merged
merged 3 commits into from
Jun 6, 2023
Merged

Post Excerpt: improve preview on the editor site #50167

merged 3 commits into from
Jun 6, 2023

Conversation

gigitux
Copy link
Contributor

@gigitux gigitux commented Apr 28, 2023 •
edited
Loading

What?

This PR improves the preview of the block on the editor.

Fixes #48964

Why?

Currently, in the editor, the preview of the Post Excerpt is hard-coded, and it is:

This is the Post Excerpt block, it will display the excerpt from single posts.

If there are any Custom Post Types with support for excerpts, the Post Excerpt block can display the excerpts of those entries as well.

Making the block title and labels more generic would improve the user experience:

This block will display the excerpt.

There is a similar issue for the Post Title block #48963. Also, this PR could be part of this macro issue #49108.

How?

The PR renames the block to Excerpt and updates the labels to make it more generic.

Testing Instructions

  1. Edit a template (Single Post, for example).
  2. Add the Excerpt block.
  3. Ensure the preview is generic and can work for other postType templates.

Screenshots or screencast

image
@gigitux gigitux requested a review from ajitbohra as a code owner April 28, 2023 12:22
@gigitux gigitux mentioned this pull request Apr 28, 2023
Closed
@mtias mtias added the [Block] Post Excerpt Affects the Post Excerpt Block label Apr 28, 2023
@gziolo gziolo added [Type] Enhancement A suggestion for improvement. [Package] Block library /packages/block-library Good First Review A PR that's suitable for someone looking to contribute for the first time by reviewing code labels Apr 28, 2023
@gigitux gigitux changed the base branch from trunk to 1_76_with_previous_aztec May 30, 2023 15:48
@gigitux gigitux changed the base branch from 1_76_with_previous_aztec to trunk May 30, 2023 15:48
@swissspidy
Copy link
Member

Assembling the string like this will not work for many languages other than English, so this should be avoided. Please consult the polyglots team on this!

I highly recommend rephrasing the sentences to avoid this issues, or add these to get_post_type_labels() in core.
@carolinan
Copy link
Contributor

I don't believe this is the correct solution, because the text must be fully translatable.
@SergeyBiryukov when convenient could you help review the code and remind us about the difficulties with translating the post type?
I will leave this link for reference: https://core.trac.wordpress.org/ticket/52047

I think a better solution that we could also use for other blocks is to use a more generic description without the post type.
Simply "Excerpt" instead of "Page excerpt" "Post excerpt".
@gigitux
Copy link
Contributor Author

gigitux commented May 31, 2023

I think a better solution that we could also use for other blocks is to use a more generic description without the post type. Simply "Excerpt" instead of "Page excerpt" "Post excerpt".

Good point! Thinking about it, this would be the easiest way to fix the issue. We could do the same for #48963
@nerrad
Copy link
Contributor

nerrad commented May 31, 2023

I think a better solution that we could also use for other blocks is to use a more generic description without the post type.
Simply "Excerpt" instead of "Page excerpt" "Post excerpt".

Agree! Generic does seem the best route here. "Title" vs "Post Title", "Excerpt" vs "Post Excerpt" etc.
@ntsekouras
Copy link
Contributor

I think a better solution that we could also use for other blocks is to use a more generic description without the post type.
Simply "Excerpt" instead of "Page excerpt" "Post excerpt".

This seems like a good solution probably. Any thoughts on this @mtias, @jasmussen @jameskoster ?
@carolinan
Copy link
Contributor

Would "Excerpt" work for WooCommerce @nerrad?
@jasmussen
Copy link
Contributor

Agree, simple and generic works well here.
@gigitux gigitux force-pushed the add/48964-display-post-type-name-as-preview branch from d79973d to f3a3037 Compare June 1, 2023 09:09
@gigitux
Copy link
Contributor Author

gigitux commented Jun 1, 2023

Thanks for your precious feedback! I make the labels more generic as well with f3a30376df53a55dd5bdfa7558ebd254e1ae049c. What do you think?
@gigitux gigitux mentioned this pull request Jun 1, 2023
Closed
@gigitux
Copy link
Contributor Author

gigitux commented Jun 1, 2023

Would "Excerpt" work for WooCommerce @nerrad?

Even if it wouldn't be the best fit, we can update the name and description of the block with a variation (as we are already doing). The main issue was the labels in the edit file that we can't control.
@ntsekouras ntsekouras mentioned this pull request Jun 1, 2023
Merged
@nerrad
Copy link
Contributor

nerrad commented Jun 1, 2023

Would "Excerpt" work for WooCommerce @nerrad?

Even if it wouldn't be the best fit, we can update the name and description of the block with a variation (as we are already doing). The main issue was the labels in the edit file that we can't control.

This. I think conceptually it also works well across the breadth of different post types that might be used on a given WordPress site (or WooCommerce store). The presence of "Post" nomenclature for all these post types was the most pressing issue here.
mtias
mtias reviewed Jun 1, 2023
View reviewed changes
packages/block-library/src/post-excerpt/block.json Outdated
"category": "theme",
"description": "Display a post's excerpt.",
"description": "Display an excerpt.",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be "the" excerpt?
@mtias
Copy link
Member

mtias commented Jun 1, 2023

There's an inconsistent use of "the excerpt" and "an excerpt" that would be good to address
@gigitux gigitux force-pushed the add/48964-display-post-type-name-as-preview branch from e0fe493 to 80d6a4d Compare June 1, 2023 12:21
@gigitux
Copy link
Contributor Author

gigitux commented Jun 1, 2023 •
edited
Loading

There's an inconsistent use of "the excerpt" and "an excerpt" that would be good to address

Thanks for the review! Fixed with 80d6a4d and d288bb5.
@gigitux gigitux requested a review from mtias June 2, 2023 08:04
nerrad
nerrad approved these changes Jun 6, 2023
View reviewed changes
Copy link
Contributor

@nerrad nerrad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good. I also did a double-check through the codebase to see if there are any other instances of the string that could be updated and didn't find any so looks like you got them all.
@nerrad nerrad merged commit e8e2ae3 into WordPress:trunk Jun 6, 2023
48 checks passed
@github-actions github-actions bot added this to the Gutenberg 16.0 milestone Jun 6, 2023
sethrubenstein pushed a commit to pewresearch/gutenberg that referenced this pull request Jul 13, 2023
@gigitux @sethrubenstein
Post Excerpt: improve preview on the editor site (WordPress#50167)
336ce7e 
* rename post-excerpt to excerpt and update labels

* improve description

* improve warning label
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Post Excerpt Affects the Post Excerpt Block Good First Review A PR that's suitable for someone looking to contribute for the first time by reviewing code [Package] Block library /packages/block-library [Type] Enhancement A suggestion for improvement.
8 participants
@gigitux @swissspidy @carolinan @nerrad @ntsekouras @jasmussen @mtias @gziolo