fix(main menu): contrast, icons, responsiveness of main menu

mndonx commented 1 year ago

Summary

This PR fixes/implements the following bugs/features See: https://emulsify-ds.github.io/compound/?path=/story/molecules-menus--main

Main menu contrast in light & dark mode does not past testing
Mobile menu icon is broken
Close icon should match open icon (uses a character instead of an SVG)
Main menu in mobile has weird padding and margins
Mobile main menu should align right (to allow for logo)
Main menu is not responsive (can't go from small with open items to large and have the menu work)

Logo is not displaying at small widths
Social icons not displaying

I starting fixing stuff because the bare minimum of contrast testing was not met ... but then I found some other stuff. However, the Main Menu is currently not accessible (the second level) so I think really more work will need to be put in here. I think this is just a stop-gap.

Documentation update (required)

If this pull request requires a change to Emulsify documentation, those changes, updates, and/or new information must accompany this pull request.

How to review this pull request

[ ] After recompiling, check the Main Menu component (also check the Site Header and Article content type (and full Pages)
[ ] Make sure the contrast is good
[ ] Make it small to check the mobile version. Make sure that looks good (especially on Content Type and Page versions)
[ ] Make sure all images are loading properly in Main Menu and Social Menu (as well as the Logo at small widths.)
[ ] Make sure when going from small (with the menu open) to large, the menu continues to work

Closing issues

Closes #81

mndonx commented 1 year ago

@ryanhagerty Thank you for the review! I've adjusted the order of the CSS throughout the component and changed some ems to rems. I also swapped the :focus to :focus-within so people tabbing through the main menu in desktop can actually get to the second level. Not a perfect a11y solution, but better than what it is now!

mndonx commented 1 year ago

@ryanhagerty I totally agree, except I'm fairly sure this component is going to be completely rewritten for the UI Kit (using a very different method) and back-ported here, so I probably already spent too much time on it already -- I think it's just making sure it isn't completely broken right now. Maybe we should create an issue for fixing inconsistencies in rem/$space/px throughout the project, though? I'll do that, and set this one ready to merge.

mcortes19 commented 1 year ago

@mndonx Could you please review/fix conflicts, thanks for all you effort 💪🏼

mndonx commented 1 year ago

@mcortes19 I fixed the conflicts (sorry about the delay!) so I think it is ready for merge. I'm not sure why the checks do not complete -- but it seems like that is the case on other PRs?

github-actions[bot] commented 1 year ago

:tada: This PR is included in version 1.15.2 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket:

emulsify-ds / compound