-
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
Add Featured Image, Title and Excerpt to top of the settings sidebar #42352
Add Featured Image, Title and Excerpt to top of the settings sidebar #42352
Conversation
Size Change: +6.05 kB (0%) Total Size: 1.26 MB
ℹ️ View Unchanged
|
- Moves the Featured Image panel to the top of the sidebar. - Uses a new dropdown-based flow for selecting a featured image. - Adds the ability to upload a featured image using the native file picker. - Adds a new Summary component to top of the sidebar containing fields for modifying the post's title and excerpt. - Removes the Excerpt panel in favour of excerpt field in Summary component.
de6027c
to
4dbafb2
Compare
I haven't updated the unit/e2e tests yet but otherwise this should be ready for a first round of review! |
Reviewing the feedback from when we last implemented something similar to this in #39973 (which I totally forgot about lol):
These were the concerns that I can see were raised:
|
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.
Thanks for working on this, @noisysocks!
✅ Tested that all new fields work.
✅ The featured image filters still apply. While filters still work, do we maintain the same CSS classes so we don't break styling for overrides?
✅ I can't see Featured Image as a contributor.
Notes
- I noticed an empty gap with borders when the post type doesn't support any of these fields (see the screenshot).
- What do you think about using
object-fit: cover
for the featured image?
Uses a new dropdown-based flow for selecting a featured image.
It is a big difference from the #39973. While I don't mind this new flow, considering the recent discussion in #42175, do you think we should leave the old image upload UX?
Screenshot
Very excited for this, will do some testing later this week! |
Oops! Fixed in 67d9a55.
I'll defer to @javierarce on these two questions 😀 Personally I really like the feel of this featured image flow. It lets us add an Upload button, which is very convenient, without forcing the user to make decisions before they have signalled an intent to change the featured image. |
I've updated the tests! 🤞 |
@javierarce @Mamaduka: How are we feeling about this? I'd like to merge it this week and then work on points 2 and 3 in #42352 (comment) as follow-up PRs once we've figured out #42175. |
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.
@noisysocks, I think PR is in good shape to merge.
My only minor concern is using popup for upload actions (#42352 (review)), but I would defer to @javierarce on that matter.
Regarding excerpt. |
Excerpt is similar to title in that it's up to the theme whether HTML is supported or if it's escaped before being rendered. It would be a good enhancement to make but requires some careful thought about how themes can indicate that they support HTML excerpts (maybe using |
This works great, great job! That said, there are a couple of things we need to address before merging it: First of all, the accessibility of the text fields. This @carolinan mentioned here and we should address it. In the design file, I proposed using an edit icon that appears on hover, but that might not be enough. I'll try to think of a solution for that. The other thing is the placement of the popover. Right now we show it besides the sidebar (and vertically centered when there's an image). Since we always open popovers inside the sidebar, we should do the same thing here and place the popover under the link or the edit icon. |
Good work Robert @noisysocks ! Hmm my initial reaction is some hesitancy. I brought a screenshot into Affinity Photo and reordered fields, and came to the conclusion that the current UI does work well. Looking at it side by side is interesting though. Here is a mockup and the current exploration. I would though when hovering over the featured image add a popup/tooltip saying "Featured Image" As have been mentioned above. Clicking into the title and excerpt it would be helpful to include a thin outline giving the signal that it is editable. The side popup.I went ahead and compared it with clicking the Color: Text, Background and Link. It would be nice to also align the Featured image popup along the top edge/line of Featured image area. |
👍 I've changed the popover to appear below the featured image button.
I agreed with this but then after implementing it I changed my mind. I don't think a label adds any value. There is already a label that appears in the popover when you click on the image, there is already a pencil icon which appears on hover, and there is already an Here's what it looked like: Kapture.2022-07-19.at.12.59.39.mp4
Would you mind if we did this in a follow-up? It's something that requires design feedback which I think would be easier to obtain in an issue or a PR that is solely focused on addressing this. |
I was going to say that it would be nice to open popups on the left side of the sidebar (similar to Color options), but then I began clicking Visibility, Publish, URL and Template and noticed they all opened below each option. Clicking Set featured image and having the popup show up in the same way keeps a consistent way of handling the popups. One thing that came up. Should there be a placeholder outline around the "Set featured image" text? (As I think more about it...there is probably no need for it.) I will add the above comment even though it really does not add to the discussion. Good work Robert! |
One small nitpick on the design: The pencil icon almost tricks me into believing I have to click it to edit, despite being able to click the entire image. I wonder if there's a hover effect we could use that better communicates that the entire area is clickable, kind of like when the featured image is not set? Apologies if this already came up :) |
And one small issue I noticed with the excerpt – if you edit via the Inspector, any formatting you had is stripped: formatting.mp4This inevitably raises the question – should there be formatting options for the excerpt in the Inspector? |
Hey all, was wondering if we could give this a try as a part of the pre-publish panel instead? It feels like we're running into some of the same issues as the previous exploration that was reverted. I left a comment on #39082 (comment) as well, let me know what you think. |
Yep, let's close this out since there's consensus on trying something else. See #39082 (comment). |
What?
Why?
Part of #39082.
How?
PostFeaturedImage
component in@wordpress/editor
.FormFileUpload
.PostSummary
in@wordpress/edit-post
.@wordpress/editor
as the design we're using is pretty tailored to the post editor sidebar.EditableText
as rich text formatting is not supported here.@wordpress/edit-post
but not@wordpress/editor
as third parties may be using them.Testing Instructions
Screenshots or screencast
Kapture.2022-07-13.at.15.06.00.mp4