fix(banner): derive placement from topbar height #1408
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ Deploy Preview for stacks ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
dancormier
changed the title
Now, it's placed inside `&[aria-hidden="false"]` to prevent cascade conflicts
gtfo custom docs junk
That border change shifted all banner test images 1px
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes a bug where the system-wide banners sit a few pixels higher than they should on Stack Overflow
@dancormier here 👋
This PR updates
s-bannerto base it's placement off of the--theme-topbar-heightvariable. Previously, it was set to a static value with no relation to the topbar height which could cause the banner to show in the incorrect spot.The issue went unnoticed because the Stacks docs topbar height was set to a custom height using an atomic class (
.h64) on the topbar itself and the demo banners were placed with custom CSS on the banners documentation page. This PR removes all that custom junk. Now, the docs to bases it's topbar height off of--theme-topbar-heightand removes the page-specific styling.Outside of placement, the only visual change is to the banner border. I've removed the top border on the banner since it was never seen… we have a positioning offset that moved the banner up 1px so the border would be overlapped by the topbar. That compensating 1px is now gone and the banner now only has a bottom border.
I've requested a review from @giamir mainly as a heads-up. An exhaustive review shouldn't be necessary.