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

Increase the z-index for the navigation #4513

Merged
merged 2 commits into from
Dec 12, 2018

Conversation

nottrobin
Copy link
Contributor

@nottrobin nottrobin commented Dec 12, 2018

Since the navigation needs to overlay practically everything, it should be
high enough to provide plenty of lower layers for other things on the page
that need to use z-index.

I'm following a vague standard, which may be enshrined in team practices
soon, that the page gets to use z-index in the range 0-50,
51-100 should be reserved for plugins (like global-nav), and then
100+ should be used to override plugins if needed.

So the nav uses 37-40, to sit near the top-end of that range, but with a little space above it.

Fixes #4442

QA

Check:

  • the nav drop-downs still work and are visible
  • the nav successfully overlays everything else on the page
  • the global nav still overlays the main nav

It was so high it was overriding all things, including the greyed
out overlay from the top menus.
Since the navigation needs to overlay practically everything,
it should be high enough to provide plenty of lower layers
for other things on the page that need to use z-index.

I'm following a vague standard, which may be enshrined in team
practices soon, that the page gets to use z-index in the range `0-50`,
`51-100` should be reserved for plugins (like global-nav), and then
`100+` should be used to override plugins if needed.
@webteam-app
Copy link

Copy link
Contributor

@anthonydillon anthonydillon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍 gaming section looks good and both navigation works well together

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants