Skip to content

Tweaks following npm package updates for Beta 3 #5441

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

Closed
wants to merge 6 commits into from
Closed

Tweaks following npm package updates for Beta 3 #5441

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1f2451f
Remove @return void comments
mikachan Oct 9, 2023
ca48382
Refactor read more link translation
mikachan Oct 9, 2023
6e74cd9
Remove additional string concatenation
mikachan Oct 9, 2023
ad845b6
Update hidden accessibility text comment
mikachan Oct 9, 2023
fdfbc77
Add translator text for read more string
mikachan Oct 9, 2023
199ea1e
Remove Read more description
mikachan Oct 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions src/wp-includes/blocks/image.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,8 +309,6 @@ function block_core_image_render_lightbox( $block_content, $block ) {
* @since 6.4.0
*
* @global WP_Scripts $wp_scripts
*
* @return void
*/
function block_core_image_ensure_interactivity_dependency() {
global $wp_scripts;
Expand All @@ -326,8 +324,6 @@ function block_core_image_ensure_interactivity_dependency() {

/**
* Registers the `core/image` block on server.
*
* @return void
*/
function register_block_core_image() {
register_block_type_from_metadata(
Expand Down
5 changes: 2 additions & 3 deletions src/wp-includes/blocks/latest-posts.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: 1: A URL to a post, 2: The static string "Read more", 3: The post title only visible to screen readers. */
__( '… <a href="%1$s" rel="noopener noreferrer">%2$s<span class="screen-reader-text">: %3$s</span></a>' ),
/* translators: 1: A URL to a post, 2: Hidden accessibility text: Post title */
__( '… <a href="%1$s" rel="noopener noreferrer">Read more<span class="screen-reader-text">: %2$s</span></a>' ),
Copy link
Member

Choose a reason for hiding this comment

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

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)

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure why this wording has been chosen, so pinging @ramonjd @andrewserong to hopefully help here.

Copy link
Member

@ramonjd ramonjd Oct 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

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!

Copy link
Contributor

Choose a reason for hiding this comment

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

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 ),
__( 'Read more' ),
esc_html( $title )
);
}
Expand Down