-
-
Notifications
You must be signed in to change notification settings - Fork 6
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
[v2.0] Refactor navigation internals #1508
Comments
Need to consider how to handle the sidebar, but initially I'm gonna try to add the navigation as a kernel property |
Thinking more about this, and if the navigation belongs in the kernel. Not all sites need navigation, and it's strongly coupled to a component, so why should the kernel be responsible for booting the navigation? The original problem I'm trying to solve is that the only way for say a package to add a navigation menu item is by adding it to the config at runtime which feels quite hacky. This is because the navigation menu instance is created from the navigation view. Maybe it makes more sense to instantiate the navigation earlier on, but in a provider or similar. Then we can expose a navigation API around that. 95% of sites still probably will use navigation, so maybe we still add a helper in the kernel to access it. |
Wondering if this API could be used in the config to initialize menus. It's kinda Filamenty. Since we don't cache configs (except for in the standalone which uses a caching hack) it should not be a problem. Worst case, that can be serialized in our configuration loader. 'menus' => [
NavigationMenu::create('main')
->withPageTypes('all', except: [DocumentationPage::class])
->withoutPages('404')
->withLabels([
'home' => 'Start',
'about' => 'About Us',
])
->withPriorities([
'home' => 100,
'about' => 200,
]),
NavigationMenu::create('docs')
->withPageTypes(DocumentationPage::class)
->withLabels([
'readme' => 'Readme',
'installation' => 'Installation',
'getting-started' => 'Getting Started',
])
->withPriorities([
'readme' => 100,
'installation' => 200,
'getting-started' => 300,
])
->withoutPages([
'404',
'changelog'
]),
], They can be accessed like this: $main = NavigationMenu::get('main'); // Main menu
$docs = NavigationMenu::get('docs'); // Sidebar
$foo = NavigationMenu::get('foo'); // Throws with message stating menu must be specified in config We could of course add the defaults to the base classes, so only overwrites would be needed. NavigationMenu::main()
NavigationMenu::docs()
NavigationMenu::create() // For custom ones And users could easily add their own navigation menus. NavigationMenu::create('footer')
->withRoutes(routeKeys: [
'privacy-policy',
'terms-of-service',
])
->withNavItems([
NavItem::create(...['some data']),
NavItem::create(...['some data']),
])
->withLinks([
'External Link' => 'https://example.com',
])
// Or shorthand
->with(routeKeys: [
'privacy-policy',
'terms-of-service',
], links: [
'External Link' => 'https://example.com',
]), We could also have subdirectory configuration on a per menu basis NavigationMenu::create('docs')
->useSubdirectoriesAs('dropdown') |
Consider updating dropdown handling according to following proposed docs: Dropdowns are represented by a NavItem with a children array containing more NavItem instances. |
Possible new Navigation internal pipeline could be as follows: After Kernel boot (perhaps in a NavigationExtension) we use a discovery-like process to generate all the navigation menus for the application (borrowing code from #1538). We then store all these in an array of a Navigation singleton (which we could also bind to the kernel). This then exposes a hook to add navigation items from a code API. |
If we change the config settings, we could consider changing |
Fixed by #1568 |
For v2.0 I want to refactor navigation internals to better unify the configuration API and reduce discrepancies with the sidebar code. I think we should also consider storing the global navigation items in the Hyde kernel as that could make it easier for extensions to programmatically add navigation items.
The text was updated successfully, but these errors were encountered: