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.

Sign up for GitHub

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

Jump to bottom

Hides Global Header on main #511

Merged
merged 8 commits into from
Oct 13, 2021
Merged

Hides Global Header on main #511

merged 8 commits into from
Oct 13, 2021

Conversation

lomky
Copy link
Contributor

@lomky lomky commented Oct 6, 2021 •
edited
Loading

Storybook asserting no changes

===

Bumps #510

  • Global Header Icons properly highlight on hover
  • Bugfix - Global Header link post context root change
  • wontfix: our spacing doesn't quite match due to our centering. Approved by Nikki.
  • hides all of this on main!
  • Manual Browser Testing performed (with modheader, either locally or on BrowserStack)
  • Changes are tested on Staging post-merge into main

@lomky lomky self-assigned this Oct 6, 2021
@lomky lomky requested review from rocketnova and nikkibrand October 7, 2021 20:12
@lomky lomky marked this pull request as ready for review October 7, 2021 20:15
nikkibrand
nikkibrand approved these changes Oct 8, 2021
View reviewed changes
Copy link

@nikkibrand nikkibrand left a comment

Choose a reason for hiding this comment

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

This looks good, thanks Kat!

@lomky lomky changed the title Global Header Icon Hover Hides Global Header on main Oct 12, 2021
rocketnova
rocketnova approved these changes Oct 12, 2021
View reviewed changes
Copy link
Contributor

@rocketnova rocketnova left a comment

Choose a reason for hiding this comment

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

LGTM!

@lomky lomky merged commit 62bfe42 into main Oct 13, 2021
@lomky lomky deleted the kat/510_global_header_tuning branch October 13, 2021 14:16
@lomky lomky mentioned this pull request Oct 22, 2021
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nikkibrand nikkibrand nikkibrand approved these changes

@rocketnova rocketnova rocketnova approved these changes

Assignees

@lomky lomky

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@lomky @rocketnova @nikkibrand