Implement Legacy Widget design iterations #30889
Conversation
noisysocks
added
[Type] Enhancement
|
@critterverse: This is still a work in progress but let me know what you think! Also please feel free to push any CSS changes you'd like to make. What do you think about the Apply / Cancel flow? I have two concerns:
|
|
Size Change: +707 B (0%) Total Size: 1.39 MB
ℹ️ View Unchanged
|
|
I made some more progress on this today and have updated the video in the PR's description. I still don't really like the Apply / Cancel flow but I think everything else is pretty smooth! 🙂 |
|
Hi all, took this PR for a quick spin and have a few notes below! Re: Apply/Cancel flow
Great question, @noisysocks. The Legacy Widget has the added complexity of switching between a preview and an editing interface, so I can see why the behavior would be different from a regular block. That being said and now that I’m playing with the PR — removing these actions does feel like it could potentially help simplify the UI and editing flow. One issue I can see with removing the Apply/Cancel part of the editing flow is when it comes to Legacy Widgets without a preview — there’s not really a good way to confirm that your changes have been applied since you don’t get the visual confirmation of seeing the preview updated. Would love to hear from @shaunandrews on this, who may have other watch-outs that I'm overlooking.
Yup — if this behavior remains, Cmd+Z should undo all of the changes that were made when the user clicked Apply. Didn't test this extensively but appears to be working correctly in the PR 👍 Styling Adding a few top-level notes that stood out for the Legacy Widgets (happy to help make these changes but also don’t want to hold anything up!):
|
|
Thanks for the feedback @critterverse! I'll get to work on fixing those styling issues. Here's how this looks with a Done editing button instead of Apply / Cancel. You can play with it yourself by checking out the Kapture.2021-04-21.at.11.17.50.mp4 |
I ticketed this one separately in #31021 as it is already an issue in |
|
Sorry for the confusion, just to clarify on my last comment — I thought the alternative suggestion was to potentially remove the confirmation step entirely, i.e. having the click-away be the Apply action... but sounds like I was misunderstanding on that 😅 Re: confirmation step, I would still vote for Apply/Cancel over Done Editing because it already exists in other unique "editing contexts" like the toolbar for the Image block (and could potentially be extended to the Cover block soon as more editing capabilities are added there). Styling updates look great! Something that may have regressed in the latest push or maybe I'm just noticing is that the tab in the inspector should switch over from the Widget Area tab to the Block tab when the block is selected:
|
You thought correct! In #30889 (comment), if you click away, the changes will be applied. I forgot to demo it 😄. The Done editing button is just a convenience button that unselects the block. We could get rid of it. |
Good catch! Looks like this was already broken in |
Ok, good to know! I really like this idea :) I think we can get rid of the Done editing confirmation step entirely — just want to make sure that the editing flow still works smoothly in that case from an accessibility standpoint. One solution could be to use the |
|
Alright I've removed the toolbar buttons, made it so that the preview shows when in Navigation mode, and updated the video in the PR's description. I'm pretty happy with how this looks and feels! All that's left is to fix the Legacy Widget E2E test which is proving incredibly difficult because it's hard in Puppeteer to tell when an iframe updates. I may have to just disable the test. In the meantime I'll mark this as ready for review. |
noisysocks
requested review from
adamziel,
ajitbohra,
draganescu,
spacedmonkey,
talldan,
tellthemachines and
TimothyBJacobs
as code owners
|
Well done @noisysocks! The simplicity of the toggle between the editing/preview modes using |
noisysocks
force-pushed
the
update/legacy-widget-design
branch
from
cffe24c to
4491cc5
Compare
|
Actually, never mind, we can use |
Closes #26179.
Closes #28671.
Closes #25960.
Closes #28995.
Implements the new Legacy Widget flow in #26179.
Kapture.2021-04-23.at.15.06.30.mp4