fix(notch): keep a full reserved band on a display with no camera housing - #548
Merged
Merged
Conversation
β¦sing The card's reserved band is not only a hole for the camera housing β it is also the row that holds the product name and the close button. Its height comes from `NSScreen.safeAreaInsets.top`, and a display with no housing reports that as a real `0` rather than as "no answer", so the frontend's own fallback never fires and `bandHeight: 0` travels to the card. Laying that out collapses the row to its 1pt hairline: the 20pt close button overflows it `items-center` and the shell's `overflow-hidden` slices it through its own middle. On every setup whose main display is an external monitor, the β was cut in half. A regression of reading per-machine metrics β before that the constant 32 was used everywhere and the notchless case rendered correctly. `resolveBandHeight` makes `NOTCH_BAND_HEIGHT` a floor rather than merely a default, so the measured inset can only ever make the band taller. Applied at the three places that consume `bandHeight` β `notchRowHeights`, `rewardConfettiOrigin` and `NotchCard`'s render β so the measurement that sizes the OS window and the layout that fills it cannot drift apart. The payload still carries the raw measurement; the geometry decides the layout, which keeps the clamp in one place.
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.
What changed
On a display with no camera housing β every external monitor, and so every setup whose main display is one β the notch card's close button was rendered sliced through its own vertical middle, along with the "GIT MANAGER" label beside it. It now renders whole, exactly as it does on a MacBook's own screen.
NOTCH_BAND_HEIGHTbecomes a floor rather than merely a default, via a newresolveBandHeight. The per-machine safe-area measurement can still make the reserved band taller; it can no longer make it shorter than the content the band itself holds.Why
The reserved band is not only a hole for the camera housing: it is also the row that holds the product name and the β (
NotchNotification'sbandStart/bandEnd). Its height comes fromNSScreen.safeAreaInsets.topviaget_notch_metrics, and a display with no housing reports that as a real0β notnullβ so the frontend's own "nothing to go on" fallback never fires andbandHeight: 0travels to the card in its URL.Laying
0out does not reserve less space, it collapses the row to its 1pt hairline. The 20pt close button then overflows a 1pt flex rowitems-center, extending ~9.5pt above the card's top edge, where the shell'soverflow-hiddencuts it in half. That is precisely the reported symptom.It is a regression of reading per-machine metrics: before that, the hard-coded 32 was used everywhere and the notchless case rendered correctly β which is also why the clamp restores known-good behaviour rather than inventing one.
Notes for the reviewer
??fallback at the source. MakingnotchWindow.tssendundefinedwhen the metric is0would fix the symptom but lose the distinction between "measured, and it is zero" and "could not measure" β and it would leaveNotchCardstill able to render a0any other caller passed. The floor lives in the geometry, which is the layer both the measurement and the render already go through.resolveBandHeightis applied at the only three places that consumebandHeightβnotchRowHeights(which sizes the OS window),rewardConfettiOrigin, andNotchCard's own render β so the window height and the content that fills it cannot drift apart, which is the failure mode this whole module is built to avoid.docs/file, per the repo convention for invariants someone could plausibly break later.notchMetrics.tscaches the metrics for the life of the process, reasoning that "a display cannot grow or lose its camera housing without the machine restarting" β docking and changing the main display does exactly that at runtime. With the floor in place the dangerous direction is neutralised (0β 32); what remains is a cosmetic few points on a machine reserving more than 32pt. Worth its own change, not this one.pnpm devwith an external display set as the main one. ThebandHeight={0}render test pins the exact geometry that was broken.Verification
pnpm typecheckpnpm lint(75 warnings in@git-manager/desktop, all pre-existing; 0 errors)pnpm --filter @git-manager/desktop testβ 762 files, 8313 tests, all passingpnpm --filter @git-manager/notch testβ 211 tests, all passingcargo testβ no Rust module touchedIf this PR touchesβ¦
resolveBandHeightis covered innotchGeometry.test.ts, and the render it fixes is pinned by a newbandHeight={0}case inNotchCard.test.tsxresolveBandHeight's doc comment, beside the code that enforces it