Give the definition-container header a whole-pixel min-height - #6078
Open
backspace wants to merge 1 commit into
Open
Give the definition-container header a whole-pixel min-height#6078backspace wants to merge 1 commit into
backspace wants to merge 1 commit into
Conversation
`1.56rem` is 24.96px at the default root size. A fractional height leaves each engine to round it however it likes, so the same content can land a pixel apart between browsers — and everything below it shifts with the header. The value reads as a conversion that lost precision: 25 / 16 is 1.5625, and 1.56 is that rounded to two places. `1.5625rem` is 25px exactly, which is what the surrounding declarations use — the button min-height two rules down is `1.5rem`, a whole 24px. This is a candidate fix rather than a confirmed one. An inspector-sidebar snapshot has been reporting a Firefox-only diff at a `diff-ratio` stable to six figures across builds and across branches, which is the signature of one region shifting by a fixed amount rather than of anything either branch changed. A fractional header height in that sidebar explains it, and only six rem values in the whole component tree fail to land on a whole pixel. Whether it is the cause is answered by the diff either recurring or stopping. The same declaration is duplicated in boxel-ui's header usage page, which is where it would be copied from next. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Contributor
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.
This is meant to address this pattern of Percy diffs, which only shows in Firefox, a tiny shift in the sidebare:
Claude:
--boxel-header-min-height: 1.56remis 24.96px at the default root size. A fractional height leaves each engine to round it however it likes, so the same content can land a pixel apart between browsers, and everything below the header shifts with it.The value reads as a conversion that lost precision: 25 ÷ 16 is 1.5625, and
1.56is that rounded to two places.1.5625remis 25px exactly. The surrounding declarations already sit on whole pixels — the button min-height two rules down is1.5rem, a clean 24px.Why this is worth changing
An inspector-sidebar snapshot has been reporting a Firefox-only diff, one comparison out of 118, at a
diff-ratioof0.00091737— stable to six figures across builds and across two unrelated branches. That is the signature of a single region shifting by a fixed amount, not of anything either branch changed, and not of random rounding noise.A fractional header height in that sidebar accounts for it. Of every
remvalue in the host component tree, only six fail to land on a whole pixel, and this is the one inside the affected component.This is a candidate, not a confirmed cause
I have not seen the rendered images, so the mechanism is inferred from the value and the diff's shape rather than observed. The test is whether the recurring diff stops. If it does not, the next thing to look at is what else in that sidebar computes to a fraction.
Expect a small one-time diff from this change itself: the header moves from 24.96px to 25px, so whichever engine was rounding down now agrees with the other.
The same declaration is duplicated in boxel-ui's header usage page. It is changed too, since that is where it would be copied from next.