-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Editor: Update post excerpt panel with new designs #60894
Editor: Update post excerpt panel with new designs #60894
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
import PluginPostExcerpt from '../post-excerpt/plugin'; | ||
import { store as editorStore } from '../../store'; | ||
|
||
export function useEditExcerptAction() { |
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.
Finding the best path forward for consolidating the actions is also discussed here, but for now I added this private action and kind of 'separated' it from the other actions here..
Size Change: +1.14 kB (0%) Total Size: 1.75 MB
ℹ️ View Unchanged
|
Nice, I'm excited to see descriptions coming to templates / patterns :) Editing in a modal isn't the perfect experience, but probably okay for now given the various changes here, and considering we may want to invoke the experience from data views. A couple of small suggestions:
I'm unsure how to reproduce, but during testing in the site editor the excerpt stopped appearing in the Inspector: excerpt.mp4 |
I've tried to preserve the editing of excerpt as it was, and that was updating the excerpt on change. If want to change this behavior we would need to create a new control and essentially preserve the old edit excerpt control for back compat. I'm not against it, just explaining my rationale for the initial decision. Also noting that we have similar dialogs, like the
Any suggestions are welcome about the copy and based on the different cases, if it makes a difference(template, pattern, etc..). Also is related to the above.
I think we should consider this separately, although my first thoughts would be that this should be part of the auto generated fields API to come.
I don't think we should do that for a11y reasons. We need to preserve the focus on close of the modal. I'll check the rest now in code. |
Wouldn't it make sense that clicking on the excerpt text would open up the ability to edit the text via the modal (or even inline?) rather than being hidden behind a 3 dot icon. I can guarantee this will be a huge downgrade in UX for the majority of users as the current way to edit the excerpt is to just type into a textarea, whereas this version has hidden behind 3 dots which doesn't mean anything to most people. |
e4e9ad3
to
f1275fd
Compare
Flaky tests detected in 069c8a4. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/8829005913
|
So we could try having the excerpt/description clickable and open a dialog, right? This would mean that we wouldn't add the |
f1275fd
to
a266fe6
Compare
Yes, that's right.
Certainly! How we treat this will depend on what trigger options there are for popovers. Does it have to be a Button? |
Another option could be a 'collapsible' textarea of sorts, where the label, help text, and input chrome are only visible on focus: excerpt.mp4This would keep the footprint small while retaining the semantic benefits of the current control. I suppose this also falls into custom control territory though? |
a266fe6
to
3a07113
Compare
I pushed some adjustments. The spacing around the button in the post editor is not ideal, but I can't see a way to fix that without also affecting the template card. Ideally the Summary panel is refactored to use flex layout (and not use The final thing to try here is only updating the button label when the dialog is closed. It's a bit distracting to see the button changing as you type in the textarea. |
Thanks for polishing Jay!
I pushed an update for this one. I'll look at the e2e tests and update the PR's description tomorrow though.. |
Thanks, I think that works better 👍 |
I found a small bug, while loading the editor, for a fraction of seconds you can see the "Add excerpt" placeholder next to the post status "draft" but when the feature image loads it's positioned properly. Maybe it's missing a wrapper div or something. |
@@ -64,6 +79,7 @@ export default function PostStatus() { | |||
<> | |||
<PostStatusPanel /> | |||
<PostFeaturedImagePanel withPanelBody={ false } /> | |||
{ showPostExcerptPanel && <PrivatePostExcerptPanel /> } |
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 believe this check should probably move within the component itself like the other PostSomething components.
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.
// Post excerpt panel is rendered in different place depending on the post type.
// So we cannot make this check inside the PostExcerpt component based on the current edited entity.
Do you think there is a better way by adding some kind of flag or something?
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 is the post excerpt rendered in different places depending on the post type?
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.
Based on the designs: #59689 (comment). I think the main reason is that in the entities that we treat excerpt as a description
feels better above in the card where we essentially describe the entity. In regular post types excerpt is more of part of the entity with a different functionality - describe the content.
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.
Are we really helping ourselves by creating these inconsistencies?
{ showPostExcerptPanel && <PrivatePostExcerptPanel /> } | ||
{ showPostContentInfo && <PostContentInfo /> } | ||
{ lastEditedText && <Text>{ lastEditedText }</Text> } | ||
</VStack> |
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.
Where is this rendered so I can test it? If I'm not wrong it's the top of the inspector no? I didn't see the excerpt there in my testing, so I'm missing something.
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.
It's in the top of the inspector yes, but for patterns, templates and template parts.
Then it will be shown if there is a description/excerpt and if it's a user create entity it will be editable too..
Fixed. |
Code wise, this is looking good, I have more issues with the design personally and specially for the ones that are not treated as descriptions. For these post types, I think adding a label on the left and trimmed excerpt on the right would bring more harmony and clarity and feel less like a regression compared to trunk. |
Even for the ones that treat "excerpt as descriptions", I understand the reasoning for wanting these to show in the card, but I think making them editable in the card by clicking is not a great UX, it's not really discoverable. My opinion is that we should also show the excerpt in the "summary" (with a label) just like all the rest of the post types and leave the one in the card as readonly. |
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.
While I have reservations about the designs. My reservations are now known and we can iterate on them separately if needed.
I totally gree and it's not even accessible. On top of that, the initial label 'Add an excerpt… / description…' looks like plain text. It;s just gray text on the page. It can't be visually identified as an interactive control, which is a no-go for accessibility. Same applies when there is en excerpt: it looks like plain text. Looking at latest proposal on #59689 (comment) that would not be ideal as well because color alone (blue) is not sufficient to identify an interactive control. This will need a follow-up based on new design. |
@afercia So that I fully understand this feedback, does that make any tertiary button instance inaccessible? Would that also rule out a dedicated "Edit excerpt" button as a solution? Edit: Perhaps we can use the |
@jameskoster yes all tertiary buttons are inherently not accessible. I failed with creating a specific issue about that because I'm uncomfortable with starting a dialectical fight with the design team. |
since this change has been made, how is it possible now to change programmaticaly with js the excerpt ? |
@frougeot2 this change was mostly a design update. The |
yes you're right. My problem is indeed elsewhere, I first try to retrieve the extract with the following code: So my problem is on the document.querySelector('.editor-post-excerpt textarea'); line that returns null now ... |
Noting that class names are not part of the public API. So this is something that could happen in the future too. I'm not sure about your exact use case, but since you've pinpoint it, you could find a workaround for the old selector. |
What?
Part of: #59689
This PR does a few things related to the move of post excerpt panel based on the new designs, as shared here and here.
excerpt
as adescription
and this PR handles the proper labelling per post type (ex.Edit excerpt
vsEdit description
). Additionally handles the proper update of this field, as there are nuances in templates and template parts that use theexcerpt
asdescription
in REST API. Finally in these post types I've added a check to not abide by the preference of showing theexcerpt
panel.PrivatePostExcerptPanel
) because the existingPostExcerptPanel
is public API and renders the control to edit the excerpt.Testing Instructions
excerpt
for all post typesScreenshots or screencast
Screen.Recording.2024-04-25.at.11.19.32.AM.mov