fix(ADBLABS-210: top of page header spacing - #123
luisSilvaEs wants to merge 4 commits into
Conversation
…t spacing below sticky header on Research, Workflows, Sneaks, and Policy pages
… so the header no longer renders taller than its sticky wrapper
|
Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed.
|
| box-sizing: border-box; | ||
| min-inline-size: 2.75rem; | ||
| min-block-size: 2.75rem; | ||
| margin-block: -0.125rem; |
There was a problem hiding this comment.
It would be a good idea to avoid having to add a negative margin.
Could we adjust the height and padding of these elements so they are the correct height and a negative margin is not needed? Inspecting a bit, it seems like the issue is that the header__bar has too much block padding, so the 44x44 header__brand doesn't fit in it. I think we can just reduce the block padding so it fits; it looks like things are already vertically centered within it.
There was a problem hiding this comment.
Good catch, totally missed that. Fixed.
| /* page-header leads the page on index templates (Research, Workflows, Sneaks, | ||
| Policy); give it breathing room below the sticky header on mobile. */ | ||
| @media (width < 48rem) { | ||
| main > .section.page-header-container:first-of-type { | ||
| padding-block-start: var(--s2a-spacing-md, 1rem); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
What about if the page does not start with the page header block? Can we handle this space generally for the top of all non-article pages? That way content editors are less limited in how they set up a page.
Examples where I would also expect to see space:
There was a problem hiding this comment.
Good call — my original thinking was that we'd steer authors toward starting pages with page-header (or document it as a content guideline) rather than handle it in CSS, so I scoped the fix narrowly. But a generic solution is more robust and doesn't rely on authors remembering a convention, so I've updated the rule to target any first section that doesn't already manage its own top spacing (rounded/colored sections and full-screen heroes are excluded since they already handle it).
Derive header__bar's mobile padding from --header-bar-height and the 44px touch target so items fit without overflow, matching the sticky wrapper's height.
Generalize the sticky-header spacing fix from page-header specifically to any first section lacking its own top padding, so pages that don't start with page-header aren't excluded.
This branch was successfully deployed
2 participants
Summary of changes
padding-block-startto thepage-headersection so Research, Workflows, Sneaks, and Policy pages get breathing room below the sticky header instead of sitting flush against itRelevant Links
Test URLs:
Checklist
Validation
Validation steps
<header>(sticky wrapper) and.header(block) report the same computed height on mobile (64px) — no overflow of.headerpast its wrapperBrowser Testing
We should aim to support the latest version of the listed browsers. For older versions or other browsers not on the list, content should be accessible, even if it doesn't completely match the designs.
Developers should test as they work in the browsers available on their machines. If they have access to other devices to test other browser/OS combinations, they should do that when possible.
Blocks and pages should undergo comprehensive testing to ensure they work as expected in real-use scenarios. Standard testing during pre-production should include at least these browsers.
Dark mode and light mode
Most pages support both dark and light mode, based on
prefers-color-scheme.Changes that affect the frontend should support both color schemes.