[2.x] fix: don't stack multiple phone header controls on top of each other - #4870
Merged
Conversation
App-primaryControl is an extension point, so a page can carry several header controls — core's 'start discussion' button plus, say, an extension's follow button on a tag page. On phones each one is positioned absolutely against the right edge, so they rendered on top of one another as a single smudged glyph. Where the controls share a parent — they are list items of the same nav — that parent now becomes a right-anchored flex row and the controls return to relative positioning, letting the browser lay them out side by side at whatever width each one is. Selected with :has() so it only applies when a second control is actually present; pages with a single control keep their existing geometry exactly. Measured at 390px: the tag page's two controls went from both occupying 339-390 to 288-339 and 339-390; index, discussion and user pages are byte-identical.
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
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.
Changes proposed in this pull request
App-primaryControlis a documented extension point, so a page can carry more than one header control — core's "start discussion" button plus, for example, an extension's follow button on a tag page. On phones every control is positionedabsolute; right: 0, which assumes a single occupant: with two present they render in exactly the same 51×46 box, appearing as one smudged glyph.Where the controls share a parent (they are list items of the same nav), that parent now becomes a right-anchored flex row and the controls return to relative positioning, so the browser places them side by side at whatever width each one is — no hard-coded control width to drift out of sync with the button padding.
The rule is selected with
:has(> .App-primaryControl ~ .App-primaryControl), so it engages only when a second control actually exists; single-control pages keep their existing geometry by construction.:has()is already used in core (forum/PostStream.less), so this breaks no new ground on browser support.Measured at 390px, authenticated
x=339 → 390— overlapping288 → 339and339 → 390339 → 390350 → 390Reviewers should focus on
App-primaryControlitems in a container where making the parent a flex row could have side effects. I checked index/discussion/tag/user at phone width and the single-control pages are unaffected, but extension-heavy forums may have combinations I can't see..App-primaryControlkeep working as-is), and no overflow strategy — with three or more controls at 390px the title will eventually collide, which is a separate design question rather than something to smuggle in here.On testing
This is pure CSS geometry, and I could not write a suite test that fails first for it: a LESS-compiling PHPUnit test can only assert that declarations exist (implementation-coupled, and blind to actual overlap), and jsdom has no layout engine, so
getBoundingClientRect()returns zeros. The red/green gate was a headless-Chrome probe measuring the real boxes on a live forum — overlap detected before, absent after, other pages unchanged (the table above). Core having no browser-based visual testing is the underlying gap here; worth its own discussion.Confirmed