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] Improved navigation group label generation #1576

Conversation

caendesilva
Copy link
Member

@caendesilva caendesilva commented Feb 20, 2024

This targets v2.x via #1568 and picks up the idea behind #1575

This PR makes improvements to how navigation group labels are generated

This PR makes improvements to how navigation item labels are generated. The biggest change which could be breaking is that all labels are now formatted. So if you create a navigation item with the label "foo" it will now be normalized to "Foo". This may be opinionated, but considering that we do the same for page titles, why should this be different?

Copy link

codecov bot commented Feb 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (d3a0a59) 100.00% compared to head (8cc5bd6) 100.00%.

Additional details and impacted files
@@                        Coverage Diff                        @@
##             improved-navigation-internals     #1576   +/-   ##
=================================================================
  Coverage                           100.00%   100.00%           
- Complexity                            1762      1763    +1     
=================================================================
  Files                                  184       184           
  Lines                                 4776      4772    -4     
=================================================================
- Hits                                  4776      4772    -4     

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

We don't have all the context at this point
Can allow us to have two states, one pretty for labels, and one normalized internal one
@caendesilva caendesilva force-pushed the improved-navigation-group-label-generation branch from 6af6764 to 7fd874e Compare February 20, 2024 13:51
@caendesilva caendesilva force-pushed the improved-navigation-group-label-generation branch from 9c36bbc to 9ab9700 Compare February 20, 2024 14:22
@caendesilva caendesilva changed the title [2.x] Improved navigation group label generation [2.x] Improved navigation item label generation Feb 20, 2024
@caendesilva caendesilva force-pushed the improved-navigation-group-label-generation branch from 385445f to bb46d13 Compare February 20, 2024 14:33
@caendesilva caendesilva added the WTD Calls What The Diff label Feb 20, 2024
Copy link

what-the-diff bot commented Feb 20, 2024

PR Summary

  • Menu Item Grouping Implemented in Sidebar
    Updates have been made to the 'generate' method in GeneratesDocumentationSidebarMenu.php to better manage how navigation items are grouped together. This allows for a more organized display of items in the sidebar menu.

  • Normalized Labels for Navigation Items
    Changes to NavItem.php include methods to normalize labels, find ideal group labels, and return group labels for items with subitems. This ensures consistency and clarity in how navigation items are presented.

  • Removal of Redundant Method
    The normalizeGroupLabel method has been removed, as normalizeLabel now handles all necessary label formatting.

  • Improved and Expanded Test Cases
    Test cases in both AutomaticNavigationConfigurationsTest.php and NavItemTest.php have been updated for navigation items in different contexts to ensure accuracy of labels and their grouping. New tests have also been added to check sidebar label formatting and label-setting in the configuration. This ensures our system handles various scenarios correctly.

@caendesilva
Copy link
Member Author

caendesilva commented Feb 20, 2024

It may violate the principle of least astonishment to always format the label. If the user manually creates a NavItem as new NavItem(label: 'foo'), the label should return foo. We only really want to change titles for autodiscovered items. And since the main navigation menu does this automatically for NavItems corresponding to pages, we should only do this for the sidebar group labels as it's just there we don't have a direct way of editing the front matter.

Edit: I think consistency wins over here.

@caendesilva caendesilva changed the title [2.x] Improved navigation item label generation [2.x] Improved navigation group label generation Feb 20, 2024
@caendesilva caendesilva force-pushed the improved-navigation-group-label-generation branch from 648d3cd to bb46d13 Compare February 20, 2024 14:56
This reverts commit b7e8588 as a group without a title indicates an issue that should be thrown.
There is no reason to expect things like "index" to be a dropdown group. Posts maybe, if for some reason a user wants to list all posts in a dropdown but at that point they need to make other changes so I see no reason to have an extra line for something so uncommon.
@caendesilva caendesilva force-pushed the improved-navigation-group-label-generation branch from 65d87e1 to 0e55c2b Compare February 20, 2024 15:38
@caendesilva caendesilva marked this pull request as ready for review February 20, 2024 16:41
@caendesilva caendesilva merged commit a8bc3ff into improved-navigation-internals Feb 20, 2024
7 checks passed
@caendesilva caendesilva deleted the improved-navigation-group-label-generation branch February 20, 2024 16:41
@caendesilva caendesilva mentioned this pull request Jun 27, 2024
74 tasks
@caendesilva caendesilva removed the WTD Calls What The Diff label Jun 30, 2024
@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.

1 participant