-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Navigation: Experiment with adding navigation link variations #29011
Conversation
Size Change: +312 B (0%) Total Size: 1.38 MB
ℹ️ View Unchanged
|
Nice start with this prototype. It shows how complex this setup is.
I agree that the placement isn't good. I don't think we have any side effects defined for others blocks outside if the Ideally, we shouldn't need to use REST API here at all. The only question is what would be the preferred way to set those block variations on the server. Could it maybe work as Block Patterns instead? See explorations for the Query block started by @ntsekouras in #28891. |
Thanks @gziolo for taking a look!
💯 on this. I sketched out this approach since there isn't a corresponding
That might be interesting. @jasmussen what do you think? I suppose the main interaction difference, is selecting a link type from the inserter, vs inserting from a flow in the block placeholder. Some cases to consider: within the navigation block, each link is relatively small so the placeholder area will be smaller too. We'll also want to consider a site that has a reasonable number of CPT/Taxonomies added |
@gziolo After playing a bit, block patterns feels like not a good match for this use case, unless we'd like to rethink how we'd like to build a menu navigation at a higher level. cc @shaunandrews @jasmussen From what I understand, block patterns are content focused. So it makes sense for when we know the full example content (eg two grouped buttons), or are using a single dynamic/query block. "Give me a navigation menu that reflects all pages" is a great example.
However if we try this with say a tag variant, we might be able to provide a placeholder, but how would we add another tag or other link variant after that? (see placeholder work in #28861):
|
All valid points if you think about it overall. The differences between block variations and block patterns are nuanced for more complex blocks. It all depends on what you want to achieve. If there is no way to make it fit the existing APIs, we should evaluate adding a new API that allows registering block variations on the server. There is prior art available for block styles (API introduced before block variations, that can be seen as a subset of functionality): |
Interesting challenge: Did you consider providing the "variations" key of the block server side instead? I mean technically this is equivalent to the suggested Conceptually speaking, I'm trying to think wether it makes sense to support "dynamic variations" from the client side perspective, or if there are other Block APIs that could benefit from being dynamic. That said, I'd prefer for things like that to take some time to materialize as a generic use-case/API so it's probably better to just start adhoc and keep in mind potential new abstractions in the future if the need materialize more obviously. |
The issue is that we don't expose variations from the block type definition to the client. We would need to add handling in a few places, similar to Temporary helper to pass definitions to the client:
REST API endpoint (long term way to pass definitions to the client): |
@youknowriad @gziolo thanks for taking a look! I sketched out the alternative in #29095, is this what you were thinking of? |
Yes, that would be another way of doing it. I'm curious what @youknowriad had in mind. |
Yep, that PR is exactly what I had in mind :) |
Thanks for the ping, sorry for the late response. There's a technical angle to this that is possibly over my head, so I'll respond mostly to the user experience side of things.
I think we should absolutely use block patterns for the navigation block, perhaps even in the setup state as it's used with the Query block soon. We might potentially be able to leverage some of this design by @jameskoster. However to your own point, block patterns are definitely about content, and providing a shortcut to nice layouts. You are meant to customize the content inside a block pattern after the fact. For example I would imagine most navigaiton block patterns to simply include the Page List block, the Social Links block, and the Search blocks, and any combination of those three. If you need custom links, you'd then need to detach the page list block, or to add your own items. Was that helpful? |
Thanks for chiming in @jasmussen!
Very helpful to confirm that 💖. I think we'll end up needing both patterns to as you said provide shortcuts and some way of providing CPT/Taxonomy links one by one (which is this PR). I do think I can rule out patterns for this problem as a technical choice, since the navigation configurations we're setting up isn't known beforehand. Eg it'd be a little different if we know we wanted 5 links of X type, or wanted to build a dynamic block that mirrors all categories. |
Going to close this draft out for now in favor of #29095 |
WIP Part of #24814 this PR experiments with registering navigation link variants.
This approach requires WordPress/wordpress-develop#1002 to test.
test.mp4
Request For Feedback
cc @noisysocks @gziolo
I put together a pretty quick draft of what this looks like if we populate navigation link variations from the API
registerBlockVariation
calls is a bit awkward. I had trouble finding a good spot. Our core data stores work best with a react component, so I instead opted for a directapiFetch
call which might not be ideal.type
sort of corresponds to a taxonomy.slug or postType.slug value (with a hardcoded exception to tag, which ispost_type
). At a minimum I think I need to pass through if it's ataxonomy
orpost-type
. I'm also not sure if slug is dependable, beyond the default built-ins. Can slug be renamed. Are collisions allowed? Eg should we be using a post type id/taxonomy id instead?TODO
customize_nav_menu_available_item_types
filter?Testing Instructions