10up / Engineering-Best-Practices

10up Engineering Best Practices
https://10up.github.io/Engineering-Best-Practices/
MIT License
760 stars 205 forks source link

Fix/348 centering main nav #349

Closed ciprianimike closed 3 years ago

ciprianimike commented 4 years ago

Description of the Change

The problem occurred on tablets and desktops, I update the header-menu SCSS, the menu is now centered for all the breakpoints:

Verification Process

Breakpoints tested with Chrome Dev Tool, iOS simulator, Android Emulator and iMac 27" and iPhone 11 Pro as physical devices.

Checklist:

jeffpaul commented 4 years ago

@tlovett1 you 👍🏼 for this to get merged in?

joesnellpdx commented 3 years ago

@tylercherpak @ciprianimike @dmtrmrv

I made some changes to the styles here: https://github.com/10up/Engineering-Best-Practices/pull/349/commits/343aefac26fd741014be7021aa64e13ebe8763fc

I'm not sure if the content changed while this was waiting to be merged, but found issues with the layout (particularly after the nav break and up to 900px viewport widths).

If I can get your approval on the current state, I'll merge it in.

This change will make the other MR here mute: https://github.com/10up/Engineering-Best-Practices/pull/352

https://drive.google.com/file/d/1dWK5vcqpe7ujYyQ4bXTvZkqhKgWxZCRa/view

CC: @jeffpaul

joesnellpdx commented 3 years ago

@tylercherpak One edge case bug found by @dmtrmrv I need to investigate and provide a solution before we continue.

joesnellpdx commented 3 years ago

@dmtrmrv Fixed issue where is-open style caused layout issues on mid/large screens. Please review

Pull latest to see changes

dmtrmrv commented 3 years ago

@joesnellpdx I tested the recent fix and it looks good to me. Layout stays consistent when user resizes the window.