WIP Off Canvas Navigation: Edit Button #45575
Closed
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
|
|
|
Size Change: +1.01 kB (0%) Total Size: 1.3 MB
ℹ️ View Unchanged
|
|
Cool stuff 🙇🏻 this is the very next step needed before the drill down system can be added to the experiment. This adds an edit button to the list view. Right now when I click on it I get the dialogue for converting a non existent page list block to navigation links 🐳edit-button-convert.mp4That dialogue should only appear if there is a page list block used in the navigation menu and the user wants to edit a link in the page list. Otherwise when clicking the edit button a panel with the detail selected appears (a la global styles). But the icon, positioning and interaction feels very neat! 👏🏻 |
Thanks for the clarification, I'll be sure to update it. One edge case I found was that we only offer the edit button on Page List blocks with fewer than 100 children: gutenberg/packages/block-library/src/page-list/edit.js Lines 26 to 28 in 6f0e046 My thought is to disable the button with an explanation in that situation, but that's a lower priority. That's what I'm working on next. I'm going to try to reuse the InspectorPanel slots for the slide-over. In the video I noticed that the Page Link edit example is a bit different from the InspectorPanel.
Right now I am thinking we can use InspectorPanel initially and possibly create another slot for off-canvas editing to add things like the URL field. |
|
Excellent. One thing that would be good is to explore the sliding UI in a new PR for clarity and ease of review. Also apparently all my PRs for the list view have been merged and so you cans base your work on trunk now. |
|
One note for the "back" interaction is that we should build it in a way that can apply to any scenario. e.g. if block is passed as prop to inspector show back button that navigates back to that block |
georgeh
force-pushed
the
add/navigation-off-canvas-edit
branch
from
7b96174 to
4cc5c78
Compare
|
@draganescu I squashed my commits to rebase off of trunk, so the diff should be much more usable. Good call on splitting up the sliding panel part from this, it'll be much easier to land this with the fixes for the Page List and follow up with the edit panel for individual items. |
georgeh
force-pushed
the
add/navigation-off-canvas-edit
branch
from
4cc5c78 to
ee39738
Compare
draganescu
reviewed
Nov 10, 2022
@draganescu I'm not seeing links underneath the page list. Is that something that should be working? Or should I add that to this PR?
|
|
Page list does not show its contents in the list view. That is a feature of the page list block that should be added in the future - in a PR specifically for that as it's about that block not about the navigation block. |
priethor
added
[Type] Enhancement
|
Connected to this, I created a PR to add the URL field to the Navigation Link inspector controls panel: #45751 |
|
@draganescu We've moved the panels out of scope for this PR, are there any more features we need before we merge this? |
getdave
reviewed
Nov 15, 2022
packages/block-editor/package.json
Outdated
| @@ -39,6 +39,7 @@ | |||
| "@wordpress/blocks": "file:../blocks", | |||
| "@wordpress/components": "file:../components", | |||
| "@wordpress/compose": "file:../compose", | |||
| "@wordpress/core-data": "file:../core-data", | |||
There was a problem hiding this comment.
The block editor shouldn't have core data as a dependency as Core Data is WP specific whereas block editor is platform agnostic.
| }; | ||
|
|
||
| const ConvertToLinksModal = ( { onClose, clientId } ) => { | ||
| const { records: pages, hasResolved: pagesFinished } = useEntityRecords( |
There was a problem hiding this comment.
We will need to find another way to grab the Pages records from Core.
Likely you will need to pass them as part of the block editor settings via the relevant edit/* store. This way the same code could be utilised across the block and this off canvas list view.
Here is an example of using @wordpress/editor to get information about Core and pass it into the block editor settings for retrieval thereby avoiding the need for the @wordpress/core-data dep in the @wordpress/block-editor package.
There was a problem hiding this comment.
Thanks, I'll see how I can get the data we need into the component without calling core-data directly.
getdave
requested changes
Nov 15, 2022
added 6 commits
November 16, 2022 10:08
It was easier to nuke the commit history than to manually sync a bunch of commits, so this is all the commits in the branch squashed
getdave
force-pushed
the
add/navigation-off-canvas-edit
branch
from
a2f7662 to
4f94b25
Compare
|
@georgeh I'm working on removing the |
|
Ok I suggest we down tools on this one until we're absolutely clear that
@draganescu is working on making the child items of Page List visible in List View so that may invalidate the need for this PR. |
getdave
reviewed
Nov 16, 2022
| ); | ||
| }; | ||
|
|
||
| const BlockEditButton = ( { label, clientId } ) => { |
There was a problem hiding this comment.
As things stand I'm pretty sure this code is going to run for all types of blocks. It's actually only required for the Page List block. It will need to be conditionalised.
|
FYI I decided a good first step would be to land the edit button on it's own. Therefore I spun out #45815. I've put @georgeh down as co-author on that PR as much of the work was borrowed from this one. In the future we may need to implement the work from this PR around Auto Menu conversion. Hopefully not as it adds complexity but at least we have it available should we require it. |
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?
Partially addresses #45442
Why?
This will simplify editing menu items from the navigation inspector
How?
This is based off of the (closed) #44534 branch and will be rebased once the follow-up PRs get merged. As of e1b012a the only branch-specific changes are in
packages/block-editor/src/components/off-canvas-editor/block-edit-button.jsTrigger block inspector overlay for other navigation child blocksmoving to follow-up PRTesting Instructions
Screenshots or screencast