feat(panel): reorder usage panel tabs by drag - #72
Conversation
Tab order was fixed to the declared case order. Let the user drag tabs sideways in the tab bar and persist the result. The row keeps its committed order while a drag is live and rearrangement is expressed through offsets only. Reordering the ForEach mid-drag moves a child inside its container, which tears down that child's gesture and splits one drag into several, each committing its own reorder. Ordering math lives in PanelTabReordering so slot placement and the threshold that separates a click from a drag can be unit tested. Persistence reuses the defensive normalization already used for reader settings, so unknown or missing stored tabs fall back to the declared order. Keyboard operation of the tab bar is lost with the Button removed; VoiceOver keeps a labelled action and the selected trait.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Walkthrough사용량 패널에 6개 탭의 순서를 저장하고 복원하는 기능을 추가했습니다. 탭 바는 드래그로 탭을 재정렬합니다. 설정 화면은 현재 순서를 표시하고 기본 순서로 초기화합니다. 재정렬 계산과 설정 영속화 테스트도 추가했습니다. Changes사용량 패널 탭 순서
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PanelTabBarView
participant PanelTabReordering
participant UsagePanelView
participant UsagePanelSettings
PanelTabBarView->>PanelTabReordering: 드래그 위치와 탭 프레임 전달
PanelTabReordering-->>PanelTabBarView: 미리보기 순서 반환
PanelTabBarView->>UsagePanelView: 새 탭 순서 전달
UsagePanelView->>UsagePanelSettings: setTabOrder 호출
UsagePanelSettings-->>UsagePanelView: tabOrder 변경 알림
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Toki/Features/UsagePanel/PanelSettingsView.swift (1)
142-148: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReset 버튼에 접근성 라벨을 추가해 주세요.
버튼 라벨이 "Reset"뿐입니다. VoiceOver 사용자는 무엇을 초기화하는지 알 수 없습니다. 같은 행의 "Tab order" 텍스트는 별도 요소입니다.
♿ 제안 수정
.buttonStyle(.plain) .font(.system(size: 11, weight: .semibold)) .foregroundColor(Color.white.opacity(settings.isUsingDefaultTabOrder ? 0.3 : 0.7)) .disabled(settings.isUsingDefaultTabOrder) + .accessibilityLabel(Text("Reset tab order"))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Toki/Features/UsagePanel/PanelSettingsView.swift` around lines 142 - 148, Update the “Reset” Button in the tab-order settings row to provide an accessibility label that clearly identifies it as resetting the tab order, while preserving the existing visible title and button behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Toki/Features/UsagePanel/PanelTabBarView.swift`:
- Around line 79-90: Update the tab item implementation in tabButton(for:) to
use a Button that sets activeTab, preserving the existing tab label content and
applying a plain button style. Attach dragGesture(for:) with simultaneousGesture
rather than replacing the Button gesture, so macOS keyboard focus and
Space/Return activation remain available while drag reordering continues to
work.
---
Nitpick comments:
In `@Toki/Features/UsagePanel/PanelSettingsView.swift`:
- Around line 142-148: Update the “Reset” Button in the tab-order settings row
to provide an accessibility label that clearly identifies it as resetting the
tab order, while preserving the existing visible title and button behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c11e7f97-241c-406b-96ae-99fa4e7ed21a
📒 Files selected for processing (9)
Toki.xcodeproj/project.pbxprojToki/Features/UsagePanel/PanelSettingsView.swiftToki/Features/UsagePanel/PanelTab.swiftToki/Features/UsagePanel/PanelTabBarView.swiftToki/Features/UsagePanel/PanelTabReordering.swiftToki/Features/UsagePanel/UsagePanelSettings.swiftToki/Features/UsagePanel/UsagePanelView.swiftTokiTests/PanelTabReorderingTests.swiftTokiTests/UsagePanelTabOrderSettingsTests.swift
| .gesture(dragGesture(for: tab)) | ||
| .onHover { isHovering in | ||
| guard draggingTab == nil else { return } | ||
| hoveredTab = isHovering ? tab : nil | ||
| } | ||
| .help(tab.title) | ||
| .accessibilityElement(children: .combine) | ||
| .accessibilityLabel(Text(tab.title)) | ||
| .accessibilityAddTraits(isSelected ? [.isSelected] : []) | ||
| .accessibilityAddTraits(isSelected ? [.isButton, .isSelected] : [.isButton]) | ||
| .accessibilityAction { | ||
| activeTab = tab | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift
탭 바의 키보드 조작이 회귀했습니다.
Button을 .gesture(dragGesture(for:))로 교체했습니다. 이제 탭 항목은 포커스를 받지 못합니다. 키보드 사용자는 탭을 전환할 수 없습니다. .accessibilityAction은 VoiceOver 액션만 제공하며, 키보드 포커스 이동과 Space/Return 활성화를 제공하지 않습니다.
배포 타깃이 macOS 13.0이므로 onKeyPress는 사용할 수 없습니다. Button으로 탭 항목을 감싸고 드래그는 simultaneousGesture로 처리하는 방식을 검토해 주세요. 재정렬 임계값이 4pt이므로 버튼 탭 인식과 충돌은 작습니다.
♿ 제안 방향 (검증 필요)
- .contentShape(Rectangle())
- .background(frameReader(for: tab))
- ...
- .gesture(dragGesture(for: tab))
+ .contentShape(Rectangle())
+ .background(frameReader(for: tab))
+ ...
+ // Button 래핑으로 키보드 포커스와 활성화를 복구하고,
+ // 드래그는 동시 제스처로 유지합니다.
+ .simultaneousGesture(dragGesture(for: tab))tabButton(for:)의 최상위를 Button { activeTab = tab } label: { ... } + .buttonStyle(.plain)로 감싸는 변경이 함께 필요합니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Toki/Features/UsagePanel/PanelTabBarView.swift` around lines 79 - 90, Update
the tab item implementation in tabButton(for:) to use a Button that sets
activeTab, preserving the existing tab label content and applying a plain button
style. Attach dragGesture(for:) with simultaneousGesture rather than replacing
the Button gesture, so macOS keyboard focus and Space/Return activation remain
available while drag reordering continues to work.
A press classified only by horizontal travel still activated the tab when the pointer was dragged vertically off it and released, where the previous Button cancelled activation on release outside its bounds. Selection now requires click-sized travel in both axes and a release inside the tab. An unmeasured tab stays clickable so the row is never dead before its first layout pass.
Hover callbacks were dropped wholesale while a drag was live, so a tab the pointer had left kept its hover colors until the cursor crossed it again. Exit events now still clear the matching tab. Enter events stay suppressed so tabs sliding under a stationary pointer do not light up mid-drag.
Codex review (lane-routed, local)Seven lanes activated: Scope resolution initially failed closed: Findings
Fixesd6f19aa — selection was classified from horizontal travel alone, so pressing a tab, dragging vertically off it and releasing still switched tabs, where the previous 82ba567 — hover callbacks were dropped wholesale during a drag, so a tab the pointer had left kept its hover colors until the cursor crossed it again. Exit events now clear the matching tab; enter events stay suppressed so tabs sliding under a stationary pointer do not light up mid-drag. Each fix was committed separately after Re-review
Left openF3 (RTL). F1 (keyboard). Replacing the Note on toolingFive of the seven lanes returned prose instead of the structured schema, so |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Toki/Features/UsagePanel/PanelTabReordering.swift (1)
54-64: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftRTL 레이아웃의 재정렬 방향을 지원하세요.
reordered는midX를 오름차순으로 정렬합니다.slotMidX는 슬롯을 왼쪽에서 오른쪽으로 누적합니다. RTL 레이아웃에서는 이 좌표 가정이 시각 순서와 다릅니다. 드래그 미리보기와 저장 순서가 잘못된 슬롯을 사용할 수 있습니다.
LayoutDirection을 전달하거나 좌표를 방향에 맞게 정규화하세요. RTL 이동과 슬롯 위치를 검증하는 테스트도 추가하세요.Also applies to: 67-90
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Toki/Features/UsagePanel/PanelTabReordering.swift` around lines 54 - 64, Update the tab reordering logic around sortedOthers, insertionIndex, and reordered to account for LayoutDirection: normalize or reverse midX comparisons so RTL uses visual right-to-left ordering while preserving LTR behavior. Ensure the same direction-aware ordering is used for drag previews and persisted slot placement, and add coverage for RTL moves and slot positions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@Toki/Features/UsagePanel/PanelTabReordering.swift`:
- Around line 54-64: Update the tab reordering logic around sortedOthers,
insertionIndex, and reordered to account for LayoutDirection: normalize or
reverse midX comparisons so RTL uses visual right-to-left ordering while
preserving LTR behavior. Ensure the same direction-aware ordering is used for
drag previews and persisted slot placement, and add coverage for RTL moves and
slot positions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c957180f-0d89-44a2-9645-ade758c6ddc7
📒 Files selected for processing (3)
Toki/Features/UsagePanel/PanelTabBarView.swiftToki/Features/UsagePanel/PanelTabReordering.swiftTokiTests/PanelTabReorderingTests.swift
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
A review comment carries only diff_hunk, the slice around itself, and on a large diff that slice often excludes the code the comment is actually about. On choi138/toki#72 the reviewer said a Button had been replaced by a gesture; the hunk's tail showed the gesture but not the Button, so the advisory could only answer 정보 부족. The pull request's own file list does contain it — "- Button {" is right there in PanelTabBarView.swift's patch. fetch_pull_files follows the house signature (subject_url, *, repository, limit) rather than taking a bare pull number, because _subject_coordinates cross-validates the GitHub-supplied subject URL against the independently allowlisted repository. Dropping that would let a poisoned notification payload redirect the read. An oversized response degrades to unavailable=True instead of raising. _decode_json rejects a body over _MAX_RESPONSE_BYTES with a *retryable* protocol_error, and runtime backoff would then retry that poll forever without ever delivering the notification — a livelock on exactly the large pull requests this read exists to explain. Wider diff context is best effort, so it degrades. unavailable is deliberately distinct from an empty files tuple, and a full page is reported as truncated=True: a reader must never conclude "nothing changed", or refute a review comment, from evidence it never received. Reading arbitrary repository files is NOT part of this change — that would let a prompt-injected review comment choose the path and exfiltrate source into Discord, and it needs its own path-traversal guard and repository allowlist. This read stays scoped to the pull request the notification is about.
1 participant
Summary
Panel tab order was fixed to
PanelTab.allCases. This lets the user drag tabs sideways in the tab bar and persists the result across launches.Gesture-based reordering rather than
.draggable: the panel is a borderless, nonactivatingNSPanelwhose local monitor dismisses on outside mouse-down, and a system drag session behaves unpredictably on that window type. A drag started inside the panel never dismisses it.Implementation
PanelTabmoves out ofUsagePanelView.swiftinto its own file and gains aStringraw value for persistence.PanelTabReorderingholds the ordering math as pure functions — slot placement, and the threshold separating a click from a drag — so both are unit tested.UsagePanelSettingspersiststabOrderunderusagePanel.tabOrder, reusing the defensive normalization already used for reader settings: unknown raw values and duplicates are dropped, and tabs missing from a stored order are appended so a shipped tab can never disappear.The row is never re-ordered mid-drag
ForEachstays on the committed order and rearrangement is expressed purely through offsets. Reordering theForEachwhile a drag is live moves a child inside its container, which tears down that child's gesture — one drag gets split into several, each committing its own reorder. Symptom: the dragged tab detaches from the cursor and the order changes several times per drag.The dragged tab therefore keeps its slot and tracks the pointer with the raw translation; every other tab slides to the slot the preview order assigns it. The spring animation applies only to the tabs making room.
Verification
500 unit tests pass;
swiftformat --lintandswiftlint --strictclean.Driven in the running app with synthetic events, capturing the drag frame by frame:
A single drag crosses the whole bar without breaking, the drop commits, and the stored order survives a relaunch. Click-to-select and the Reset row were exercised the same way.
Trade-off
Replacing the
Buttonwith a gesture drops keyboard operation of the tab bar (Full Keyboard Access). VoiceOver keeps a labelled action and the selected trait. This was chosen deliberately over layering a drag on aButton, where the button action races the gesture'sonEndedon mouse-up.Summary by CodeRabbit
새로운 기능
버그 수정