[ experimental/nav-menus ] Delete menus in nav menus screen#21486
Merged
draganescu merged 13 commits intomasterfrom May 7, 2020
Merged
[ experimental/nav-menus ] Delete menus in nav menus screen#21486draganescu merged 13 commits intomasterfrom
draganescu merged 13 commits intomasterfrom
Conversation
2 tasks
|
Size Change: +3.57 kB (0%) Total Size: 822 kB
ℹ️ View Unchanged
|
Member
|
For me, this wouldn't be a button and I think we should avoid this. There are a few issues in the implementation:
A very easy win here I think is just to make this simply have the text 'delete', this aligns more with the wp-admin interface. What there is now:
Let's bring this in with the save button at the top, right and then a simple text 'Delete Navigation' at the bottom:
|
Contributor
Author
|
Sure @karmatosed that is a great observation! I just went ahead and implemented "a button to click on" :) This mockup is a lot better. It also solves #21523 |
noisysocks
requested changes
Apr 17, 2020
e4102a5 to
f74c24e
Compare
Contributor
Author
|
@noisysocks take this for a new spin and check out the current state code, I've added your suggestions. Thanks! |
f74c24e to
239fe59
Compare
Given deleting entities is a complex endeavor, that will eventually allow to undo deletes before saving, this approach advances the navigation screen and allows for user testing.
removes extra getEntities call
239fe59 to
a3d44c8
Compare
noisysocks
reviewed
May 1, 2020
Member
noisysocks
left a comment
There was a problem hiding this comment.
This is looking great! Nice work.
packages/edit-navigation/src/components/menu-editor/block-editor-panel.js
Outdated
Show resolved
Hide resolved
Contributor
Author
|
Howdy @noisysocks this is ready for re-reviewing :) |
adamziel
reviewed
May 5, 2020
adamziel
reviewed
May 5, 2020
adamziel
approved these changes
May 5, 2020
Contributor
adamziel
left a comment
There was a problem hiding this comment.
I left some minor notes but nothing blocking, good job!
noisysocks
approved these changes
May 7, 2020
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Adds delete button to #21281
Given deleting entities is a complex endeavor, that will eventually allow to undo deletes before saving, this approach advances the navigation screen and allows for user testing.
It is not the best way to implement this, but it allows the new screen to have this behavior now and not wait on some technicalities to be solved.
How has this been tested?
Tested locally by:
Screenshots
Types of changes
Non breaking change that adds a delete call to the API inside the
MenuEditorcomponent and creates a state from initial menus insideMenusEdtiorcomponent.