Add submenu menu item to list view #45794
Merged
Conversation
This file contains 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
ajlende
added
[Block] Navigation
ajlende
requested review from
draganescu,
scruffian,
getdave and
MaggieCabrera
|
|
|
Size Change: +91 B (0%) Total Size: 1.3 MB
ℹ️ View Unchanged
|
draganescu
reviewed
Nov 16, 2022
There was a problem hiding this comment.
Cool! It's a start 🚀 Tested this and it's tricky so far:
- adding a sumbenu on a link converts the link to a submenu and adds a link under it. Is this what I wanted? Did I want to add a "submenu" after this item?
- we need to fix how this works if the item is already a submenu and has contents (they need to be preserved ofc).
- this PR would also be a great exploration of how the LinkUI appears on top of the list view like in the designs.
I also think the wording could be a bit fiddled with ... Add submenu item or Add nested link or something.
packages/block-editor/src/components/off-canvas-editor/block.js
Outdated
Show resolved
Hide resolved
|
We should exclude the "show / hide more settings" given we are in that panel already. Also we should move this item towards the top. |
|
@ajlende FYI I'm going to remove the "Select parent" as well because you already have the "parent" selected (strangely). |
@ajlende Do you have any ideas on how we can selectively enable/disable items in this menu without disrupting the existing List View? Perhaps we just need props which default to Open to better suggestions though... |
The list view and the nav list are probably going to continue to diverge, so I think adding a boolean for every option will be bad for maintainability in the future. I think it's really common to add options like this in Gutenberg because it makes for smaller PRs that are more likely to get merged, but they get in the way in the long-term. I think a better approach would be to break the component down into parts that we can reuse, and for the nav list, we just don't use the parts that we don't need. Unfortunately, doing it this way seems substantially more difficult in the short-term as it already feels a bit like a tangled mess. |
ajlende
force-pushed
the
add/nav-list-add-submenu-menu-item
branch
from
4e32ca3 to
5d28673
Compare
I think these should be done in separate follow-up PRs since they involve updating the |
ajlende
changed the title
|
I don't think they should diverge that much. The show/settings is a context one (you are already in settings). Outside of that, we should aim to have the same set. The "add submenu" one would be useful in the main list view as well. One thing I think we might need to introduce is block specific context so that block specific tools are grouped and separated from general block tools. |
I think that could be pretty useful. |
scruffian
force-pushed
the
add/nav-list-add-submenu-menu-item
branch
from
5d28673 to
2e49a98
Compare
scruffian
approved these changes
Nov 18, 2022
82 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What?
Closes #45441
Adds a menu item to the list view items to add a submenu
Why?
How?
Testing Instructions
Off canvas navigation editorexperiment is enabledScreenshots or screencast