Components: fix AdminPage SCSS module header selectors for admin-ui 2.1#49018
Conversation
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! |
Replace the three `> header` selectors in the AdminPage SCSS module with `> :first-child` so they keep matching after @wordpress/admin-ui 2.1 changed the page header element from <header> to <div>. Together with #49006 (which applied the same fix to the shared admin-page-layout mixin in js-packages/base-styles), this resolves the publicize page header spacing regression that surfaced on PR #48404 — the title/subtitle ' compaction' was the cumulative effect of the three jetpack-side overrides (sticky-disable, border removal, visual-slot centering) silently ceasing to fire, not a missing gap. Also bumps @wordpress/admin-ui from 2.0.0 to 2.1.0 in this package as the consumer-side pin of the version this fix targets.
0dc21dc to
add52fe
Compare
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
Code Coverage SummaryThis PR did not change code coverage! That could be good or bad, depending on the situation. Everything covered before, and still is? Great! Nothing was covered before? Not so great. 🤷 |
…ader The dashboard test was scoping the Jetpack Logo locator via `page.locator( 'header' )` — coupling to admin-ui 2.0's <header> element. admin-ui 2.1 renders the page header as a <div> (WordPress/gutenberg#78001), so the locator finds nothing once this PR's admin-ui pin lands. Swap the locator to `.jp-admin-page__page > :first-child` — same positional pattern this PR uses in the SCSS module, matches both the old <header> and the new <div>. Updates the surrounding comment to explain the choice.
…9101) The consistent-header-height rule from #49080 still selected the page header via `> header`, but admin-ui 2.1 (#49018) renders it as a `<div>`, so the selector matched nothing and the tab strip began shifting again on admin-ui 2.1 consumers (Newsletter). Switch to `> :first-child`, matching the sibling header-normalization rules — it targets the header positionally and works for both admin-ui 2.0's `<header>` and 2.1's `<div>`. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sibling fix to #49006.
Proposed changes
Adapt the AdminPage SCSS module in
js-packages/componentsto@wordpress/admin-ui2.1's DOM change. admin-ui 2.1 removedrender={ <header /> }from the Page component's outer Stack (Gutenberg PR #78001 — "Fix nested landmark in header"), so the rendered page header is now a<div>instead of a<header>. The three jetpack-side overrides in this module all targeted the<header>element by tag and silently stopped matching on 2.1, restoring admin-ui's defaults beneath us.:global(.jp-admin-page__page > header)selectors with> :first-child.:first-childmatches the same element<header>did in 2.0 and the new<div>in 2.1 — backward + forward compatible. The selectors fix the three regressions:.without-bottom-bordermode (used by AdminPage when tabs are present) no longer hides admin-ui's hairline border under the header.position: relative; z-index: 1;(JETPACK-1386, disables admin-ui's sticky header on Jetpack admin pages) no longer applies — sticky re-engages globally.[aria-hidden="true"]centering rule no longer applies — the JetpackLogo pins to top-left of the 24px grid cell.@wordpress/admin-uifrom2.0.0to2.1.0injs-packages/components'package.json— consumer-side pin for the version this fix targets (same convention used in Base styles: Fix admin-page-layout header selector for admin-ui 2.1 #49006 and Newsletter: Drop dead admin-ui build-style import; pin admin-ui 2.1.0 #49007).Together with #49006 (the sibling fix in the
js-packages/base-stylesadmin-page-layout mixin), this resolves the publicize page header spacing regression that surfaced during local testing of #48404 (Renovate's monorepo-wide@wordpress/*bump). On a connected dev install with admin-ui 2.1 + the bumped consumers, the Publicize dashboard's title and subtitle were rendering "compacted" — that turned out to be the cumulative visual effect of the three overrides above ceasing to fire, not a missing gap in admin-ui's CSS. Verified empirically: applying the swap restores Publicize to its expected appearance.Why this dashboard was the visual canary: publicize's "Publish once. Share everywhere." subtitle is the shortest in the codebase at 33 characters (single line). Long subtitles (Newsletter, Scan, Search, VideoPress, Protect — all 45–71 chars) wrap and visually mask the symptom; publicize doesn't.
Related product discussion/links
js-packages/components/ADMIN_UI_BUMP_CHECKLIST.mdon the parent task's branch — to be split into its own PR) — context for why this class of regression keeps catching us out.Does this pull request change what data or activity we track or use?
No.
Testing instructions
@wordpress/admin-ui@2.1.0is the resolved version. Without 2.1 in the tree, this PR is a no-op visually — the rules still match<header>via:first-child./wp-admin/admin.php?page=jetpack-publicize— the Publicize page header. Confirm:.without-bottom-borderis firing).