Multi-entity save UI: Add Discard Changes panel #36185
Conversation
ockham
added
[Feature] Synced Patterns
|
Size Change: +802 B (0%) Total Size: 1.1 MB
ℹ️ View Unchanged
|
|
Looks good Bernie! Thank you! The process is like so:
Clicks Save again.
(I am also seeing a "Continue editing" button further down.) Clicks the "Discard changes" button. Panel is closed and user is returned to working on the layout. The question comes up.... what does discard actually do? |
Yeah, that's what I also think it means. I just haven't implemented that behavior yet 😅 (that's the "Correctly wire buttons, checkboxes, etc" item in my little todo list in the PR desc 😉) |
|
Took me a while to figure out how to implement discarding changes; turns out we didn't have any actions to reset an entity record's edits (that's what discarding changes translates to in Gutenberg parlance), so I had to implement something. This still needs a bit of polish plus some test coverage etc, which I could probably finish tomorrow 🤞 The most important question however is what to do with the undo stack: Typically, every change (edit) creates an entry on the undo stack, which allows the user to go back in time step by step. AFAICT, the feature we're introducing here -- discarding changes on a per-entity basis -- doesn't seem to be entirely compatible with that pattern:
Here's an example: After freshly opening some template in the site editor, the user does the following the site editor, in order:
Using undo, we could obviously go back step by step. Now let's say the user clicks Save, but decides to only save the text change to the footer template part. This means that our new discard panel then asks them if they want to discard the other changes that they made to the header, site title, and tagline. Let's say the user chooses to discard the changes to the header and the site tagline: Those changes are then rolled back, whereas the ones to the site title are kept. Now what should happen to the undo stack (and button)? What would be most intuitive for the user? I can think of some options.
Would that feel intuitive to the user, if they started clicking the undo button? Are there any more intuitive options that I'm missing? |
ockham
added
Needs Design Feedback
|
Forgot to say, there's also the concept of transient edits (which don't create entries on the undo stack, nor do they dirty an entity's state), but I believe that they're "merged" into the next item on the undo stack, and I don't think we want that(?) (Though I have to say my understanding of transient edits might be a bit fuzzy.) |
ockham
force-pushed
the
add/multi-entity-saving-discard-entities-panel
branch
from
7662051
to
4ef3551
Compare
ockham
force-pushed
the
add/multi-entity-saving-discard-entities-panel
branch
from
07e8a7b
to
184d61e
Compare
Thanks a lot @talldan, I was unaware of #36030 -- it's indeed a similar problem. I've been giving this some more thought. We might not necessarily need to allow the user to undo the discard -- it's after all highlighted as a destructive option. For now, I've decided to try to implement the discard through |
ockham
force-pushed
the
add/multi-entity-saving-discard-entities-panel
branch
from
52b8766
to
f652bef
Compare
|
I think this is coming together. I need a bit of design input for one transition, though: After clicking 'Save', GB takes a moment to actually save the changed entities. Currently, it already moves on to the 'Discard' screen, and for a brief moment, all entity change checkboxes are carried over (even the ones that were just saved):
As a minimum, we should probably disable the interactive elements (checkboxes, "Discard changes", and "Continue Editing" buttons) in the sidebar while saving. I'd appreciate some guidance/mockups from a designer! 🙏 |
|
Testing the PR with a "ops I made a change to a Reusable block" situation. As this part of the feature will be extremely helpful for all those folks making accidental changes to a Reusable block. This is what I see: Discard-save-reusable-block.mp4Walking through the scenario. 8- Click the checkbox and then the Discard changes button to discard the option. |
ockham
force-pushed
the
add/multi-entity-saving-discard-entities-panel
branch
from
e007e8d
to
dc377d0
Compare
Sorry for my late reply, @paaljoachim & @ockham… yes, that's exactly what the button does. Regarding the rest of the PR, it looks good! I only miss the success message indicating that the previous save action was successful. Otherwise, the "What's next" title will be confusing. That title is also important because we aren't showing the snack bar. I also think that we could remove the "Continue editing" button. It's too far away from the other actions (the discard button and the close button at the top) and competes with them. If you all agree to make this change, we should then update the message to something like:
|
As @paaljoachim suggests, could we do the opposite? Hide every item and just show the ones that make sense to show? |
|
Something important we should also include is a snackbar message after the user discards the changes. Some suggestions for the message (depending on the number of items reverted):
Another idea. In case there's just one change reverted, we could try this:
|
|
That is a good idea Javier! |
|
What if the user makes one change and decides not to save it? Like so: Should the button with the word "Save" change to a button with the word "Discard"? As there is no way to move onward to the Discard section. |
Ah right, I meant to add that later but forgot. I'll add it!
Agree 👍
Will do! |
|
I've changed the logic for the discard panel to filter the changes that are currently being saved from the list. (It needs some tweaking, since it always removes site entities, such as title and tagline.) Furthermore, I've removed the "Continue editing" button, adapted the wording accordingly, and added the "Templated updated!" message. I now remember why I procrastinated on the latter: its bottom padding is too small, and it seems to require quite a bit of effort to get it right 🙄
Still hatin' CSS 😬 I'll continue on Monday. |
My 2 cents: I don't think that the same button (i.e. same style, same position) should contextually change its function, especially if it's the exact opposite of what it normally does. Users will develop some sort of "muscle memory" and will get used to that button doing one (non-destructive) thing (saving) in most cases; we shouldn't change that to a destructive one in exceptional cases (discarding). (There might even be some more abstract/general design pattern for this kind of thing, but I'll have to defer to designers on that 😅 )
I think that's sufficient -- it's kind of a "cancel operation" function in all cases.
Right, so the major difference is that we'd be lacking an option that allows us to roll back that one change to the state when it was last saved. I think that's okay, since it's only one change, so the user probably remembers what it was like before, and can revert it manually. (Alternatively, reloading the page should have the same effect.) Happy to discuss/implement alternatives! |
ockham
force-pushed
the
add/multi-entity-saving-discard-entities-panel
branch
from
30e4e33
to
b9deaea
Compare
|
The Site Editor Discard flow works well! Site-Editor-Discard-Save.mp4Moving on to.... Making one change. Site EditorStep 1. Step 2. We get trapped. No way to discard the change to the Title. Post EditorStep 1. Step 2. We get trapped. No way to discard the change to the Reusable block. (First of I would suggest to remove the dot seen in the Update button (Post Editor). It would be nice to keep the Post Editor flow as similar to the Site Editor save flow as possible.) We need a discard flow for when no item is to be saved. How would we move on to the discard panel when no item has been checked and the Save button is greyed out? Here is one suggestion. Unchecking so no item it so be saved the Discard button could show up below.
@javierarce Javier what would you suggest? |
|
I see this PR has stopped up. It would be great to get some movement again here! |
|
👋 Chiming in just to say that I'll try to review this PR this week. |
Yeah, sorry about that 😕 As stated on the related issue, the multi-entity saving code is pretty complex, and I had to prioritize some other WP 5.9 stuff. As for this PR, it's even worse: There's the bug (#36096), there's the UX questions (such as this), and then, there's different contexts (the premises for multi-entity saving in the site editor are quite different from the post editor). We might actually need more UX input, especially for the post editor context.
@javierarce Thanks a lot for offering -- I'm just not quite sure this is ready for prime-time yet. Maybe do try it out both in the site and post editors, and have a look at @paaljoachim's comments above; we might need some more UX guidance 😅 |
|
Hey Bernie @ockham I am checking in. Can we get a status update as to the next steps needed? |
annezazu
added
[Feature] Site Editor
Description
This needs some UX feedback from designers, see #36185 (comment)
WIP, needs more work.
Add a second step to the multi-entity save UI which offers the user to discard any changes they made and didn't opt to save. This implements the second part of #31456 (comment) (and thus closes #31456), after #35933 implemented the first part.
How has this been tested?
Screenshots
Current state per this PR:
Types of changes
New feature
TODO
discardEditedEntryRecordsif needed.titleinstead ofkey?)