-
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
[Site Editor]: Update the add template menu #50595
Conversation
@@ -25,46 +25,8 @@ | |||
|
|||
&__contents { | |||
> .components-button { | |||
padding: $grid-unit-30; |
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.
These styles were moved below to be reused by both Modals. Only a couple of properties need to be different from each Modal right now. Also I'll have to remove any obsolete css in the end and update probably to use a single Modal.
Size Change: -1.52 kB (0%) Total Size: 1.39 MB
ℹ️ View Unchanged
|
Flaky tests detected in 8cfd4d2. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4959846764
|
This work may end up being superseded by #50395, but I think it's still an enhancement worth merging. If we don't get to 50395, at least we'll have a destination in which to send people for template creation, which is something the current popover doesn't offer. Couple of issues to address:
|
I pushed some adjustments to the modal dimensions: add-template.mp4I'm not in love with the middle step, the whitespace of overly pronounced, but I think this is an acceptable trade-off vs the current experience, where the modals are constantly jumping around due to their different dimensions. The process feels like a single flow now rather than three disparate steps. |
50a5cb5
to
12e50cf
Compare
Thank you @jameskoster! I've been refactoring a bit to reuse the same modal and will need more polishing. I pushed the first batch of changes and also added your changes, because I had to force push due to lots of conflicts. In general do you think this is something worth spending a bit more time to merge? Because you mentioned this could be superseded by #50395. |
b171bcb
to
2517beb
Compare
😍 That was my only remaining gripe, and I assumed it would involve more work. Thanks for tackling this! For me this is worth merging, if it's not too time-consuming to get into a merge-able state. It's not 100% clear to me how much of #50395 we'll get to for 6.3, there's still some design work to be done there. This change feels like a really nice quality-of-life improvement to the current flow. |
253ccea
to
5a939f2
Compare
Pushed a couple of minor tweaks. I think this feels like an enhancement versus the flow on trunk and is ready for merge, but would appreciate another set of eyes from @WordPress/gutenberg-design. |
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.
Definitely a step forward, kudos for pushing through on this, all involved.
Trunk shown in this GIF with a template dropdown:
This PR with a gif showing the dropdown replaced by a large and spacious modal:
There's an open question on mobile, forgive me if I missed conversation. But that simply fails silently, it appears:
Or rather, it appears, it's just impossible to create or even manage templates on mobile. Is this intentional? It seems separate to this issue though.
Co-authored-by: Marco Ciampini <[email protected]>
Yes, I checked the mobile now and observed some issues there in general with the header as well. Let's check them separately. |
What?
Resolves: #50134
This PR updates the add template menu in site editor.