First pass at Navigation screen #21036
Conversation
noisysocks
added
[Type] Technical Prototype
|
Size Change: +23 B (0%) Total Size: 866 kB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Was unable to test, but I wonder if this useNavigationBlocks is an approach that augments the Navigation block. I was under the impression that we'd add an entity provider and have the Navigation block "augmented" to work with that context when it existed and save to block structure otherwise.
|
I don't see the purpose in keeping the block interface at the bottom. There's no way for this interface to know how the theme is styling the navigation on the front-end, including whether or not it is vertical or horizontal. A block-based WYSIWYG interface only makes sense in the context of full site editing, in my opinion. Trying to jam it into this screen is confusing (since it misleads users into thinking the block interface is an accurate front-end preview), and not useful at all in terms of accessibility. I think the purely semantic/structure-based UI at the top is all we need. We could refine it to expose all controls needed to replicate the same functionality that the current wp-admin Menus interface provides. This non-WYSIWYG interface could then be backported to the block, allowing for a refined alternate editing experience for keyboard users. |
|
Howdy @ZebulanStanphill :) This will move towards the recent design iterations in #19170.
This is a very good point! |
|
@ZebulanStanphill: You raise some good points about the design of this screen that are important to discuss! I'd like us to keep that discussion in #19170, though, so that this thread can focus more on the technical approach. I've replied here: #19170 (comment).
@draganescu: This sounds interesting, but I don't quite grok what you mean. (I don't really know anything about entity providers!) Could you elaborate with some pseudocode or, even better, spin up an alternative technical prototype PR? I'm happy for us to explore the problem space by having a few "competing implementations". |
| @@ -67,6 +67,27 @@ export const defaultEntities = [ | |||
| plural: 'comments', | |||
| label: __( 'Comment' ), | |||
| }, | |||
| { | |||
| name: 'menu', | |||
There was a problem hiding this comment.
The generated functions become part of the stable api of core data, but meanwhile the endpoint is still experimental and could be changed. I think it'd be good if the entity could be marked as experimental to match the endpoint. I made small PR that enables that - #21072
| const [ menuId, setMenuId ] = useState( 0 ); | ||
|
|
||
| useEffect( () => { | ||
| if ( menus?.length ) { |
| import Layout from './components/layout'; | ||
|
|
||
| export function initialize( id, settings ) { | ||
| registerCoreBlocks(); |
There was a problem hiding this comment.
Nitpick, do we need all these? We can probably get away with only registering the Nav and Nav Link blocks.
Me neither! I am not even sure this is possible. I was thinking (and trying to no avail) to use Actually I have no idea if anything I am saying here makes sense. After all selecting |
Adds a new Navigation screen, a new edit-navigation package, and a very rough UI for editing menus using the Navigation block.
talldan
force-pushed
the
try/navigation-screen
branch
from
d6f2ada to
ddcbc58
Compare
talldan
requested review from
ajitbohra,
chrisvanpatten,
jorgefilipecosta,
nerrad,
Soean,
TimothyBJacobs and
youknowriad
as code owners
There was a problem hiding this comment.
This give us a good starting point to go off and work on smaller PRs, and it doesn't touch too much existing code, so I think it's worth getting this merged asap so that we can make progress 👍
Good work @noisysocks!
|
Thanks for getting this in |
|
I've created the above issues for the follow-ups. |
|
Hi @noisysocks , I'm trying to use the block API to get the selected menu item ID (the actual post ID). I think it's stored as the clientId (or maybe blockId - I'm not entirely sure at this point) on this screen. The following code works on a standard post/page:
... but not on the experimental navigation page. Is this a bug or am I doing something wrong? I'm happy to open a new issue if needed, but right now I'm not sure if I'm just taking the wrong approach (maybe there is a better way to get the ID of the selected menu item?). Thanks! Edit, I have reported this here #22022 |
|
@manmotive: There's no way to do that right now as the mapping between block client ID and menu item post ID is stored in component state. This might be possible later if/when we move to using |
Very rough first pass at implementing #19170.
Adds a new Navigation screen, a new edit-navigation package, and a very rough UI for editing menus using the Navigation block.
The key functionality is in
useNavigationBlocks— this is a hook which turns menu items from the REST API into blocks, and blocks into CRUD requests. I haven't tested it very thoroughly but I think it's a pretty viable way of implementing this screen.The Navigation block is displayed on the page by rendering a
@wordpress/block-editorBlockListwithtemplateLock: 'all'. This also works pretty well. I tried rendering just theBlockEditcomponent but couldn't get it working.I decided to build an entirely new screen. I did think about a different approach where we swap out
#nav-menus-framein the existingnav-menus.phppage with a React app, but there's a few drawbacks to doing this: 1) We can't have both screens at once in the plugin; 2) We can't implement an "opt out" plugin similar to the Classic Editor plugin; 3) We would have to add a hook tonav-menus.phpin Core to do this; 4) We can't usewp.dataand have to do a lot of painful syncing between jQuery and React.Tasks remaining:
useNavigationBlocks.README.mdto the package.cc. @talldan @draganescu @tellthemachines @adamziel