Slim down the height of the announcement bar on desktop #2807
Conversation
|
Looks great so far! I am testing out certain scenarios and I noticed that the slider doesnt align with the drawer menu and shop cart icon in mobile. I know your PR mentions that "The PR does NOT make any adjustments to the mobile state of the announcement bar", but I wanted to bring up a point that the alignment does look off on mobile |
|
Thanks, @lougoncharenko! That should be fixed in 310b916. |
| @@ -2266,6 +2276,7 @@ product-info .loading-overlay:not(.hidden) ~ *, | |||
| padding: 1rem 0; | |||
| margin: 0; | |||
| letter-spacing: 0.1rem; | |||
| min-height: 3.8rem; | |||
There was a problem hiding this comment.
If you're ok with this, I probably am, but historically we do make a best effort to keep things 44px. Just want to double check we don't want to add in the additional 6px. In theory a tablet touch device could display at the desktop breakpoint and still benefit from the 44px min.
There was a problem hiding this comment.
Yeah, I'm ok with 38px for desktop. 👍
| } | ||
|
|
||
| .js .header-localization:not(.menu-drawer__localization) .localization-form__select { | ||
| padding: 0 2.7rem 0 1.2rem; | ||
| padding: 0.85rem 2.7rem 0.85rem 1.2rem; |
There was a problem hiding this comment.
Not a big fan of the kinda magic numbers. I'd rather set the desired height of 38px more transparently and align things with flex box, but if that would require additional refactoring to accommodate than this works for now.
There was a problem hiding this comment.
Deeeeefinitely. I don't know why I bothered trying to figure out these weird rem values. 😂 Fixed in dff48f2.
|
Full screen mode is adding extra margins to both the announcement bar and header This PR: |
|
@lougoncharenko I think that's expected! I'm not seeing your screenshotted "Current in Dawn" behavior in my testing, but if you check the Dawn Demo here, you'll see that the header is meant to max out at the page width. |
|
@lougoncharenko I think you're probably seeing the difference when the desktop drawer menu is used, where we allow everything to go more full width. If using the dropdown or megamenu types everything should be restricted to page width. |
|
Ah yes, that's got to be it. I just checked, and the announcement bar respects that setting too — when the menu is set to Drawer, the announcement bar goes full width alongside the rest of the header. And when it's set to Dropdown or Mega Menu, it continues mirroring the header by maxing out at the page width. 👍 I think we're good in that case. Thanks everyone for the quick testing of this today! |
* Adjust height of announcement bar and sub-items. * Adjust announcement bar button height on desktop only. * Fix width of the utility bar on mobile. * Remove bizarre rem-based values.
* Adjust height of announcement bar and sub-items. * Adjust announcement bar button height on desktop only. * Fix width of the utility bar on mobile. * Remove bizarre rem-based values.
PR Summary:
Slims down the height of the announcement bar on desktop.
Why are these changes introduced?
Addresses some feedback we've received about the general size of the announcements bar. This PR:
It also fixes the following small bug:
The PR does NOT make any adjustments to the mobile state of the announcement bar.
Visual impact on existing themes
Screenshots:
New height:
Note the left/right arrows alignment:
Testing steps/scenarios
Checklist