Cleanup `MenuItem` API and deprecate using partials for second level menus #5309

elia · 2023-08-07T14:11:10Z

Summary

Remove #sections that was probably introduced in order to generate the submenu and later became just a way to check if the MenuItem was "selected" or not.

This paves the way for replacing the second level navigation partials with a list of children items.

The tab helper is also updated and uses explicit label: and match_path: options instead of the initial list of sections.

`MenuItem#position` replacement

MenuItem#position is deprecated and can be replaced by reordering menu items explicitly in the backend config:

products_menu = config.menu_items.find { _1.label == :products }
config.menu_items.insert(0, config.menu_items.delete(products_menu))

or

config.menu_items.sort_by! { %i[products orders settings promotions].index(_1.label) }

Checklist

Check out our PR guidelines for more details.

The following are mandatory for all PRs:

I have written a thorough PR description.
I have kept my commits small and atomic.
I have used clear, explanatory commit messages.

The following are not always needed:

📖 I have updated the README to account for my changes.
📑 I have documented new code with YARD.
🛣️ I have opened a PR to update the guides.
✅ I have added automated tests to cover my changes.
📸 I have attached screenshots to demo visual changes.

Block handling in ERB is different than a regular method. Regular methods are interested in the return value, while for ERB we need to capture the ERB output produced by running the block and discard the return value. The `capture` method does just that.

codecov · 2023-09-21T15:44:37Z

Codecov Report

Merging #5309 (78497dd) into main (3994370) will increase coverage by 0.05%.
The diff coverage is 98.94%.

@@            Coverage Diff             @@
##             main    #5309      +/-   ##
==========================================
+ Coverage   88.71%   88.76%   +0.05%     
==========================================
  Files         563      564       +1     
  Lines       13896    13959      +63     
==========================================
+ Hits        12328    12391      +63     
  Misses       1568     1568

Files Changed	Coverage Δ
...ckend/lib/spree/backend_configuration/menu_item.rb	`98.18% <97.56%> (-1.82%)`	⬇️
...ckend/app/helpers/spree/admin/navigation_helper.rb	`90.62% <100.00%> (+1.61%)`	⬆️
backend/lib/spree/backend_configuration.rb	`100.00% <100.00%> (ø)`
.../backend_configuration/deprecated_tab_constants.rb	`100.00% <100.00%> (ø)`

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Also moves `#icon` from a sequential argument to a keyword argument. The list of sections was only used to match the current path, because top-level menu items should be active whenever any of the children items are active.

The first part will just setup options and the bottom of the method takes care of generating the "tab".

The menu item will now be able to tell if a request matches itself.

Use MenuItem instances for second level backend menus instead of partials.

The condition option was duplicating what the children were already checking.

rainerdema

Great work @elia! I appreciate the simplicity you've introduced, especially in creating new menu items without needing partial views. Thanks a lot!

Rely on the implicit array ordering.

elia self-assigned this Aug 7, 2023

github-actions bot added the changelog:solidus_backend Changes to the solidus_backend gem label Aug 7, 2023

elia force-pushed the elia/menu-items-cleanup branch from 273f683 to 2cd1512 Compare August 8, 2023 16:42

github-actions bot added the changelog:solidus_core Changes to the solidus_core gem label Aug 8, 2023

elia force-pushed the elia/menu-items-cleanup branch 2 times, most recently from 58ed5d3 to 423029d Compare August 10, 2023 15:38

elia force-pushed the elia/menu-items-cleanup branch 5 times, most recently from ab47d60 to f6e8e70 Compare September 21, 2023 15:33

elia force-pushed the elia/menu-items-cleanup branch from f6e8e70 to 4903d35 Compare September 21, 2023 19:26

elia changed the title ~~[Backend] cleanup MenuItems api~~ [Backend] cleanup MenuItems generation Sep 21, 2023

elia mentioned this pull request Sep 21, 2023

Use menu items for subitems too #5305

Closed

3 tasks

elia added 10 commits September 21, 2023 22:43

Deprecate MenuItem#sections in the backend navigation

c9e151c

Also moves `#icon` from a sequential argument to a keyword argument. The list of sections was only used to match the current path, because top-level menu items should be active whenever any of the children items are active.

Let MenuItem expand the URL if it's a symbol

453dbb3

Allow explicitly setting the selected state for the #tab helper

8b950eb

Deprecate the :route option in the #tab helper

324bd68

Keep #tab helper HTML generation close

89d3929

The first part will just setup options and the bottom of the method takes care of generating the "tab".

Don't modify an option hash coming from outside in the #tab helper

efb4243

Add MenuItem#match_path? to backend

06bf240

The menu item will now be able to tell if a request matches itself.

Inline and deprecate Backend *_TABS constants

14a1cb3

Add MenuItem#children and #data_hook

b41415d

Use MenuItem instances for second level backend menus instead of partials.

Deduplicate menu item visibility checks

ff604f6

The condition option was duplicating what the children were already checking.

elia force-pushed the elia/menu-items-cleanup branch from 3295e54 to f8f801e Compare September 21, 2023 20:43

elia requested a review from rainerdema September 21, 2023 20:49

elia marked this pull request as ready for review September 22, 2023 07:00

elia requested a review from a team as a code owner September 22, 2023 07:00

rainerdema approved these changes Sep 22, 2023

View reviewed changes

elia force-pushed the elia/menu-items-cleanup branch from f8f801e to b79954e Compare September 25, 2023 07:55

Deprecate the use of MenuItem#position

78497dd

Rely on the implicit array ordering.

elia force-pushed the elia/menu-items-cleanup branch from b79954e to 78497dd Compare September 25, 2023 08:50

elia merged commit 1891495 into main Sep 25, 2023

elia deleted the elia/menu-items-cleanup branch September 25, 2023 15:33

elia changed the title ~~[Backend] cleanup MenuItems generation~~ Cleanup MenuItems api and deprecate partials Sep 26, 2023

elia changed the title ~~Cleanup MenuItems api and deprecate partials~~ Cleanup MenuItem API and deprecate using partials for second level menus Sep 26, 2023

kennyadsl mentioned this pull request Feb 8, 2024

Menu item should not match url if match_path is set #5643

Merged

3 tasks

pelargir mentioned this pull request May 27, 2024

Fix deprecated *_TABS constant solidusio-contrib/solidus_volume_pricing#71

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cleanup `MenuItem` API and deprecate using partials for second level menus #5309

Cleanup `MenuItem` API and deprecate using partials for second level menus #5309

elia commented Aug 7, 2023 •

edited

Loading

codecov bot commented Sep 21, 2023 •

edited

Loading

rainerdema left a comment

Cleanup MenuItem API and deprecate using partials for second level menus #5309

Cleanup MenuItem API and deprecate using partials for second level menus #5309

Conversation

elia commented Aug 7, 2023 • edited Loading

Summary

MenuItem#position replacement

Checklist

codecov bot commented Sep 21, 2023 • edited Loading

Codecov Report

rainerdema left a comment

Choose a reason for hiding this comment

Cleanup `MenuItem` API and deprecate using partials for second level menus #5309

Cleanup `MenuItem` API and deprecate using partials for second level menus #5309

elia commented Aug 7, 2023 •

edited

Loading

`MenuItem#position` replacement

codecov bot commented Sep 21, 2023 •

edited

Loading