Post excerpt: Fix JavaScript error and other misc. bug fixes #48730
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
carolinan
commented
Mar 3, 2023
carolinan
commented
Mar 3, 2023
|
Size Change: +89 B (0%) Total Size: 1.34 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 213aa20. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4324210386
|
carolinan
changed the title
carolinan
changed the title
carolinan
added
the
[Priority] High
t-hamano
reviewed
Mar 22, 2023
There was a problem hiding this comment.
Thanks for the PR, @carolinan!
As far as I can see, this PR is trying to solve 4 problems at the same time:
- Use
…instead of... - The ellipsis shows even if the text is not truncated
- Remove undo step when changing the value in the excerpt length setting
- The excerpt is editable but not saved, when the post type doesn't support excerpt
I would recommend splitting it into at least two PRs to target changes. How about dealing with No.1, No.2, and No.3 in this PR and focusing on the ellipsis issue? No.3 has nothing to do with ellipsis but could be considered an improvement in code quality.
Also, could you rebase the changes I made in #49123 as they are causing conflicts?
Co-authored-by: Aki Hamano <54422211+t-hamano@users.noreply.github.com>
Mamaduka
reviewed
Mar 22, 2023
Comment on lines
42
to
47
| // Check if the post type supports excerpts. | ||
| let postTypeSupportsExcerpts = useSelect( ( select ) => | ||
| postType | ||
| ? !! select( coreStore ).getPostType( postType )?.supports.excerpt | ||
| : false | ||
| ); |
There was a problem hiding this comment.
A few notes:
- The
useSelectneeds apostTypedependency. - We can return
trueearly whenpostType === pagefrom the selector. - There's no need for ternary. Casting the return value to
booleanwill do the trick.
The end results will be something like this:
// Check if the post type supports excerpts.
const postTypeSupportsExcerpts = useSelect(
( select ) => {
if ( postType === 'page' ) {
return true;
}
return !! select( coreStore ).getPostType( postType )?.supports
.excerpt;
},
[ postType ]
);
Mamaduka
reviewed
Mar 22, 2023
| const rawOrRenderedExcerpt = !! renderedExcerpt | ||
| ? strippedRenderedExcerpt | ||
| : rawExcerpt; | ||
| let rawOrRenderedExcerpt = rawExcerpt || strippedRenderedExcerpt; |
There was a problem hiding this comment.
Which excerpt variation is preferred here when we have both?
There was a problem hiding this comment.
Hmm yes In this case the rawExcerpt should be prioritized because it is the customized excerpt text.
There was a problem hiding this comment.
I asked because the previous logic prioritized renderedExcerpt if it was available, and here we're changing the logic.
Suggestion (non-blocking): You can also use parentheses around expression and trim returned value in a single go:
const rawOrRenderedExcerpt = ( rawExcerpt || strippedRenderedExcerpt ).trim();There was a problem hiding this comment.
I think the previous logic was wrong and this would correct it.
Looking at the code before #44964, the rawExcerpt comes first.
There was a problem hiding this comment.
Thanks for the confirmation, @carolinan!
|
@carolinan |
|
@carolinan, the Post Excerpt block displays "No post excerpt found" in the editor for a post type that doesn't support excerpts. I can see the excerpt rendered when previewing the CPT. Is this expected behavior? Screenshot
|
|
In my opinion, it should not even allow adding the excerpt block... but it is too late for it since it has been available since the beginning. |
|
What if the text in the "is not editable" condition was changed to "No excerpt preview available"? This text, on line 213: |
Mamaduka
approved these changes
Mar 24, 2023
|
Agreed. This inconsistency isn’t great. |
|
@carolinan The fixes here look good to me to be merged ✅ |
aristath
approved these changes
Mar 24, 2023
14 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Attempts to address feedback about the post excerpt block:
…instead of...Removes one instance of
setExcerpt. The excerpt does not need to be updated when the length is adjusted. This was a left over from a previous implementation.This fix assures that there is only one undo step needed when changing the value in the excerpt length setting.
Solves a bug where the excerpt was editable but not saved, when the block is used with a custom post type that does not support excerpts.
Solves a JavaScript error in the editor when the block is used with a custom post type that does not support excerpts.
Introduced in #44964:
Why?
How?
Testing Instructions
Create a new post.
Add a post excerpt block.
Add less than 10 words to the excerpt.
Set the excerpt length setting in the block settings sidebar to 10.
Deselect the block: no
…should show after the text (this is the fix).Select the block add text so that there are more than 10 words.
Deselect the block. the
…symbol should show.Create a new page.
Add more than ten words to the page content.
Add a post excerpt block.
Select the post excerpt block and confirm that the text can be edited.
Set the excerpt length setting in the block settings sidebar to 10.
Confirm that the length is applied to the excerpt.
Register a custom post type that does not have support for excerpts.
See https://developer.wordpress.org/reference/functions/register_post_type/ for sample code
get_the_excerptfunction when it is used with a custom post type without excerpt support, without the block.