Skip to content

Window shadow control and adaptive window sizing - #240

Merged
superhighfives merged 4 commits into
1.9.0from
238-window-shadow-and-size
Jul 19, 2026
Merged

Window shadow control and adaptive window sizing#240
superhighfives merged 4 commits into
1.9.0from
238-window-shadow-and-size

Conversation

@superhighfives

Copy link
Copy Markdown
Owner

Closes #238.

Addresses both parts of the issue — the window shadow influencing colour readings, and the minimum window size being larger than necessary — plus a fair bit of related polish.

Window shadow

  • New Window Settings section in Preferences with a shadow dropdown: Show shadow / Hide shadow while picking / Hide shadow.
  • "Hide shadow while picking" drops the shadow only for the duration of a pick (handling the chained foreground→background pick without flicker).
  • When the window is shadowless it can blend into the desktop, so a hairline border is drawn via a borderless, click-through companion window laid over it (sized onto the visible glass edge, corner radius matched by eye since macOS masks the corners).

Adaptive sizing

  • Lowered the window minimum (~200pt tall, 360 wide) and shed UI as the window shrinks — palettes → contrast footer → preview pill → colour names → type labels — via height and width breakpoints, suppress-but-remember (saved toggles untouched).
  • Footer and palette drawer share one hide width so they appear/disappear together; compliance badge titles truncate gracefully when tight.
  • The colour value shrinks to fit two lines via proportional font scaling (AdaptiveValueText) — smooth on resize, no flicker, no truncation.
  • Floating expand-to-fit control (hover-gated, tucks top-right at short heights) that grows the window to reveal hidden-but-enabled elements.

Polish

  • Shorter footer labels ("Contrast", "Contrast (Lc)", "Body").
  • Standard window-background panels in light mode; AdaptiveDivider for consistent section seams; near-matched drawer tint; removed the divider between the two swatches.
  • Fixed the Preferences header shader vanishing when a Picker menu closed (scoped the window notifications to the view's own window).

Notes

  • New preference/UI strings are English-only with English fallbacks in the other locales — they'll need real translations before release.

🤖 Generated with Claude Code

superhighfives and others added 3 commits July 18, 2026 15:53
Window shadow:
- New WindowShadow setting (show / hide while picking / hide) applied to
  the primary window, suppressed around picks for the "while picking" mode
- Lives in a new "Window Settings" section in Preferences

Adaptive sizing:
- Lower the window min size (≈200pt tall, 360 wide) and shed UI as the
  window shrinks — palettes, contrast footer, preview pill, colour names,
  type labels — via height/width breakpoints, suppress-but-remember
- Value text auto-fits two lines within its column
- Floating "expand to fit" affordance (hover-gated) that grows the window
  to reveal hidden-but-enabled elements

Footer / drawer polish:
- Shorten footer labels (Contrast, Contrast (Lc), Body) so footer and
  palette drawer share one hide width
- Standard window background in light mode; AdaptiveDivider for consistent
  section seams; nearly-matched drawer tint
- Remove the divider between the two colour swatches

Fix header shader vanishing when a Picker menu closes by scoping the
window notifications to the view's own host window.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Shadow options read "Show shadow" / "Hide shadow while picking" /
  "Hide shadow"
- Move shadow control into a "Window Settings" section (unlabeled dropdown)
- Lower the shared footer/palette hide width to 410; compliance badge
  titles truncate ("Bo…") rather than clip when space is tight
- Footer slides without an opacity fade (always full opacity)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Draw a hairline outline via a borderless, click-through companion window
  laid over the main window whenever it has no shadow, so a shadowless
  window keeps a defined edge. Sized past the frame to sit on the visible
  glass edge; tuned stroke inset and corner radius.
- Move the windowShadow observation into WindowCoordinator so the border
  tracks the setting (and the while-picking suppression).
- Replace the value text's minimumScaleFactor with AdaptiveValueText, which
  scales the font proportionally to the measured column width. The wrap
  point stays fixed as the window resizes (no bistable line-jump flicker)
  and long values shrink to two lines instead of truncating.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@superhighfives

Copy link
Copy Markdown
Owner Author

@claude Review please.

@claude

claude Bot commented Jul 19, 2026

Copy link
Copy Markdown

Claude finished @superhighfives's task in 3m 52s —— View job


Review

I read through the full diff against origin/1.9.0 (window shadow, adaptive sizing, and polish changes across WindowCoordinator.swift, ContentView.swift, EyedropperButton.swift, Eyedroppers.swift, PikaWindow.swift, Visualisation.swift, and the footer/drawer/toggle views). I wasn't able to run xcodebuild/tests in this sandbox (no permission to execute build tools), so this is a read-through review only — no compile/runtime verification.

Overall this is a well-structured, carefully-commented PR. The shadow-suppression handoff logic for chained foreground→background picks (Eyedroppers.swift:456-549, tracked via handedOff) is correct on all exit paths I traced, including cancellation of a chained pick. The adaptive layout's suppress-but-remember approach and the AdaptiveValueText proportional-scaling trick to avoid bistable line-wrapping are both nice solutions. A couple of smaller issues worth a look:

1. Border window's level goes stale if "float on top" is toggled after it's created
WindowCoordinator.makeBorderWindow() sets window.level = pikaWindow.level once, at creation time:

window.level = pikaWindow.level

(Pika/Services/WindowCoordinator.swift, in makeBorderWindow())

But PikaWindow.swift's Defaults.observe(.appFloating) only updates pikaWindow.level, not the border window's:

Defaults.observe(.appFloating) { change in
    window.level = change.newValue == true ? .floating : .normal
}.tieToLifetime(of: self)

Since borderWindow is created lazily once and reused for the rest of the app session (private var borderWindow: NSWindow?), if a user selects "Hide shadow" (so the border is showing) and later toggles "always float on top" in preferences, the border's level never updates to match — it stays on whatever level it had when first created. This could leave the hairline border rendering at the wrong stacking layer relative to other windows.

Fix this →

2. Stale comment vs. actual stroke math in updateShadowBorder()

if let borderView = border.contentView as? ShadowBorderView {
    // Push the stroke ~1.5pt beyond the main frame edge so it lands on the visible
    // glass edge, and match the window's rounded-corner radius.
    borderView.strokeInset = borderPad
    borderView.cornerRadius = 20
}

strokeInset is set to borderPad (6.0), the same value used to expand the border window's frame beyond the main window's frame. Since ShadowBorderView.draw insets the stroke rect by strokeInset from the (already-expanded) view bounds, this places the stroke exactly on the main window's original frame edge — not "~1.5pt beyond" it as the comment states. Either the comment is stale (left over from earlier tuning) or the value should be something like borderPad - 1.5 to match the documented intent. Worth double-checking against how it actually looks on-screen with "Hide shadow" selected.

Fix this →

3. Minor: PikaAdaptiveHeight.typeLabels threshold is effectively unreachable
In ContentView.swift, PikaAdaptiveHeight.typeLabels = 150, but the window's SwiftUI minHeight is 160 (WindowCoordinator.installMainWindowContent) and PikaAdaptiveHeight.floor = 160. Since content height can never drop below 160, the height >= PikaAdaptiveHeight.typeLabels check in showsTypeLabels is always true — type labels only ever hide because of the preview-pill overlap (!previewVisible), never because of height alone. Not a functional bug (the type-label-last comment intent is still roughly honored via the preview overlap), but the 150 threshold is dead weight and could be confusing to a future reader tuning these values. Consider removing the height check for this one, or lowering floor/documenting why it's intentionally unreachable.

Notes

  • Confirmed the described shadow-suppression-during-chained-pick logic is correct across all termination paths in Eyedroppers.swift (normal pick, cancelled pick, cancelled chained pick).
  • Confirmed the new localization keys are present with literal English text in every non-English .lproj file, matching what the PR description states.
  • Couldn't verify visually (no way to build/run the app here) — the corner-radius/stroke-inset math in particular (On toggle, set focus on the primary window #2 above) is worth eyeballing against a real "Hide shadow" window.

Repository owner deleted a comment from claude Bot Jul 19, 2026
- Keep the companion border window's level in sync with the main window so
  toggling "float on top" while shadowless doesn't strand it on a stale level
- Correct the stale stroke-inset comment (stroke sits on the frame edge)
- Drop the unreachable typeLabels height threshold; type labels hide via the
  preview-pill overlap only

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@superhighfives

Copy link
Copy Markdown
Owner Author

Thanks @claude — worked through all three findings (fixed in b1d3940):

  1. Border window level stale on float toggle — Fixed. WindowCoordinator now observes .appFloating and updates borderWindow?.level, and updateShadowBorder() re-asserts border.level = pikaWindow.level on each refresh, so the border tracks the main window's level.
  2. Stale stroke-inset comment — Fixed the comment. The values (strokeInset = borderPad, cornerRadius = 20) are the ones tuned to sit on the visible glass edge on-device; the "~1.5pt beyond" note was left over from earlier tuning, so it now reads as sitting on the frame edge.
  3. Unreachable typeLabels threshold — Fixed. Removed the typeLabels constant and the always-true height >= … check; showsTypeLabels is now just !previewVisible, with a comment noting type labels hide only via the preview-pill overlap since the floor (160) sits above any useful height threshold.

@claude

claude Bot commented Jul 19, 2026

Copy link
Copy Markdown

Claude finished @superhighfives's task in 39s —— View job


Verified fixes in b1d3940

  • Review fix for border window level staleness on float toggle
  • Review fix for stale stroke-inset comment
  • Review fix for unreachable typeLabels threshold
  • Post verification summary

All three findings are correctly addressed:

  1. Border window levelWindowCoordinator (Pika/Services/WindowCoordinator.swift:53-58) now observes .appFloating and updates borderWindow?.level directly, and makeBorderWindow()/updateShadowBorder() also re-asserts border.level = pikaWindow.level on every refresh (line 98). Between the two, the border window's stacking level can no longer go stale relative to the main window.
  2. Stroke-inset comment — now reads "Sit the stroke on the main window's frame edge (which lines up with the visible glass edge)", matching the actual strokeInset = borderPad behavior. No more mismatch between comment and code.
  3. typeLabels threshold — the dead constant is fully removed from Constants.swift/ContentView.swift, and showsTypeLabels is now simply !previewVisible with a comment explaining why height gating was dropped (the 160pt floor sits above any useful threshold). Confirmed no remaining references to typeLabels anywhere in the codebase.

Nothing further to flag — this looks good to merge from a correctness standpoint (still no ability to build/run the app in this sandbox, so the on-screen stroke/corner-radius appearance is worth a final visual glance, but the logic and comments are now consistent).

@superhighfives

Copy link
Copy Markdown
Owner Author

@claude Re-review the fixes.

@claude

claude Bot commented Jul 19, 2026

Copy link
Copy Markdown

Claude finished @superhighfives's task in 1m 13s —— View job


Re-reviewing the fixes from b1d3940

  • Gather context (diff since last review, read updated files)
  • Verify fix 1: border window level staleness on float toggle
  • Verify fix 2: stroke-inset comment
  • Verify fix 3: unreachable typeLabels threshold
  • Post final summary

All three findings from the previous review are correctly and cleanly addressed in b1d3940:

1. Border window level stalenessWindowCoordinator.setupMainWindow() now observes .appFloating directly and updates borderWindow?.level (Pika/Services/WindowCoordinator.swift:55-58), and updateShadowBorder() additionally re-asserts border.level = pikaWindow.level on every refresh (WindowCoordinator.swift:98), on top of the level being set at creation time in makeBorderWindow() (WindowCoordinator.swift:126). This gives three points of convergence, so the border can no longer be stranded on a stale level when "float on top" is toggled while shadowless. Confirmed PikaWindow.swift's own .appFloating observer (lines 33-34) and this new one don't conflict — they just independently keep two different windows' levels in sync with the same setting.

2. Stroke-inset comment — now reads "Sit the stroke on the main window's frame edge (which lines up with the visible glass edge)," which matches the actual strokeInset = borderPad behaviour. I also checked the related doc comment on ShadowBorderView.strokeInset itself (WindowCoordinator.swift:324-326, "reducing this pushes the outline outward...") — it's still accurate as a general description of the mechanism and doesn't contradict the fixed call-site comment, so no follow-up needed there.

3. Unreachable typeLabels threshold — the constant is fully removed from ContentView.swift, showsTypeLabels is now simply !previewVisible, and the surrounding doc comment on PikaAdaptiveHeight was updated to explain why (the 160pt floor sits above any useful threshold). Grepped the whole Pika/ tree — no remaining references to typeLabels anywhere.

No new issues found in this pass. As before, I don't have permission to run xcodebuild/tests in this sandbox, so this remains a read-through verification — logic and comments are now internally consistent, but a final on-screen glance at the border/stroke appearance is still worth doing before merge.

@superhighfives
superhighfives merged commit b1127b8 into 1.9.0 Jul 19, 2026
2 checks passed
@superhighfives
superhighfives deleted the 238-window-shadow-and-size branch July 19, 2026 18:01
@superhighfives superhighfives mentioned this pull request Jul 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant