-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Post excerpt: Add excerpt length control #44964
Conversation
Size Change: +209 B (0%) Total Size: 1.31 MB
ℹ️ View Unchanged
|
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Tested as of commit 86342e2 with ernesto theme and this function commented out Create a new post, add a long paragraph. Works as expected. |
This comment was marked as outdated.
This comment was marked as outdated.
Is there a way we could indicate what's truncated while selected? Like, would wrapping what's truncated in the markup be helpful for a11y? If so we could make the wrapped (i.e. truncated) text slightly transparent (maybe 90%) for a visual, and perhaps have assistive text indicating what is truncated, or where the divide exists. Though I'm not 100% that this would work. @alexstine? |
@richtabor That would be nice but it is likely out of scope for this PR. The whole issue in general is well out of scope of the PR but this is one of Gutenbergs many issues with not communicating contextual info to screen readers. You either have to know exactly that selecting another block, going in to virtual mode, and using down arrow will read the truncated text or we as devs can figure out how to make sure screen reader users do not have less of an experience. Either way, both of these probably best belong in a follow-up. Do not want to hijack this PR with a ton of extra work. |
I'm reading the comments in this issue and I have a question... Why is this issue blocked? There are small things that can be improved in follow-up PRs, sure... That's always the case with all PRs. But what's actually blocking us from adding the excerpt-length control here? 🤔 |
@aristath Nothing from my side. I was simply making a point to say that this should be followed-up on as the experience is less than great for screen reader users. Thanks. |
Agreed! I just don't want the ideas to fall into the ether :) I think this solution is fine, though follow-ups for improved screen reader (and visual) indicators for what's truncated would be good — so you don't have to de-select the block, to understand where the split happens. |
I have an idea, what if the block had a preview option like the HTML block. With a preview button in the toolbar. |
@carolinan Good idea. We'll still need to figure out how to make the preview mode accessible at some point, but implementing a base model would be a good start. Once preview is used in multiple places, it should be higher on priority list to improve. It is not unusable currently but it is not very discoverable for users due to some odd issues with tabindex on the block wrapper. |
I’d rather merge and circle back with something like the previous idea, where you wouldn’t have to do anything to understand what’s truncated. |
Then, let's click the button 😄 |
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 haven't reviewed edit.js
much because I'm mainly worried about the PHP issues, which have introduced some bugs here and have correctly triggered test failures in large multisites like WP.com. I think that is the top priority.
That said, the block experience inside the editor also needs to be reviewed more closely. For instance, just from a cursory read and a quick test:
- Two Undo levels are introduced due to the
setAttributes( … ); setExcerpt( … );
sequence of calls. See__unstableMarkAutomaticChange
for a likely fix. - As I select in and out of the block, the block will seemingly fail at times, returning "No post excerpt found".
- Ellipsis are appended even when there is no trimming needed.
- The interaction between the
excerptLength
attribute and the fact that a user can edit the excerpt directly is strange.
if ( is_admin() || | ||
defined( 'REST_REQUEST' ) || | ||
'REST_REQUEST' ) { |
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.
This is very wide net to cast which will affect all instances of excerpt filtering, regardless of source (Post Excerpt block, Latest Post block, traditional theme templates, etc.) — see #48403.
Why not surround the rendering logic in render_block_core_post_excerpt
with a pair of add_filter
/ remove_filter
calls?
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.
What I meant by my suggestion was something like this (untested):
function render_block_core_post_excerpt( $attributes, $content, $block ) {
global $_block_core_post_excerpt_length;
// ...
$_block_core_post_excerpt_length = int( $attributes['excerptLength'] );
add_filter( 'excerpt_length', '_block_core_post_excerpt_filter', PHP_INT_MAX );
get_the_excerpt();
remove_filter( 'excerpt_length', '_block_core_post_excerpt_filter' );
// ...
with
function _block_core_post_excerpt_filter() {
global $_block_core_post_excerpt_length;
return $_block_core_post_excerpt_length;
}
The whole global
dance is a possible way to work with the fact that $attributes['excerptLength']
is dynamic.
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 is already a pull request open that tries to address 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.
There is already a pull request open that tries to address this.
Which?
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.
Not sure if it's the one @carolinan was referring to, but I opened #48598 when I discovered that my excerpts outside of the block editor were not obeying my own excerpt_length filter and traced it back to the line you referenced here:
This is very wide net to cast which will affect all instances of excerpt filtering, regardless of source (Post Excerpt block, Latest Post block, traditional theme templates, etc.) — see #48403.
My last revision in my PR did something like what you suggest here:
Why not surround the rendering logic in
render_block_core_post_excerpt
with a pair ofadd_filter
/remove_filter
calls?
But it only applies to the rendered block on the page; it is not reflected in the editor. The condition on REST_REQUEST
is the part that makes it work in the editor. I was also concerned that condition casts too wide a net and raised the question in my PR, but received no feedback on that point.
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.
Yes, that is the PR I was thinking of
$excerpt_length = $attributes['excerptLength']; | ||
if ( isset( $excerpt_length ) ) { | ||
$excerpt = wp_trim_words( get_the_excerpt(), $excerpt_length ); | ||
} else { | ||
$excerpt = get_the_excerpt(); |
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.
Why not inject a filter with the dynamic excerpt length and just call get_the_excerpt
, which is already filtered by wp_trim_excerpt
?
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 don't believe I tried that during the development, so there is no obvious reason for not trying that as a possible improvement in a separate PR.
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.
The addition of the call to "wp_trim_words" will remove any HTML that users have put into a custom excerpt by over-riding get_the_excerpt. Was this deliberate, or an unfortunate by-product?
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.
The addition of the call to "wp_trim_words" will remove any HTML that users have put into a custom excerpt by over-riding get_the_excerpt. Was this deliberate, or an unfortunate by-product?
I believe that was accidental, so thank you for the flag.
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.
Workaround to bypass the wp_trim_words
function:
function fzs_filter_metadata_registration( $metadata ) {
if ($metadata['name'] === 'core/post-excerpt') {
unset($metadata['attributes']['excerptLength']);
}
return $metadata;
};
add_filter( 'block_type_metadata', 'fzs_filter_metadata_registration' );
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.
Thank you - that's a really helpful workaround for now
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.
This was also reported in the forums and I've confirmed it was introduced in WordPress 6.3. Created an issue at #66195
trimmedExcerpt = rawOrRenderedExcerpt.trim().split( '', excerptLength ); | ||
} | ||
|
||
trimmedExcerpt = trimmedExcerpt + '...'; |
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'll ask the same thing Rich asked.
A very important piece of history is #33366, which removed the So the obligatory question is: have we worked around the issues that applied to #33366 in 2021? How? Or are they no longer applicable? |
The PR does not use the rendered content like the previous version, only the excerpt. |
I don't believe that is introduced in this PR. I keep seeing this on WP trunk without Gutenberg (pre 6.2) on different installs (different hosts so different setups). I can't reproduce it consistenly which is why I have not personally created an issue for it. I know this is not the technical answer you are looking for but as a user I experience it as if the excerpt is not being loaded fast enough.
It is, and that is why there has been so many iterations, and why I have been asking for the option to only be applied to the excerpt when it is not editable. |
What?
Provide a UI control to control the length of the excerpt.
Closes #34757
Currently there is no way to limit the length of the automatically generated excerpt without code.
It was requested in discussions in the issue that the feature should limit the length of both the user created and the automatically generated excerpts.
Description updated January 23.
Why?
To add the ability to control the excerpt length in a design.
How?
The PR adds a new block attribute to the post excerpt block called
excerptLength
. The default value is 55 words, to match the WordPress default length for excerpts.An excerpt length control is added to the block sidebar. The control uses a
RangeControl
component with the min value of 10 and max value of 100.The control limits the number of words that are displayed by the post excerpt block on the front.
It does not limit the number of words that are in the Excerpt panel in the post tab when editing a post.
The limitation is applied to the display of both auto generated and user-generated excerpts.
In index.php, the number of words is limited by passing
get_the_excerpt
and theexcerptLength
attribute through wp_trim_words.In the block editor, outside of a query loop:
In a query loop, the trimmed text shows.
Testing Instructions
Please test with both classic and block themes, and with a theme that filters the excerpt length.
Testing instructions for screen reader users
Create a new post.
Add a post excerpt block.
Enter a sample excerpt with more than ten words.
Press tab to move to the settings sidebar.
Move past the Posts tab.
In the Block tab, navigate forwards to the Settings panel.
Navigate past the Show link on new line option.
You should now find a new setting for the text length:
Using VoiceOver, I first hear the number of words, the label, and then the type of control:
"55, Max number of words, slider".
There are three ways to change the value:
Change the value to 10.
Navigate back to the editor.
Activate the "read all" command or similar for the screen reader.
Confirm that the excerpt only includes the first ten words.
Save and view the post on the front.
Confirm that the excerpt only includes the first ten words.