-
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 Featured Image: New design for Replace and Remove buttons #50269
Conversation
Size Change: +1.14 kB (0%) Total Size: 1.37 MB
ℹ️ View Unchanged
|
Nice, thanks for doing that. This is important polish. I feel like it's nearly there. Before: After: Overall this is much better than before, with a better footprint. A few things, though. In this state, we don't need the flyout. You can drag and drop images, and you can upload inside the media library modal anyway. We can remove the upload and go straight to the media library: In fact, if we remove the upload button, we are left with just two actions, open media library and remove. So can we remove the dropdown entirely, and have two buttons? Just an edit button, and a trash button, as overlay buttons in the top right corner. Clicking the thumbnail just opens the media library: As an overlay button, it's important that it overlays the image, it can't just sit in the white space when the image doesn't fill the area, as it does in square or portrait orientations: Additionally, portrait featured images get very tall, there's no reason to. I meant to go in and push a fix, so instead of these: we get this: I pushed a small fix to the focus style and radius as well. Note that there is a bottom margin we need to remove: But I can't find it in the codebase. Can you find it and remove it? We might also not need the position relative. So to summarize:
Thank you Robert! |
Popping in as a user to say that I would very much prefer to see ^ this rather than the flyout menu. Some kind of overlay for editing and deleting is a better user experience. Flyout menus that live outside the Inspector visually but are triggered from the Inspector can become problematic very quickly. |
Thanks for the feedback @jasmussen. I think I addressed all of it. Let me know if not. Kapture.2023-05-04.at.16.17.09.mp4Not sure if you meant for the aspect ratio to be like this on narrow viewports? |
I'm glad we got rid of the flyout 🎉 |
If the icons were centred on the bottom, then they would appear on top of the image even if the selected image is a square and doesn't take up the entire featured image div., unless I'm misinterpreting and the plan is to make all images fit the entire featured image div. 🤔 |
Looks good to me, thanks for the followup! This looks pretty much ready to go to me, though I think I saw Matías mention Replace/Remove buttons instead of icons? I don't personally have a strong opinion there and can go either way. Tabbing works too: That said, notice at the end of the GIF, there appears to be a focus loss if you invoke either of the two Replace/Remove buttons. I.e. if you tab to the thumbnail itself, press enter, you open the media library modal. Escape closes it again, and sets focus back on the thumbnail. However if you tab to the Edit button, press Enter, then Escape to close the modal, the focus is on |
The design he shared in #50269 (comment)? I've gone ahead and implemented it in 258b603. I also added a transition in 4706919 as that felt right. Feel free to tweak the glass effect, transition, or revert the commits altogether if you prefer the icon design. Kapture.2023-05-05.at.10.13.48.mp4
Fixed in 3909b6e! |
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 tests well for me, and is a substantial improvement over trunk. Let's get it in.
Let's get it in and iterate, it's nice to see all the QoL improvements to the post settings panel! |
What?
Updates the post editor's Featured Image panel to use
the dropdown-based design that was built in #42352 but never shippeda new design.Why?
It gives us room to add an Upload file button.Testing Instructions
Screenshots or screencast
Kapture.2023-05-03.at.11.59.26.mp4