Repository navigation
Add setting to disable menu card hover highlight - #4414
cynicalight wants to merge 1 commit into
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
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. |
clawsweeper
Bot
added
P3
|
Codex review: needs real behavior proof before merge. What this changesThis PR adds a default-on setting that controls selection backgrounds and text tinting for menu cards and Refresh. Example: Turn off “Highlight menu cards on hover” and point at the Codex Overview card.
Review scores
ProductKind: Preference · Worth it: Needs a maintainer decision · Fix scope: Complete Merge readiness⛔ Blocked before merge - 4 items remain Keep this PR open for an owner decision on the appearance preference; the patch also removes visual keyboard selection when the setting is disabled. Priority: P3 Decision needed
Before merge
Findings
Tests
Agent review detailsHow this fits togetherSettingsStore persists the preference, menu factories pass it into custom row views, and the menu selection delegate drives their SwiftUI or GPU rendering. flowchart TD
A[Menu settings toggle] --> B[SettingsStore preference]
B --> C[Menu row factories]
C --> D[Custom card and Refresh views]
E[Pointer and keyboard selection] --> F[Menu highlight delegate]
F --> D
D --> G[Selection background and text tint]
Technical reviewBest possible solution: If the owner approves the opt-out, suppress pointer hover while retaining a visible keyboard selection cue in the existing row renderers. Do we have a high-confidence way to reproduce the issue? Source inspection establishes that disabling the setting hides selection for both pointer and keyboard navigation; no runtime reproduction was performed. Is this the best way to solve the issue? The shared renderer is the appropriate implementation boundary, but gating every selection event is broader than the stated hover preference. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 6e118bdb5782. Provenance checked
TestingProof path: in-process harness. SecurityNone. EvidenceWhat I checked:
Likely related people:
Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. LabelsLabel changes: No label changes. Label justifications:
Rating scale6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior. WorkflowClawSweeper edits this one comment on every review. Comment HistoryReview history (1 earlier review cycle)
Reviewed October 10, 2026, 1:26 PM ET / 17:26 UTC (Revision 2). |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e6a964ffc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| private func applyHighlight(animated: Bool) { | ||
| let visuallyHighlighted = self.hoverHighlightEnabled && self.isRowHighlighted |
There was a problem hiding this comment.
When a user turns this setting off and navigates the menu with the arrow keys, this predicate also suppresses the keyboard-selected state because NSMenuDelegate.menu(_:willHighlight:) drives both pointer hover and keyboard selection. Since MenuCardMenuItem.isHighlighted is hard-coded to false in StatusItemController+MenuCardItems.swift, there is no native fallback, so cards (and the similarly gated Refresh row) provide no visual indication of the current keyboard target even though navigation and submenus still work; retain a focus treatment for keyboard-origin highlights while suppressing only pointer hover.
Useful? React with 👍 / 👎.
This branch has not been deployed
1 participant
Summary
Why
The current hover treatment recolors the whole selected card. On the Codex overview card, this turns colored text and the Weekly left remaining-progress bar white. It makes the color cues harder to read and can be visually distracting.
I previously proposed custom hover color and opacity in #4412. Local testing showed that approach did not change the Codex card as intended, and it added more settings and rendering changes than this issue needs. I closed that PR and replaced it with this smaller opt-out.
Validation
PATH=/opt/homebrew/bin:$PATH make check— passed.PATH=/opt/homebrew/bin:$PATH make test— passed: 150/150 groups, 0 failures, 0 timeouts.CODEXBAR_DISABLE_KEYCHAIN_ACCESS=1; the new setting is off in the debug preference domain for manual hover inspection.Screenshot
Live menu capture pending from the locally running debug build.