-
Notifications
You must be signed in to change notification settings - Fork 773
new config: app:hideaibutton, app:disablectrlshiftarrows, app:disablectrlshiftdisplay #2850
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis PR introduces three new boolean settings: Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/docs/config.mdx (1)
110-144:⚠️ Potential issue | 🟡 MinorThe default configuration example block is stale and doesn't include the new keys.
Line 110 references "v0.11.5" but the three new settings (
app:hideaibutton,app:disablectrlshiftarrows,app:disablectrlshiftdisplay) aren't listed in this example. Consider updating the version reference and adding the new defaults to keep the docs in sync.
🧹 Nitpick comments (1)
frontend/app/store/keymodel.ts (1)
533-564: Consider extracting the repeated settings check into a helper.The same
globalStore.get(getSettingsKeyAtom("app:disablectrlshiftarrows"))call appears in all four arrow handlers. A small helper or a single check before the switch could reduce duplication.♻️ Example refactor
+function isCtrlShiftArrowsDisabled(): boolean { + return globalStore.get(getSettingsKeyAtom("app:disablectrlshiftarrows")); +} + globalKeyMap.set("Ctrl:Shift:ArrowUp", () => { - const disableCtrlShiftArrows = globalStore.get(getSettingsKeyAtom("app:disablectrlshiftarrows")); - if (disableCtrlShiftArrows) { + if (isCtrlShiftArrowsDisabled()) { return false; } switchBlockInDirection(NavigateDirection.Up); return true; });Apply the same pattern to the other three handlers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@docs/docs/config.mdx`:
- Line 145: The default in the shown JSON ("term:durable": false) conflicts with
the keys table that states term:durable defaults to true; update the JSON
default to "term:durable": true so the sample config matches the documented
default (or, if you prefer the JSON value, update the keys table text to say
"defaults to false"); locate the term:durable entry in the config block and make
the corresponding change so both the sample config and the keys table are
consistent.
- Line 137: Add a documentation entry for the configuration key
"window:fullscreenonlaunch" to the Configuration Keys table: describe its
purpose (controls whether the app launches in fullscreen), its type (boolean),
default value (false), and any platform/behavior notes; update the keys table in
docs/docs/config.mdx (the Configuration Keys section rows) to include a new row
for "window:fullscreenonlaunch" matching the format used by the other keys so it
appears alongside keys from lines ~35–108.
| app:hideaibutton | bool | Set to true to hide the AI button in the tab bar (defaults to false) |
| app:disablectrlshiftarrows | bool | Set to true to disable Ctrl+Shift+Arrow keybindings for block navigation (defaults to false) |
| app:disablectrlshiftdisplay | bool | Set to true to disable the Ctrl+Shift visual indicator display (defaults to false) |