Skip to content
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

Navigation screen: Better handling of unsaved changes #30322

Closed
talldan opened this issue Mar 29, 2021 · 7 comments
Closed

Navigation screen: Better handling of unsaved changes #30322

talldan opened this issue Mar 29, 2021 · 7 comments
Labels
[Feature] Saving Related to saving functionality Needs Design Feedback Needs general design feedback. [Type] Tracking Issue Tactical breakdown of efforts across the codebase and/or tied to Overview issues.

Comments

@talldan
Copy link
Contributor

talldan commented Mar 29, 2021

What problem does this address?

The navigation editor currently doesn't warn if a users attempts to exit without saving.

There are a few instances this may happen:

  1. A user makes some changes, and navigates away
  2. A user makes some changes and switches to a different menu
  3. A user makes some changes and creates a new menu

Some of the challenges here are because the menu screen allows users to edit multiple menus without reloading.

On the same topic, the screen also allows users to repeatedly save even if there are no unsaved changes, so it seems there's currently no detection of unsaved changes at all.

What is your proposed solution?

Step 1 - Detect unsaved changes, and only enable the save button when there are unsaved changes.

Implement detections of unsaved changes. In the nav editor I think there are two 'entities' that an be edited - the navigation post and the menu. This would need to be double-checked, but hopefully it should be possible to use the entity system to check for edits.

Then make the save button 'disabled' when there are no edits.

Step 2 - Warn the user if they navigate away

The user should always be shown a 'confirm' warning if they try to navigate away.

Step 3 - Handling saving for multiple menus

The screen could also borrow from the other editors and allow the user to save multiple menus at once, a bit like editing reusable blocks in the post editor or template parts in the site editor:
Screenshot 2021-03-29 at 2 11 09 pm

Alternatively the screen could use the 'confirm' warning when switching menus or creating a new menu if there are unsaved changes.

@talldan talldan added [Type] Enhancement A suggestion for improvement. [Feature] Saving Related to saving functionality [Feature] Navigation Screen labels Mar 29, 2021
@talldan talldan added this to Inbox in Navigation editor via automation Mar 29, 2021
@talldan talldan moved this from Inbox to Needs dev in Navigation editor Mar 29, 2021
@talldan talldan added the Needs Design Feedback Needs general design feedback. label Mar 30, 2021
@shaunandrews
Copy link
Contributor

For "Step 1" of your outline, I think we can take some cues from the site editor and make use of the dot to help indicate when there are unsaved changes. Along with that, it might be helpful to have a disabled state for the save button with a tooltip explaining why it's disabled.

Menus Unsaved Changes

"Step 2" makes sense, too, and I think would just make use of a normal browse dialog to confirm or cancel navigating away from the editor when there are unsaved changes.

I'm not so sure about "Step 3." I think the multi-entity saving flow might be too complex in the context of the menu editor. Though, I do like that it could allow for tracking changes while switching menus — I'm just not sure we need to display the checkboxes and pre-publish confirmation sidebar. Maybe we can just always save all the changes when the button is clicked?

@grzim
Copy link
Contributor

grzim commented Mar 30, 2021

@shaunandrews so all but delete menu will be saved upon clicking the save button. Shouldn't the delete button be placed somewhere else then? I think it would be more UX friendly

And the second question - how the dots should look like when there are changes made in checkboxes (theme location) and toggle (add new pages)?

@talldan
Copy link
Contributor Author

talldan commented Mar 31, 2021

I like the way the dot works, that's very cool.

I'm not so sure about "Step 3." I think the multi-entity saving flow might be too complex in the context of the menu editor. Though, I do like that it could allow for tracking changes while switching menus — I'm just not sure we need to display the checkboxes and pre-publish confirmation sidebar. Maybe we can just always save all the changes when the button is clicked?

Yeah true. Lets go with that.

@paaljoachim
Copy link
Contributor

Remember that sometimes the user does not want to save a change but wants to discard changes.
So one should be able to Save or Discard changes. Both instances the dot should go away...

@Mamaduka
Copy link
Member

Should we also copy the UnsavedChangesWarning component for the edit-navigation package as well? This is based on #26081.

@Mamaduka
Copy link
Member

@talldan, I think we can close this one. First, two steps are now handled in the navigation editor.

@Mamaduka
Copy link
Member

@getdave created a new issue for tracking step 3 - #31813.

Navigation editor automation moved this from Needs dev to Done May 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Saving Related to saving functionality Needs Design Feedback Needs general design feedback. [Type] Tracking Issue Tactical breakdown of efforts across the codebase and/or tied to Overview issues.
Projects
No open projects
Development

No branches or pull requests

5 participants