-
-
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
[2.x] Refactor navigation menu container bindings #1581
[2.x] Refactor navigation menu container bindings #1581
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## improved-navigation-internals #1581 +/- ##
===================================================================
- Coverage 100.00% 99.91% -0.09%
+ Complexity 1758 1757 -1
===================================================================
Files 183 183
Lines 4756 4754 -2
===================================================================
- Hits 4756 4750 -6
- Misses 0 4 +4 ☔ View full report in Codecov by Sentry. |
This reverts commit e4957b8.
ca7e73c
to
c9414b5
Compare
This reverts commit 4349a46.
packages/framework/src/Foundation/Providers/NavigationServiceProvider.php
Outdated
Show resolved
Hide resolved
34a055d
to
63a2d87
Compare
This reverts commit a746597.
63a2d87
to
ff17137
Compare
7248ccd
to
a25423e
Compare
Being able to do app(MainNavigationMenu::class) instead of MainNavigationMenu::get() offers little value. It does have some benefit compared to using the aliases as the former can probably get better IDE support, but since the idea behind the alias is convenience I don't think it's worth it since the static method offers full type support. We also don't need the initial null bindings as the string versions throw an exception instead of trying to resolve the classes. It does mean the static get method breaks but that's fine as I will revert that next
This reverts commit 9675665.
This reverts commit fce0950.
This reverts commit fb7a8d5.
e414a8b
to
1ec568e
Compare
1ec568e
to
fce1f3f
Compare
77d6a42
to
5afae61
Compare
PR Summary
|
Targets v2.x via #1568
See #1568 (review)