- Issue created by @michaellander
- πΊπΈUnited States michaellander
I'm still a bit conflicted, in that I think we should just allow modules to extend on the layout builder usage, and also allow users to add blocks to the navigation footer. However, I realize there's some hesitancy here, so we still should consider giving some means for modules to extend these areas.
- π¨π¦Canada m4olivei Grimsby, ON
Thanks for your work! It's great having your work on env indicator to get a look at the real world pain points in integration.
Adding a related issue around Navigation's extensibility. I'm not sure ATM if we want to merge the issues or keep both.
- πͺπΈSpain plopesc Valladolid
Not sure if the aim of this issue is exactly the same, but discussion about Config Actions for Navigation has been opened in β¨ Provide Config Action to add new blocks to navigation from recipes Active .
- πͺπΈSpain plopesc Valladolid
This issue has been discussed internally and for now it seems that we are going to rely on Layout Builder's capabilities to manage this.
β¨ Provide Config Action to add new blocks to navigation from recipes Active Will give the possibility to modify the config using recipes, as this is the way to extend layout builder config is expected to use.
3rd party integrations without recipes might need the manual step of adding blocks to the navigation bar once the modules are installed.
- π·π΄Romania amateescu
3rd party integrations without recipes might need the manual step of adding blocks to the navigation bar once the modules are installed.
If "manual" here means "the user has to do it", isn't that quite a regression from what was previously possible with
hook_toolbar()
andhook_toolbar_alter()
? - π·π΄Romania amateescu
A bit more context for #6: in π Integrate Navigation with Workspaces Needs work I proposed a new
hook_navigation_alter()
hook that would provide a similar DX to the Toolbar module.Was that option considered?
- πͺπΈSpain plopesc Valladolid
If "manual" here means "the user has to do it", isn't that quite a regression from what was previously possible with hook_toolbar() and hook_toolbar_alter()?
Yes, I agree.Difference is that new navigation allows to modify the order of the sections through the UI, and it adds an extra layer of complexity that makes the automatic integration of new elements slightly more complex.
This issue was postponed because we currently have other priorities in the roadmap to convert Navigation into a stable module and integrate it with Drupal CMS. π New Toolbar Roadmap: Path to Beta & Stable Active
A proposal for having 2 "toolbar" like sections for the navigation top and footer could be a possible approach. That would leave only the "content" region managed through LB.
Or provide a simpler API for adding Navigation blocks to the LB during module installation, that could be exported as config afterwards.
- πͺπΈSpain plopesc Valladolid
Moving back to active since other issues like π Integrate Navigation with Workspaces Needs work or π Integrate Umami message Needs review
- Merge request !10026Issue #3463142: Allow modules to hook into top of content/footer sections of new core navigation β (Open) created by plopesc
- πͺπΈSpain plopesc Valladolid
Implemented an initial approach to create a new "promoted" region in the Navigation bar that allow modules to place their content programmatically.
My initial feeling was to name it "top", but it could cause confusion with the top bar, so ended up using the "promoted" name for the section and the related hooks. I'm totally open to better naming suggestions.
Regarding the footer, given that's a region that already exists, this change might need more internal discussion. I'm more inclined to leave it out of the scope of this MR for now.
- π¨π¦Canada m4olivei Grimsby, ON
Suggesting a different name of 'header' to go with existing 'content' and 'footer'. Let me know what you think.
Also, do we want to address the region just above the footer, which is asked for in the issue description?
Otherwise, I think this is a fine way to cover this requirement. It offers a lot of flexibility to module authors to put whatever render array they want there. To @finnsky's point he's been arguing for in various places, we'll have to flesh out our components more in order for module authors to author things that fit well in these spots.
- πͺπΈSpain plopesc Valladolid
@m4olivei Property renamed. Could you please take a look into this again?
Static tests are failing due to an unrelated issue in 11.x.
- π¨π¦Canada m4olivei Grimsby, ON
Looking good to me!
The MR doesn't address
footer_top
? I'm wondering if there was a specific reason why not that I'm missing. We either need to add that in as well, or update the issue description to reflect only content_top being covered here. Marking as Needs Work for this reason, but not wanting to discourage integrations from leaving thoughts as well, so please do feel free to leave review comments. - πͺπΈSpain plopesc Valladolid
Thank you for you review @m4olivei.
Footer is a more complex region than content and I would rather to discuss it in more detail in a follow-up issue.
Merging this one would unblock other integrations like Dashboard, Environment Indicator, Workspaces or Umami.
If you are OK with that, i think the only remaining step here would be to write the corresponding Change Record.
- π¨π¦Canada m4olivei Grimsby, ON
Thanks @plopesc! That makes sense. Thanks for the updates.
I've written a draft change record.
Also, updating the description to remove one more reference to footer_top as well as updating the screenshot to reflect the actual implementation.
As we only bumped back to needs work on docs related issues, marking this all the way to RTBC!
- Status changed to RTBC
19 days ago 5:57am 3 December 2024 - πͺπΈSpain ckrina Barcelona
Adding the "Drupal CMS release target" to see if we could hopefully get to it.
- πͺπΈSpain ckrina Barcelona
Adding the "Drupal CMS release target" to see if we could hopefully get to it.
- πΊπΈUnited States michaellander
I went ahead and created β¨ Allow modules to ALSO hook into top of footer section of new core navigation Active so that we could come back to it when timing is more appropriate.
The Needs Review Queue Bot β tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide β to find step-by-step guides for working with issues.
- πͺπΈSpain plopesc Valladolid
Back to RTBC once conflicts have been solved.