Skip to content

fix: reclaim the unused side of a single-provider island - #110

Closed
dltacube wants to merge 1 commit into
ericjypark:mainfrom
dltacube:fix/single-provider-footprint
Closed

dltacube wants to merge 1 commit into
ericjypark:mainfrom
dltacube:fix/single-provider-footprint

Conversation

@dltacube

@dltacube dltacube commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

With one provider selected, the compact and peek silhouettes still reserve space for two providers. With Always show usage enabled, the empty right wing occupies 134 points of menu-bar space.

Size the island for the selected provider count and share the geometry between rendering and mouse hit regions. On notched displays, preserve the remaining provider's left-side position while removing the unused right wing. On other displays, center the smaller pill. Keep the expanded panel centered at its existing size. Provider changes update the layout and stationary-pointer click-through state live.

Related work

#101 retains both sides in single-provider mode and fills the right side with a quota gauge. This PR instead reclaims that menu-bar space; adopting both approaches would require a design decision or an optional layout preference. #34 reduces the idle footprint by hiding logos; this change also reduces the single-provider peek / Always show usage footprint.

Validation

  • Full ./scripts/run-tests.sh passed, including 22 new layout regression assertions.
  • Universal arm64/x86_64 ./build.sh passed.
  • Installed and ran on a notched Apple Silicon Mac. Before/after menu-bar captures confirmed that the removed wing exposes underlying menu-bar items.
  • Expanded panel opening and smaller layout after restart verified.
  • Hit-region geometry is tested; physical click-through into an underlying system menu was not separately verified.
  • Merges cleanly with current upstream main.

Bundle identity, version, Sparkle configuration, credentials, and polling behavior are unchanged.

Summary by CodeRabbit

  • New Features
    • In compact and peek states with one provider selected, the unused right-side area no longer blocks clicks. On notched displays, the remaining provider sits beside the notch; on other displays, the smaller island stays centered. The expanded panel remains centered.
  • Tests
    • Added coverage for island positioning across display types, provider selections, and island states.

@coderabbitai

coderabbitai Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 986be0f0-97e9-4034-bc44-1473b7b7a2fd

📥 Commits

Reviewing files that changed from the base of the PR and between 810b8ac and 0b999c3.

📒 Files selected for processing (8)
  • README.md
  • Sources/Model/IslandLayout.swift
  • Sources/Model/IslandModel.swift
  • Sources/Views/IslandRootView.swift
  • Sources/Window/IslandHostingView.swift
  • Sources/Window/IslandWindowController.swift
  • Tests/IslandLayoutTests.swift
  • scripts/run-tests.sh

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.


📝 Walkthrough

Walkthrough

IslandModel now computes island geometry from provider selection and display characteristics. The content view and hit testing use the shared layout. Tests cover provider selection, display changes, geometry, and persisted selection.

Changes

Provider-aware island layout

Layer / File(s) Summary
Compute layout from provider selection
Sources/Model/IslandLayout.swift, Sources/Model/IslandModel.swift, Tests/IslandLayoutTests.swift, scripts/run-tests.sh, README.md
IslandModel computes compact and peek layouts from provider visibility and display characteristics. The layout value defines the island rectangle and offset. The executable tests and README cover the behavior.
Apply layout to content and hit testing
Sources/Views/IslandRootView.swift, Sources/Window/IslandHostingView.swift, Sources/Window/IslandWindowController.swift
The content view applies the layout offset. Hosting and window hit testing use the layout rectangle. The window controller updates the cursor hit area when the layout changes.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant ProviderVisibilityStore
  participant IslandModel
  participant IslandRootView
  participant IslandWindowController
  ProviderVisibilityStore->>IslandModel: Publish provider selection
  IslandModel->>IslandModel: Recompute layout
  IslandModel->>IslandRootView: Apply horizontal offset
  IslandModel->>IslandWindowController: Publish layout change
  IslandWindowController->>IslandWindowController: Update cursor hit area
Loading

Suggested reviewers: ericjypark

Merge Risk: ⚪ Minimal · up to 0b999

The provider-aware layout appears mergeable after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 0b999

The change narrows the interactive island to match its visible layout and updates mouse pass-through when provider selection changes. No introduced security issue was established. Physical click-through into an underlying menu was not independently verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The security-relevant exposure is local desktop input routing: an incorrect pass-through state could intercept clicks intended for menu-bar items beneath the island window.

Trust Boundaries and Controls

  • observed — View hit testing and window-level mouse pass-through use the same layout rectangle rather than independently calculated bounds.

Hardening Proposals

  • proposed — Physically verify clicks into an underlying system menu after provider changes, including with a stationary pointer and during the layout transition; geometry tests alone do not establish operating-system click-through behavior.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 7 files. (1 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: reclaiming the unused side of the island when one provider is selected.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 7 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@ericjypark

Copy link
Copy Markdown
Owner

Thanks for the work on this, especially the layout and click-through handling. For the single-provider experience, we've decided to keep the balanced layout in #101 and use the right side for the quota gauge, so we're closing this PR in favor of that direction. We appreciate the contribution!

@ericjypark ericjypark closed this Sep 30, 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.

2 participants