-
-
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
Refactor navigation internals to store navigation in the kernel #1538
Conversation
This breaks due to NavItem constructor forcing routes to strings which makes the links resolve too early. We can't evaluate dynamic links until individual page compile time. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## 2.x-dev #1538 +/- ##
==========================================
Coverage 99.95% 99.95%
- Complexity 1746 1754 +8
==========================================
Files 181 181
Lines 4725 4738 +13
==========================================
+ Hits 4723 4736 +13
Misses 2 2 ☔ View full report in Codecov by Sentry. |
Now needed in unit tests to get the navigation. Not an issue for actual sites.
a5ea5fa
to
89730ae
Compare
…-deferring-route-resolves Update NavItem class to support deferring route resolves
dfc5a58
to
94c1f37
Compare
packages/framework/src/Foundation/Concerns/ManagesHydeKernel.php
Outdated
Show resolved
Hide resolved
This reverts commit ed6c6a7.
Consider updating dropdown handling according to following proposed docs: Dropdowns are represented by a |
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') |
c454dc1
to
7b6b987
Compare
I think I need to hold of on this implementation for a bit. It doesn't support multiple navigation menus well. |
No description provided.