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

Underline active links in Backend menu (Next only) #3452

Closed
ADDISON74 opened this issue Aug 19, 2023 · 10 comments · Fixed by #4246
Closed

Underline active links in Backend menu (Next only) #3452

ADDISON74 opened this issue Aug 19, 2023 · 10 comments · Fixed by #4246

Comments

@ADDISON74
Copy link
Contributor

Please note this issue is only in the NEXT branch. I did not check when the change occurred in /skin/adminhtml/default/default/menu.css line 64.

Steps to reproduce it

1 - Go to Backend.

2 - From the menu select any section, for example Catalog > Manage Products.

3 - After the page loads, go to the menu again. The active page link is underlined. See the image bellow.

menu_underline

@ADDISON74 ADDISON74 added the bug label Aug 19, 2023
@fballiano
Copy link
Contributor

it's coming from #2737 and I like it this way actually

@ADDISON74
Copy link
Contributor Author

It doesn't look good at all.

Underlining the active link has not been done for a long time. I consider that it is enough for the menu header to get a different color (orange), but no distinguishing elements are necessary for the submenu. I am sorry I approved that PR I missed the underline part.

@fballiano
Copy link
Contributor

I don't think it can be considered a bug anyway, it's a preference that we can change in case

@ADDISON74
Copy link
Contributor Author

ADDISON74 commented Aug 19, 2023

It's a visual bug, it looks very bad and unprofessional. I immediately noticed it after seeing this interface for 15 years. It must be fixed, that underlining of the active link must be removed from both in the legacy theme and in openmage.

If you still like to highlight the active link of the menu, then a better option would be not to underline the link but to give it the hover background. Please make the change in the Inspector and you will see that it looks much better.

#nav ul li a.active { /*! text-decoration:underline; */ background-color: #d0dfe2;}

highlight

@ADDISON74
Copy link
Contributor Author

This behavior introduced in PR #2737 must be removed at once. Even if we replace the underlining of active links with a colored background in the menu, there are issues I did not know before. Not all active links are underlined. Visit CMS > Widgets and after the page loads go to the menu, Widgets is not underlined. It is not the only case, visit the sections of the Reports, some are underlined, others are not. Problems in submenus. If you take every section accessible from the menu there are around 35% active link having this issue.

@fballiano
Copy link
Contributor

the "widgets" bug is for sure due to the widgets mask, not the PR itself

@fballiano
Copy link
Contributor

I think that having "something" to show that the menu is "active" is a good thing, maybe the underlining could be not the best? that's debatable I think, but to have an active state I think it's a good thing.

@sreichel
Copy link
Contributor

sreichel commented Oct 3, 2024

@ADDISON74 any suggestions to make it look better - w/o underline. Different background color e.g.?

@ADDISON74
Copy link
Contributor Author

This one is related to the legacy theme. The change should be

#nav ul li a.active { /*! text-decoration:underline; */ background-color: #d0dfe2;}

Any active link will be highlighted in the menu, being easy to know which one is. Underlining is not a suitable visual solution in my opinion, valid for both Backend themes.

@sreichel
Copy link
Contributor

sreichel commented Oct 3, 2024

Thanks. Did not see it in your former comment. PR comes later.

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 a pull request may close this issue.

3 participants