-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Fix "Stock" admin nav double highlight #2969
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that this is a better solution. It also is extensible (if we see this issue elsewhere) and can be used by extensions as well. 👏 Great call @tvdeyen.
Without setting match_path the tab helper uses the current controller name as criteria to activate the tab in the menu. Since the stock items controller can be accessed for a single product or all products we need to make sure it only matches the /stock_items path for the Stock menu tab. Closes solidusio#2950
@jacobherrington done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Without setting
match_path
thetab
helper uses the current controller name as criteria to activate the tab in the menu. Since the stock items controller can be accessed for a single product or all products we need to make sure it only matches the/admin/stock_items
path for the "Stock" menu tab.