Fix menu bar layout editor drag-and-drop - #3121
Merged
steipete merged 13 commits intoAug 21, 2026
Merged
Conversation
Layout editor chips were Buttons with .draggable attached. On macOS the button's gesture recognizer claims the mouse-down, so no drag session ever starts: reordering and the trash drop zone were unreachable by mouse and the Delete key was the only way to remove a token. Moving .draggable onto the button label does not help, because the button still owns the gesture. Replace the chips with a plain draggable view that keeps click selection, keyboard activation, and accessibility, and make the trash zone a click target for the selected token as well.
|
🦞👀 Pull request received. I will update this pull request when review starts. |
![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: 1b67f6e355
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| /// removed without dragging or reaching for the Delete key. | ||
| private var removeDropTarget: some View { | ||
| HStack(spacing: 6) { | ||
| let canRemoveSelection = self.selectedPosition != nil |
There was a problem hiding this comment.
After selecting a token on the second line and clicking “Remove line break,” the merge action leaves selectedPosition pointing to the now-nonexistent line 1. This condition still presents the new trash target as active and labels it “Remove selected,” but clicking it silently does nothing because the stored position is invalid; clear the selection during the merge or validate that the selected position still exists before setting this state.
Useful? React with 👍 / 👎.
Comment on lines
+62
to
+64
| .allowsHitTesting(!self.isDisabled) | ||
| .accessibilityElement() | ||
| .accessibilityAddTraits(.isButton) |
There was a problem hiding this comment.
When the layout already has two lines, the line-break chip sets isDisabled, but this only suppresses focus and mouse hit testing while the following modifiers still expose it as a button with a default accessibility action. Unlike the previous .disabled(...), VoiceOver can therefore encounter what appears to be an enabled control whose activation silently no-ops at handleActivate; apply disabled accessibility semantics as well as suppressing pointer interaction.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
P2
|
Codex review: needs real behavior proof before merge. Reviewed August 21, 2026, 6:52 PM ET / 22:52 UTC. ClawSweeper reviewWhat this changesThe PR replaces layout-editor token buttons with accessible draggable chips and adds selected-token removal through the trash target. Merge readiness⛔ Blocked until real behavior proof is added - 5 items remain Keep open: current main still uses button-wrapped draggable layout tokens, but this patch has two P2 interaction/accessibility defects and lacks native after-fix proof. Priority: P2 Review scores
Verification
Live VerificationCommand: Result: FAIL (failed) — execution before step 1 Assertions:
How this fits togetherCodexBar’s menu bar layout editor turns saved usage-token settings into editable chips. Selecting, dragging, dropping, and removing chips updates the persisted menu bar layout. flowchart LR
A[Saved layout settings] --> B[Layout editor]
B --> C[Token chips]
C --> D[Selection and keyboard input]
C --> E[Drag and drop]
D --> F[Updated layout]
E --> F[Updated layout]
Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Keep the chip approach, clear or validate selection across layout changes, expose disabled accessibility state, and land only after a redacted native interaction recording proves reordering and removal. Do we have a high-confidence way to reproduce the issue? No native macOS reproduction was supplied or run here; current main source clearly maps the reported drag path, but the required click-through remains unproven. Is this the best way to solve the issue? No. The plain-chip direction is narrowly targeted, but stale selection and disabled accessibility semantics must be repaired before it is a complete solution. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 00faa6ed477a. LabelsLabel changes:
Label justifications:
EvidenceAcceptance criteria:
What I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (1 earlier review cycle)
|
…its) (steipete#3116) * fix(commandcode): Add support for individual-pro-v1 plan ($80/mo credits) Command Code's updated Pro tier ($20/mo → $80 credits) ships as individual-pro-v1 (legacy individual-pro remains $30). The new planId hit the hard unknownPlan throw in CommandCodeUsageFetcher, surfaced as 'Unknown Command Code plan: individual-pro-v1'. Add the versioned plan to CommandCodePlanCatalog (verified against live frontend bundle assets/constants-B6rERLKd.js and pricing docs). Pro pricing now: $20 → $80 (docs/resources/pricing-limits), legacy Pro kept for backwards compat. * test(commandcode): Cover individual-pro-v1 plan resolution Regression coverage for the $80 Pro v1 tier: an active subscription with planId individual-pro-v1 must resolve through CommandCodePlanCatalog instead of hitting the unknownPlan throw, and the catalog check now pins pro-v1 at 80 credits.
…ponses (steipete#3128) Mainland Personal/Solo Token Plan (cn-personal) intermittently shows "Could not parse Alibaba Token Plan usage: Missing Personal usage windows" even though auth succeeds. The `/tokenplan/personal/api/v2/usage` gateway sometimes answers with a 200 "Success" envelope (`code=SUCCESS`, `successResponse=true`, empty errorCode) whose payload omits the `per5HourPercentage`/`per1WeekPercentage` windows. The parser treated that absence as a hard parse failure. - Throw a distinct `.usageWindowsUnavailable` for a Success-with-no-windows body instead of `parseFailed("Missing Personal usage windows")`. - Retry the usage call a few times (the payload is usually populated on an immediate re-request) before surfacing it. - When it stays empty, surface the softer "temporarily unavailable; it will refresh automatically" message. CodexBar keeps the last-good card either way (a returned empty snapshot would blank it, so this stays a thrown error). Verified on a real cn-personal account: the fetch retries and, while the endpoint was returning empty, surfaced the transient message with the last-good card preserved. Adds AlibabaTokenPlanPersonalUsageRetryTests (recovers when a full response follows an empty one; throws .usageWindowsUnavailable when every attempt is empty). Refs steipete#2500. Co-authored-by: LeoLin <leolin990405@gmail.com>
…ete#3111) * fix(claude): migrate email-keyed iCloud snapshots to slot keys * fix(sync): confirm CloudKit snapshot saves before deleting predecessors Terminal delete failures are reported once with delayed retries only for recoverable errors, and email-keyed leftovers wait until the replacement record is saved. * fix(sync): persist leftover snapshot deletes across delayed retries Keep pending predecessor deletes in the persistence envelope before sleeping so a relaunch can finish the CloudKit migration if the retry task never ran. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(sync): stop retrying terminal CloudKit replacement saves Mark slot-keyed migration snapshots complete after permission, auth, or invalid-argument save failures so the 120s snapshot push does not keep requeueing the same record. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(sync): bind delayed CloudKit deletes to the originating engine Skip leftover-record retries after an account switch so a sleeping task cannot delete a same-named snapshot in a newly signed-in iCloud account. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(sync): drop stale CloudKit predecessors that are live again A later live email-keyed snapshot must not stay queued for delete just because an earlier slot-keyed save still has a pending predecessor set. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(sync): cancel leftover snapshot deletes when they become live A delayed CloudKit retry must not delete an email-keyed snapshot that was published again after a transient predecessor delete. Drop that name from the persisted retry set and the engine queue. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(sync): do not retry CloudKit deletes for live snapshots A transient in-flight delete can land after the predecessor is live again. Skip persist-and-retry when the record is in the current live snapshot set so cancellation is not resurrected. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(sync): isolate CloudKit migration state from the next iCloud account Clear predecessor maps and snapshot hashes when persistence is wiped, and requeue leftover deletes only after the current live snapshot set has been reconciled. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(sync): persist predecessor deletes and requeue empty publications Keep the replacement-to-predecessor map in the persistence envelope across relaunch, and still requeue leftover snapshot deletes when the next publication is empty. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(sync): wait for every shared-mailbox replacement before deleting Two Claude Swap slots can share one email-keyed predecessor. Delete that leftover only after no unsaved replacement still points at it. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(sync): retry lost CloudKit responses and keep shared leftovers Treat serverResponseLost as a recoverable retry, and confirm saved replacements before abandoning failed siblings so a shared email-keyed record is not deleted early. * fix(sync): queue predecessor deletes for unchanged slot payloads When a slot snapshot is already published, newly obsolete email-keyed leftovers still need to be recorded and deleted instead of being skipped by the payload-hash shortcut. * fix(sync): confirm slot saves and ignore remote cache as live Predecessor deletes now wait for a confirmed replacement hash, and delete retries treat only local pending/confirmed snapshots as live so a fetched leftover cannot cancel its own removal. * fix(sync): do not treat terminal save failures as confirmed Skip retrying an unchanged terminal replacement without recording it in lastSnapshotHashes, so an unconfirmed slot cannot retire an email-keyed leftover. * fix(sync): record confirmed save hashes and skip all terminal snapshot saves Confirmed CloudKit saves now keep the in-flight payload hash, and terminal failures skip retrying that hash even when the snapshot has no predecessor. * fix(sync): requeue in-flight snapshot updates and clear save markers on stop A newer payload that arrives during an unconfirmed save stays pending and is flushed after that save completes, and toggling iCloud off no longer leaves in-flight hashes that skip every later publication. * fix(sync): retry unavailable iCloud accounts and drop in-flight hashes on conflict accountTemporarilyUnavailable is treated as a transient CloudKit error, and a server-winning conflict no longer leaves pendingSaveHashes blocking later snapshot publications. * fix(sync): do not requeue fetched snapshots over in-flight local saves Fetched CloudKit snapshots no longer overwrite an in-flight local payload, and pending local updates win when merging unpublished fleet cache entries. * fix(sync): drop retained snapshots when iCloud sync stops Pending snapshot payloads from before disable are discarded so re-enabling sync cannot upload or delete against a stale account set. * fix(sync): limit email-keyed snapshot cleanup to Claude Swap Predecessor deletes must not run for other providers that move from email to a durable account ID. Drop the unreleased changelog line. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(sync): defer restored predecessor deletes until live snapshots reconcile CKSyncEngine can confirm an in-flight slot save on relaunch before local snapshots publish. Wait until that set is applied so a leftover email-keyed record that became live again is not deleted. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
* perf(spend): parallelize loads and memoize model build - Parallelize independent provider refreshes in makeRequest via TaskGroup (was sequential 400ms-6s additive). - Parallelize Codex multi-account loads in SpendDashboardSource.load via ThrowingTaskGroup (was sequential 2s×N). - Memoize SpendDashboardModel: cache CurrencyExchange conversion per currency pair, hoist bounds once per build, and reuse static utcCalendar for Mistral/OpenRouter bucket (was new calendar per entry, 2920× per build). Evidence: - SpendDashboardController.swift:237 parallel baselines - SpendDashboardController.swift:433 parallel codex - SpendDashboardModel.swift:300 conversionCache + bounds hoist - SpendDashboardModel.swift:1069 utcCalendar reuse Before: 全部 3-10s empty → 2-3s; build 40ms → 8ms. After: TaskGroup wall-time = slowest provider, model build cached. * fix(spend): restore Codex account order after parallel load Task group completion order was appended directly to inputs, but providerRows uses input offset as tie-breaker for equal/unknown totals, so out-of-order completions reordered Codex · steipete#1/steipete#2 rows. Carry request index and sort results before appending, with compact formatting to keep file_length under warning. Fixes ClawSweeper P2 for steipete#3105. * test(spend): add out-of-order Codex concurrent order regression Verifies that parallel Codex loads restore configured request order even when second account's snapshot completes first. Equal totals make completion order visible via providerRows tie-breaker, so without sorting the rows would reorder. Covers ClawSweeper P2 for steipete#3105 and serves as needs-proof evidence. * test: update gatekeeper anchors after rebase to 54.0 * fix(spend): repair parallel load CI - file_length and escaping captures * fix(spend): debounce frequent refresh and throttle date window rebuilds - 250ms debounce for withObservationTracking and token publication bursts - 30s throttle for refreshDateWindow same-day revisits - display-only fast path to avoid Codex scan for filter/currency changes - update gatekeeper anchors for line shifts * Improve Antigravity retrieval: retired Flash alias and offline fallback - Map retired Flash wire ids (3.6/3.5/3-flash-agent) to 3.7-flash via canonicalModelID (opencodex RETIRED_FLASH_TIERS lesson), humanize via canonical, dedup collapsed windows by lowest remaining - Add AntigravityOfflineStore counting ~/.gemini/antigravity-cli/conversations/*.db (GEMINI_CLI_HOME override) with tokscale cache fallback, and AntigravityOfflineFetchStrategy as terminal offline probe in auto/cli pipelines - Cover with AntigravityModelLabelTests retired alias cases and AntigravityOfflineStoreTests (db/cache/count) * fix(gate): add missing provider-specific markers and sync anchors - add // Provider-specific by design for OpenCodex enrichment and token publication sync - sync gate anchors after debounce/throttle line shifts * fix(lint): wrap long provider-specific comment * Fix provider architecture gatekeeper for Antigravity offline and retired alias - Move CLI home marker to before gemini literal and update SpendDashboard marker to avoid flagging its own reason - Add marker before family() and update suppressed anchors to 748/751/754/757 - Allowlist offline gemini, family cluster, and UsageStore codex constructs * test: include offline strategy in antigravity pipeline expectations * fix(gate): sync remaining anchors and add missing markers * chore: trigger CI * fix(spend): make debounce instant for testing * fix(gate): update anchors after merge with main * fix(lint): break long delay line * fix(gate): drop stale codex anchor absorbed by sourceRevisions cluster
clawsweeper
Bot
added
the
merge-risk: 🚨 compatibility
steipete
added a commit
that referenced
this pull request
Aug 21, 2026
steipete
added a commit
that referenced
this pull request
Aug 21, 2026
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
7 participants
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.
Summary
Drag-and-drop in the menu bar layout editor never starts a drag session, so the trash
drop zone ("Drag here to remove") and chip reordering are unreachable with the mouse —
the Delete key is the only way to remove a token.
The chips are
Buttons with.draggableattached. On macOS the button's gesturerecognizer claims the mouse-down before the drag can begin. Attaching
.draggableto thebutton's label (as the placed chips currently do) does not help either, because the
button still owns the gesture.
Changes
MenuBarLayoutEditorChip: a plain draggable view instead of aButton, keepingclick selection,
.focusable()+ space/return activation, and the button accessibilitytrait plus the named "Remove" action.
conditional context menus and the strip's drop destinations are unchanged.
selected" and removing works without dragging or the Delete key.
menu_bar_layout_remove_selected, translated in all 22 catalogs.Commands run
swift build --target CodexBar— clean (typechecks the whole module)node Scripts/check-app-locales.mjs—App locales OK: Checked 22 catalogs against 1480 English keys.Verification caveat
This was developed on a machine with only CommandLineTools and no Xcode, so
make testand
swiftlintcould not run here (Testingmodule unavailable; swiftlint fails loadingsourcekitdInProc). No screenshots/GIF for the same reason. The change is behaviouraldrag-and-drop in a settings pane, so it needs a click-through on a machine with Xcode —
happy to iterate if CI or review finds anything.