Convert templates/ and nav/menu/footer family to logical properties - #1147
Draft
stephaniehobson wants to merge 1 commit into
Draft
stephaniehobson wants to merge 1 commit into
stephaniehobson wants to merge 1 commit into
Conversation
stephaniehobson
force-pushed
the
v23/logical-templates
branch
from
September 11, 2026 20:29
c3f5837 to
a1ab063
Compare
stephaniehobson
added this pull request to stack #1149
September 11, 2026 20:33
…1084) Second pass of the logical-properties migration -- the templates/ and navigation/menu/footer component family, all built against current main (post mixed-decls merge), not the older WS-4 branch state. 59 of the 110 remaining @include bidi() calls removed: templates/_card-layout.scss (15 -> 0) _navigation.scss (11 -> 0) _footer.scss (10 -> 0) _menu-item.scss (6 -> 0) _menu.scss (5 -> 0) _menu-list.scss (4 -> 0) _sidebar-menu.scss (4 -> 0) templates/_main-with-sidebar.scss (4 -> 0) All eight files are now fully bidi()-free. Notable non-mechanical cases: - _navigation.scss / _menu-list.scss: several bidi() calls used the 3-value "same property, different value per direction" form in pairs (e.g. two separate (padding-left, X, 0) / (padding-right, 0, X) tuples) rather than one 4-value tuple. Same underlying swap pattern, just spelled differently -- traced each pair through by hand to confirm which logical property they resolve to. - _sidebar-menu.scss: one bidi() tuple paired a margin swap with a `transform: none / translateY(3px) rotate(180deg)` pair, flipping a ▸ triangle glyph to point the other way in RTL. transform has no logical/direction-aware equivalent (it's always in the element's own coordinate space), so that one stays an explicit [dir='rtl'] override -- only the margin half converted to margin-inline-start. - _navigation.scss: two `background-position` bidi() calls stay physical with [dir='rtl'] overrides (same policy as the select rule in E1 -- no safe logical keyword syntax for background-position across this browser matrix). One of the two also had a bidi() tuple that was identical in both directions (dead weight, a pure no-op) -- collapsed to a single plain declaration. - templates/_card-layout.scss, _menu.scss, _menu-item.scss: also converted several bare (non-bidi-wrapped) margin-left/margin-right:0 pairs to margin-inline: 0 -- these were plain symmetric physical declarations sitting right next to the bidi() calls, in scope for the same inline-axis cleanup even though they weren't wrapped in the mixin. Two real bugs found via a reported visual regression (menu-list component rendering padding: 0 24px 0 4px locally vs. production's padding: 0 24px 0 0) and fixed here, both the same class as the label.mzp-u-inline bug found in the E1 commit -- a full 4-value padding/margin shorthand implicitly zeroes every side it doesn't otherwise set, and replacing it with a single logical longhand drops that protection for the sides not touched: - _menu-list.scss's `.is-details .mzp-c-menu-list-title button`: the original bidi() call's LTR value was the full shorthand "0 (16px + $spacing-sm) 0 0", explicitly zeroing padding-top/bottom/ left. My conversion had only set padding-inline-end, so the button's padding-block and padding-inline-start fell through to the browser's UA default <button> padding (non-zero on every browser) instead of the intended 0. Fixed with padding-block: 0; padding-inline: 0 X. - _footer.scss's `.mzp-c-footer-section:first-child`/`:last-child` at $mq-lg: the parent rule sets an explicit padding: 0 (X); shorthand (both inline sides non-zero), and :first-child/:last-child each need to zero out *one specific side* against that parent value -- not restate the other side, which is what I'd written (a copy-paste-shaped mistake: I pattern-matched this diff's shape against a different, unrelated footer case that had no competing parent padding, and used the same-looking conversion without re-deriving it against this rule's actual cascade). Fixed to padding-inline-start: 0 / padding-inline-end: 0 respectively. Given the severity, went back and systematically re-audited every other bare "padding"/"margin"-shorthand-origin bidi() conversion across both this commit and the E1 commit against the *actual* parent-cascaded baseline (not just diff-shape pattern matching) -- checked ~20 selectors individually against true-original compiled output. Everything else checked out: card-layout's, navigation's, and menu-item's single-side overrides all rely on an explicit sibling declaration (an earlier-cascading media query, or a base rule in the same file) that already supplies the correct value for the side not touched -- confirmed byte-for-byte via the same before/after compile diff technique used throughout, this time checked per-selector rather than by diff shape. Also confirmed the already-merged mixed-decls PR doesn't have this bug class: it only did float/text-align keyword swaps (no "sides" to drop), never converted a padding/margin shorthand. Branched directly off current main rather than the stale WS-4 chain (D1/D2/E1 sit on pre-mixed-decls-merge main via the WS-3 tip) -- avoided converting bidi() calls against a structure that would need re-reconciling again once that chain rebases. This PR has no file overlap with D1/D2/E1's own scope. Part of #1084. Verified: npm run lint, npm test (47 specs, Firefox + Chrome), npm run build-docs (523 items, no errors), npm run build-package, and the exhaustive before/after compile diff described above all pass.
stephaniehobson
force-pushed
the
v23/logical-templates
branch
from
September 14, 2026 23:08
a1ab063 to
c357f50
Compare
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Description
Second pass of the logical-properties migration.
templates/_card-layout.scss (15 -> 0)
_navigation.scss (11 -> 0)
_footer.scss (10 -> 0)
_menu-item.scss (6 -> 0)
_menu.scss (5 -> 0)
_menu-list.scss (4 -> 0)
_sidebar-menu.scss (4 -> 0)
templates/_main-with-sidebar.scss (4 -> 0)
I have documented this change in the design system.CHANGELOG.md.Issue
Part of #1084.
Testing
Enter helpful notes for whoever code reviews this change.