fix: make mouse back/forward navigation configurable#1708
Open
kingdomseed wants to merge 2 commits intosuperset-sh:mainfrom
Open
fix: make mouse back/forward navigation configurable#1708kingdomseed wants to merge 2 commits intosuperset-sh:mainfrom
kingdomseed wants to merge 2 commits intosuperset-sh:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds an opt-in setting to enable mouse back/forward button navigation between workspace tabs in the desktop app (defaulting to disabled to avoid conflicting with OS/system mouse shortcuts), persisted via local-db.
Changes:
- Add
mouseNavigationEnabledsetting to the local DB schema + Drizzle migration/snapshot metadata. - Expose
getMouseNavigationEnabled/setMouseNavigationEnabledvia the settings tRPC router and wire it into the Behavior settings UI. - Gate the
NavigationControlsmouse button listener behind the new setting.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/local-db/src/schema/schema.ts | Adds mouseNavigationEnabled boolean setting column to the Drizzle schema. |
| packages/local-db/drizzle/meta/_journal.json | Registers migration 0032 in Drizzle’s migration journal. |
| packages/local-db/drizzle/meta/0032_snapshot.json | New Drizzle snapshot reflecting the updated schema. |
| packages/local-db/drizzle/0032_normal_cardiac.sql | Migration adding mouse_navigation_enabled column to settings. |
| apps/desktop/src/shared/constants.ts | Introduces default value DEFAULT_MOUSE_NAVIGATION_ENABLED = false. |
| apps/desktop/src/renderer/routes/_authenticated/settings/utils/settings-search/settings-search.ts | Adds a searchable settings entry for the new Behavior toggle. |
| apps/desktop/src/renderer/routes/_authenticated/settings/behavior/components/BehaviorSettings/BehaviorSettings.tsx | Adds UI toggle + optimistic mutation for mouse navigation setting. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/NavigationControls/NavigationControls.tsx | Conditionally installs mouseup handler only when setting is enabled. |
| apps/desktop/src/lib/trpc/routers/settings/index.ts | Adds tRPC procedures to get/set the new persisted setting. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
kingdomseed
changed the title
Contributor
📝 WalkthroughWalkthroughThe changes introduce a configurable mouse navigation feature, allowing users to enable or disable mouse button 3/4 navigation between workspace tabs. New database schema, TRPC procedures, and UI settings components enable fetching and persisting this user preference. Changes
Sequence DiagramsequenceDiagram
participant User
participant UI as BehaviorSettings UI
participant Query as getMouseNavigationEnabled Query
participant Mutation as setMouseNavigationEnabled Mutation
participant TRPC as TRPC Router
participant DB as Settings Table
User->>UI: Opens settings page
UI->>Query: Fetch mouse navigation enabled
Query->>TRPC: Call getMouseNavigationEnabled()
TRPC->>DB: Read mouseNavigationEnabled
DB-->>TRPC: Return value or null
TRPC-->>Query: Return (value ?? DEFAULT_FALSE)
Query-->>UI: Display current setting
User->>UI: Toggle mouse navigation switch
UI->>UI: Optimistic update local state
UI->>Mutation: Call setMouseNavigationEnabled({enabled})
Mutation->>TRPC: Call setMouseNavigationEnabled()
TRPC->>DB: Upsert mouseNavigationEnabled
DB-->>TRPC: Confirm success
TRPC-->>Mutation: Return {success: true}
Mutation-->>UI: Invalidate query, sync state
Note over UI,DB: NavigationControls component now<br/>conditionally listens to mouse<br/>buttons 3/4 based on setting
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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}
Contributor
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 (2)
packages/local-db/drizzle/meta/0032_snapshot.json (1)
138-182:⚠️ Potential issue | 🔴 CriticalJSON syntax error: missing commas before duplicate key sections.
The
organization_members_organization_id_organizations_id_fkandorganization_members_user_id_users_id_fkforeign key definitions contain syntax errors that make the file unparseable. Line 150 ends without a comma before the duplicate keys begin at line 151; the same issue occurs at lines 171–172. This is a merge or generation artifact that resulted in malformed JSON.Regenerate the snapshot with
drizzle-kit generateto produce valid output.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/local-db/drizzle/meta/0032_snapshot.json` around lines 138 - 182, The JSON in the snapshot contains duplicated/merged sections and missing commas in the foreign key objects for organization_members_organization_id_organizations_id_fk and organization_members_user_id_users_id_fk which makes the file invalid; fix by removing the duplicated blocks and restoring proper JSON structure (ensure each object has a single set of keys: name, tableFrom, tableTo, columnsFrom, columnsTo, onDelete, onUpdate, with commas between entries) for the two foreign keys, or simply regenerate the snapshot using drizzle-kit generate to recreate a valid packages/local-db/drizzle/meta/0032_snapshot.json containing correct definitions for organization_members_organization_id_organizations_id_fk and organization_members_user_id_users_id_fk.packages/local-db/drizzle/meta/_journal.json (1)
229-237:⚠️ Potential issue | 🔴 CriticalCritical: Duplicate
when/tagkeys in journal entry idx 32 — migration will be silently lost.Entry idx 32 in
_journal.jsoncontains two"when"and two"tag"keys within the same JSON object (lines 232–235). When parsed as standard JSON, only the last occurrence of each key is retained, meaning"0032_normal_cardiac"is silently overwritten by"0032_migrate_workspace_ids_to_uuid_v4". As a result, the migration from0032_normal_cardiac.sqlwill never be applied to existing databases.This is a merge conflict where both migrations received idx 32. Renumber the new migration to idx 33 (or the next available index) and rename all corresponding files (
0033_normal_cardiac.sql,0033_snapshot.json, etc.) to match. Alternatively, regenerate viadrizzle-kit generateafter rebasing onmain.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/local-db/drizzle/meta/_journal.json` around lines 229 - 237, The journal entry at idx 32 contains duplicate "when" and "tag" keys ("0032_normal_cardiac" and "0032_migrate_workspace_ids_to_uuid_v4") so the first migration is being overwritten; fix by renumbering the conflicting migration to the next available index (e.g., change "0032_normal_cardiac" -> "0033_normal_cardiac"), update the corresponding filenames (0032_normal_cardiac.sql, 0032_snapshot.json, etc.) to match the new index, and update the _journal.json entry so idx 32 refers only to the intended migration and the new idx (33) has the renamed migration and correct "when"/"tag" values; alternatively re-generate the journal with drizzle-kit generate after rebasing main to ensure consistent indices.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/routes/_authenticated/settings/behavior/components/BehaviorSettings/BehaviorSettings.tsx (1)
462-483: LGTM — UI block is well-formed; consider adding a test for the new toggle.The
htmlFor/idpairing is correct, the?? falsedefault aligns with the opt-in intent, and the disabled logic mirrors all other switches. However, no test file is provided for the new toggle path. As per coding guidelines, tests should be co-located usingBehaviorSettings.test.tsx.#!/bin/bash # Check whether a test file already exists for BehaviorSettings fd -t f "BehaviorSettings.test.tsx" --full-path🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/routes/_authenticated/settings/behavior/components/BehaviorSettings/BehaviorSettings.tsx` around lines 462 - 483, Add a co-located test file BehaviorSettings.test.tsx that covers the new mouse navigation toggle in the BehaviorSettings component: mount/render BehaviorSettings with showMouseNavigation=true and assert the Switch with id "mouse-navigation" / Label exists, verify it defaults to unchecked when mouseNavigationEnabled is undefined (the component uses "mouseNavigationEnabled ?? false"), verify the Switch is disabled when isMouseNavigationLoading or setMouseNavigationEnabled.isPending are true, and simulate toggling to assert setMouseNavigationEnabled.mutate is called; reference the component and props: BehaviorSettings, showMouseNavigation, mouseNavigationEnabled, setMouseNavigationEnabled, and isMouseNavigationLoading when writing tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/local-db/drizzle/meta/_journal.json`:
- Around line 229-237: The journal entry at idx 32 contains duplicate "when" and
"tag" keys ("0032_normal_cardiac" and "0032_migrate_workspace_ids_to_uuid_v4")
so the first migration is being overwritten; fix by renumbering the conflicting
migration to the next available index (e.g., change "0032_normal_cardiac" ->
"0033_normal_cardiac"), update the corresponding filenames
(0032_normal_cardiac.sql, 0032_snapshot.json, etc.) to match the new index, and
update the _journal.json entry so idx 32 refers only to the intended migration
and the new idx (33) has the renamed migration and correct "when"/"tag" values;
alternatively re-generate the journal with drizzle-kit generate after rebasing
main to ensure consistent indices.
In `@packages/local-db/drizzle/meta/0032_snapshot.json`:
- Around line 138-182: The JSON in the snapshot contains duplicated/merged
sections and missing commas in the foreign key objects for
organization_members_organization_id_organizations_id_fk and
organization_members_user_id_users_id_fk which makes the file invalid; fix by
removing the duplicated blocks and restoring proper JSON structure (ensure each
object has a single set of keys: name, tableFrom, tableTo, columnsFrom,
columnsTo, onDelete, onUpdate, with commas between entries) for the two foreign
keys, or simply regenerate the snapshot using drizzle-kit generate to recreate a
valid packages/local-db/drizzle/meta/0032_snapshot.json containing correct
definitions for organization_members_organization_id_organizations_id_fk and
organization_members_user_id_users_id_fk.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/settings/behavior/components/BehaviorSettings/BehaviorSettings.tsx`:
- Around line 462-483: Add a co-located test file BehaviorSettings.test.tsx that
covers the new mouse navigation toggle in the BehaviorSettings component:
mount/render BehaviorSettings with showMouseNavigation=true and assert the
Switch with id "mouse-navigation" / Label exists, verify it defaults to
unchecked when mouseNavigationEnabled is undefined (the component uses
"mouseNavigationEnabled ?? false"), verify the Switch is disabled when
isMouseNavigationLoading or setMouseNavigationEnabled.isPending are true, and
simulate toggling to assert setMouseNavigationEnabled.mutate is called;
reference the component and props: BehaviorSettings, showMouseNavigation,
mouseNavigationEnabled, setMouseNavigationEnabled, and isMouseNavigationLoading
when writing tests.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/desktop/src/lib/trpc/routers/settings/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/NavigationControls/NavigationControls.tsxapps/desktop/src/renderer/routes/_authenticated/settings/behavior/components/BehaviorSettings/BehaviorSettings.tsxapps/desktop/src/renderer/routes/_authenticated/settings/utils/settings-search/settings-search.tsapps/desktop/src/shared/constants.tspackages/local-db/drizzle/0032_normal_cardiac.sqlpackages/local-db/drizzle/meta/0032_snapshot.jsonpackages/local-db/drizzle/meta/_journal.jsonpackages/local-db/src/schema/schema.ts
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
1 participant
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.
Description
Make mouse back/forward navigation configurable in the desktop app. The default is disabled to respect system shortcuts while keeping the behavior opt‑in.
Related Issues
Closes #1701
Type of Change
Testing
Screenshots (if applicable)
N/A
Additional Notes
Adds a new settings toggle under Settings → Behavior and persists it to the local DB. Includes a new local‑db migration.
Summary by CodeRabbit