-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Page List: Invoke the convert to links modal on any interactions with the page list item block #47748
Conversation
event | ||
); | ||
} | ||
if ( blockSelectionResult !== false ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is needed to allow blocks to prevent their selection.
packages/block-library/src/navigation/edit/menu-inspector-controls.js
Outdated
Show resolved
Hide resolved
'This menu is automatically kept in sync with pages on your site. You can manage the menu yourself by clicking "Edit" below.' | ||
); | ||
|
||
export function ConvertToLinksModal( { onClick, disabled, closeModal } ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to convert this into a controlled component so it could be invoked from other places.
@@ -242,6 +202,10 @@ export default function PageListEdit( { | |||
title: page.title?.rendered, | |||
link: page.url, | |||
hasChildren, | |||
onSelect: () => { | |||
openModal(); | |||
return false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
returning false stops the block being selected in the canvas.
Size Change: +79 B (0%) Total Size: 1.31 MB
ℹ️ View Unchanged
|
I don't really like that the modal opens up on click. To me it seems like clicking should select the page list block instead where you then have the option to edit it. That seems to match the experience of the in-canvas editor right now. |
We shouldn't need to edit off canvas and the navigation block, in my head, it should be something that only page list knows about. That means:
|
The changes in these components are simply to pass control of how interactions are handled to the block itself. Other blocks could handle this in different ways depending on what behaviour they wanted. |
But it shouldn't be a general thing, right? It's a feature of the page list that it's locked but not really only when in navigation. Sounds so specific to me, I can't think of other blocks that would have this weird behavior. |
I broke the refactoring part into another PR: #47922 |
Taking this one for a spin, but I may be doing something wrong as I'm not able to reproduce the modal: On a separate note, Can we visually hide the Page List "container" block? I.e. instead of:
We have just:
? |
This is the wrong interaction model. We should close it and do it a different way (with a useEffect in Page List Item). |
Close in favor of #48723 |
What?
When users interact with the Page List Items in the Navigation List View, we should invoke the "convert to links" modal, to make it clear that these items can't be edited.
Questions
cc @SaxonF @richtabor
Fixes #47064
Why?
To make it clear that these items can't be edited, and allow users to do so.
How?
This allows blocks to create their own
onSelect
attribute which is a function which is called when the block is selected in the OffCanvasEditor.Testing Instructions
Testing Instructions for Keyboard
As above.
Screenshots or screencast
Screen.Recording.2023-02-03.at.18.51.06.mov