Skip to content

Conversation

@emmadesilva
Copy link
Member

@emmadesilva emmadesilva commented Feb 13, 2024

TLDR: This breaks... everything. The entire Navigation API is essentially rewritten. The easiest way to upgrade is going to be reimplementing existing code from scratch.

New attempt at fixing #1508 after closing #1538. It targets HydePHP v2 #1565

With this, NavigationMenu classes are no longer responsible for generating the menus, instead the API is changed so that they are just data containers. This better follows SOLID principles, and changes the point where we generate the menus in a way that's more controllable.

This rewrites the navigation internals in a few breaking ways. (Note that this list may be incomplete/outdated, the final version will be present in the release notes)

  1. Breaking: Change configuration option docs.sidebar_order to docs.sidebar.order
  2. Breaking: Change configuration option docs.table_of_contents to docs.sidebar.table_of_contents
  3. Breaking: The NavItem::fromRoute() method is now NavItem::forRoute()
  4. Breaking: The docs.sidebar.footer no longer supports true as a value
  5. Breaking: Rename class NavigationMenu to MainNavigationMenu
  6. Breaking: Remove class MainNavigationMenu (essentially replaced/rewritten to the new NavigationMenu class)
  7. Breaking: Remove class BaseNavigationMenu
  8. Breaking: Remove class DropdownNavItem
  9. Breaking: Make BaseNavigationMenu::$items protected
  10. Breaking: NavItem::$destination is now a Route instead of string (This is necessary in order to defer route resolving to compile time)
  11. Breaking: Remove method DocumentationSidebar::getGroups
  12. Breaking: Remove method DocumentationSidebar::getItemsInGroup
  13. Breaking: Remove method DocumentationSidebar::makeGroupTitle
  14. Deprecate class BaseNavigationMenu
  15. Deprecate class MainNavigationMenu
  16. Deprecate class DocumentationSidebar
  17. Deprecate class DropdownNavItem
  18. Deprecate method DocumentationSidebar::makeGroupTitle()
  19. Merges DropdownNavItem features into main NavItem class

Changes:

Since the navigation menu is now essentially cached in the kernel, instead of being created each time the component is rendered, the kernel must be cleared during some tests.

@codecov
Copy link

codecov bot commented Feb 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.97%. Comparing base (c42d912) to head (d0bc2d4).

Additional details and impacted files
@@             Coverage Diff              @@
##             2.x-dev    #1568     +/-   ##
============================================
  Coverage      99.97%   99.97%             
+ Complexity      3504     1775   -1729     
============================================
  Files            364      184    -180     
  Lines           9474     4791   -4683     
============================================
- Hits            9472     4790   -4682     
+ Misses             2        1      -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@emmadesilva emmadesilva force-pushed the improved-navigation-internals branch from a038839 to ae98599 Compare February 13, 2024 16:26
@emmadesilva emmadesilva changed the title Improved navigation internals [2.x] Rewrite navigation internals Feb 13, 2024
@emmadesilva emmadesilva force-pushed the improved-navigation-internals branch from a294739 to 786bdf6 Compare February 13, 2024 18:22
* @covers \Hyde\Framework\HydeServiceProvider
* @covers \Hyde\Framework\Concerns\RegistersFileLocations
* @covers \Hyde\Foundation\Providers\ConfigurationServiceProvider
* @covers \Hyde\Foundation\Providers\NavigationServiceProvider
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be in its own test

@emmadesilva emmadesilva force-pushed the improved-navigation-internals branch 3 times, most recently from 6515174 to b617d88 Compare February 13, 2024 19:15
@emmadesilva emmadesilva force-pushed the improved-navigation-internals branch 9 times, most recently from dfc8d29 to 1df9636 Compare February 14, 2024 15:30
@emmadesilva emmadesilva force-pushed the improved-navigation-internals branch 2 times, most recently from 9515635 to 82ee80d Compare February 14, 2024 17:43
@emmadesilva emmadesilva force-pushed the improved-navigation-internals branch 6 times, most recently from 9469d52 to f2becad Compare February 14, 2024 20:04
emmadesilva and others added 19 commits March 27, 2024 20:50
While the built-in views does not support it (and the generators won't use it), the group class itself now supports containing groups as it's children, allowing developers to utilise the system to create more complex navigation menus.
…-design

[2.x] Extract trait for shared code with navigation menu class
This partially reverts commit bccde3d.
…-design

[2.x] Merge trait back into menu class and make navigation groups extend the menu class
[2.x] Simplify and normalize public navigation APIs
@emmadesilva emmadesilva marked this pull request as ready for review March 31, 2024 14:30
@emmadesilva emmadesilva force-pushed the improved-navigation-internals branch from d0bc2d4 to cc2756e Compare March 31, 2024 15:40
@emmadesilva emmadesilva merged commit 7b6a7ad into 2.x-dev Mar 31, 2024
@emmadesilva emmadesilva deleted the improved-navigation-internals branch March 31, 2024 15:41
@emmadesilva emmadesilva added this to the v2 milestone Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants