Skip to content
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] Simplify and normalize public navigation APIs #1642

Merged

Conversation

caendesilva
Copy link
Member

@caendesilva caendesilva commented Mar 26, 2024

Targets #1568 as I when writing the docs realized that having two NavigationItem constructors with differing syntaxes and internal logic adds a ton of complexity for no real reason. This PR protects the constructor to force a single unified API through the static create method. unifies the constructor signatures and logic to provide the exact same results regardless of the syntax used.

It also changes navigation groups to use collections instead of an array for the internal state to match the navigation group system.

Copy link

codecov bot commented Mar 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.97%. Comparing base (f7ae4e1) to head (6042642).

❗ Current head 6042642 differs from pull request most recent head 89c4890. Consider uploading reports for the commit 89c4890 to get more accurate results

Additional details and impacted files
@@                         Coverage Diff                         @@
##             improved-navigation-internals    #1642      +/-   ##
===================================================================
- Coverage                            99.97%   99.97%   -0.01%     
+ Complexity                            1778     1775       -3     
===================================================================
  Files                                  184      184              
  Lines                                 4799     4791       -8     
===================================================================
- Hits                                  4798     4790       -8     
  Misses                                   1        1              

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

@caendesilva caendesilva force-pushed the simplify-public-navigation-apis branch from 8263717 to 3bcafeb Compare March 26, 2024 16:38
@caendesilva caendesilva force-pushed the simplify-public-navigation-apis branch from 3979fa8 to 1b35793 Compare March 26, 2024 17:07
@caendesilva caendesilva changed the title [2.x] Simplify public navigation APIs [2.x] Simplify and normalize public navigation APIs Mar 26, 2024
@caendesilva caendesilva force-pushed the simplify-public-navigation-apis branch from e00c3cf to b3c87c2 Compare March 26, 2024 17:16
To normalize with the navigation menu types
@caendesilva
Copy link
Member Author

caendesilva commented Mar 27, 2024

Consider reopening #1630 but rebased to target this branch

@caendesilva caendesilva force-pushed the simplify-public-navigation-apis branch from a23d266 to bfa64bf Compare March 27, 2024 19: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.
@caendesilva caendesilva force-pushed the simplify-public-navigation-apis branch from ca0baad to 8b2daf0 Compare March 28, 2024 13:47
@caendesilva caendesilva marked this pull request as ready for review March 29, 2024 18:23
@caendesilva caendesilva merged commit c681406 into improved-navigation-internals Mar 29, 2024
6 of 7 checks passed
@caendesilva caendesilva deleted the simplify-public-navigation-apis branch March 29, 2024 18:24
@caendesilva caendesilva mentioned this pull request Jun 27, 2024
74 tasks
@caendesilva caendesilva 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.

2 participants