Tweaks following npm package updates for Beta 3 #5441
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -152,10 +152,9 @@ function render_block_core_latest_posts( $attributes ) { | |
| if ( $excerpt_length <= $block_core_latest_posts_excerpt_length ) { | ||
| $trimmed_excerpt = substr( $trimmed_excerpt, 0, -11 ); | ||
| $trimmed_excerpt .= sprintf( | ||
| /* translators: The static string "Read more". %1$s: A URL to a post, %2$s: Hidden accessibility text: Post title */ | ||
| __( '… <a href="%1$s" rel="noopener noreferrer">Read more<span class="screen-reader-text">: %2$s</span></a>' ), | ||
|
There was a problem hiding this comment. Curious why we're using "Read more: {title}" here, as it reads a bit weird. Most existing core themes have been using something like "Continue reading {title}", which makes for a better reading flow. Also see #5439 (comment) There was a problem hiding this comment. I'm not sure why this wording has been chosen, so pinging @ramonjd @andrewserong to hopefully help here. There was a problem hiding this comment. Thanks for the ping. "Read more" has been a feature of this block for a long time and I didn't want to change things up too much. So the basic reason was backwards compatibility. That meant deciding how to structure the string so that it made sense grammatically. A semi-colon is fine. The other option could have been "Read more of", or about, but I discarded the superfluous preposition. I'm happy for the wording to change completely if folks think it best. As mentioned, my only intention was to avoid the situation where existing sites install 6.4, and are then left wondering why the "Read more" link text has changed. P.S.: I'll make a note to backport any changes made here back to Gutenberg. Thanks a lot for helping to tidy this up, everyone! There was a problem hiding this comment. Yeah the main reason the post title is added after the read more is so that when a screen reader provides a list of links on the page there is some context to each link and not just a bunch of "read more"s. In those circumstances something like "continue reading" won't make any more sense than "read more". In any case this is probably not something to be changed during Beta as it's not, strictly speaking, a bug 😄 |
||
| esc_url( $post_link ), | ||
| esc_html( $title ) | ||
| ); | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's some description in here that probably shouldn't be, I think these comments should only include the description of the placeholders.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed!
I recommend checking the handbook for best practices like that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added
The static string "Read more"here as the "Read more" string is now part of this single placeholder. Is it OK if this isn't described in the comments, or is there a different way to do this?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I see from the examples here that any translatable text doesn't need to be described specifically, only the variables. Makes sense, the suggestion above works 🎉
Although as per @kebbet's suggestion, should we keep the "Hidden accessibility text" comment?