Move personal settings to DG:settings dialog #86
Conversation
|
@trangdoan982 is attempting to deploy a commit to the Discourse Graphs Team on Vercel. A member of the Team first needs to authorize it. |
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThis pull request implements modifications to the settings interface and related components. The UI for the node menu trigger now includes an extra wrapper with descriptive text regarding refresh needs. A new component manages a toggle for the asynchronous query feature using React hooks. The settings dialog has been reorganized with new tabs for the personal node menu trigger and the backend query option, while outdated settings have been removed from the settings panel. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant AQS as AsyncQuerySettings Component
participant API as Extension API
U->>AQS: Mount component
AQS->>API: Fetch initial async query setting
API-->>AQS: Return current setting
Note over AQS: Component state initialized
U->>AQS: Toggle the switch
AQS->>API: Persist new setting
API-->>AQS: Acknowledge update
AQS-->>U: Update toggle state
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@CodeRabbit full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
apps/roam/src/components/DiscourseNodeMenu.tsx (1)
263-289: Consider handling refresh requirement programmatically.While the text clearly communicates that a refresh is needed after editing, consider implementing an automatic refresh or providing a refresh button to improve user experience.
Here's a suggested implementation:
<div> <p className="mb-2 text-neutral-dark/80"> - Override the global trigger for the Discourse Node Menu. Must refresh - after editing. + Override the global trigger for the Discourse Node Menu. </p> <InputGroup inputRef={inputRef} placeholder={isActive ? "Press keys ..." : "Click to set trigger"} value={shortcut} onKeyDown={handleKeyDown} onFocus={() => setIsActive(true)} onBlur={() => setIsActive(false)} rightElement={ + <> + <Button + hidden={!comboKey.key} + icon="refresh" + onClick={() => window.location.reload()} + minimal + title="Apply changes" + /> <Button hidden={!comboKey.key} icon={"remove"} onClick={() => { setComboKey({ modifiers: 0, key: "" }); // personal settings extensionAPI.settings.set("personal-node-menu-trigger", ""); }} minimal + title="Clear shortcut" /> + </> } /> </div>apps/roam/src/components/settings/Settings.tsx (1)
171-177: Remove commented-out code.Dead code should be removed rather than commented out. If this code needs to be referenced later, it can be found in the version control history.
- {/* <Button - icon="cross" - minimal - intent={Intent.NONE} - onClick={onClose} - className="absolute top-0 right-0" - /> */}apps/roam/src/components/settings/AsyncQuerySettings.tsx (3)
5-8: Consider memoizing the extension API instance.While the current implementation works, you could optimize it by memoizing the extension API instance using
useMemoto prevent unnecessary re-creation on re-renders.- const extensionApi = getExtensionApi(); + const extensionApi = React.useMemo(() => getExtensionApi(), []);
9-14: Add error handling and use a more descriptive setting key.Consider these improvements:
- Add error handling for the API call
- Use a more descriptive setting key than "async-q"
useEffect(() => { - const savedSetting = extensionApi.settings.get("async-q"); - setIsEnabled(!!savedSetting); + try { + const savedSetting = extensionApi.settings.get("useBackendQuery"); + setIsEnabled(!!savedSetting); + } catch (error) { + console.error("Failed to load async query setting:", error); + setIsEnabled(false); // Fallback to disabled state + } }, []);
23-38: Well-structured UI with clear user feedback.The UI implementation is clean and informative. Consider adding an aria-label to improve accessibility further.
<Switch checked={isEnabled} onChange={handleToggle} label="Use Backend Query (Beta)" className={Classes.LARGE} + aria-label="Toggle backend query feature" />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/roam/src/components/DiscourseNodeMenu.tsx(1 hunks)apps/roam/src/components/settings/AsyncQuerySettings.tsx(1 hunks)apps/roam/src/components/settings/Settings.tsx(3 hunks)apps/roam/src/utils/createSettingsPanel.ts(0 hunks)
💤 Files with no reviewable changes (1)
- apps/roam/src/utils/createSettingsPanel.ts
🔇 Additional comments (4)
apps/roam/src/components/DiscourseNodeMenu.tsx (1)
263-267: LGTM! Clear and well-styled descriptive text.The added wrapper with descriptive text improves user experience by providing clear instructions about the node menu trigger customization.
apps/roam/src/components/settings/Settings.tsx (2)
18-19: LGTM! Clean modular organization of settings components.The new imports are well-organized and follow good modular design principles.
92-94: LGTM! Clear and organized section headers.The new section headers improve UI organization and maintain consistent styling. The separation between "Global Settings" and "Personal Settings" provides clear navigation for users.
Also applies to: 155-157
apps/roam/src/components/settings/AsyncQuerySettings.tsx (1)
1-4: LGTM! Clean and well-organized imports.The imports are properly structured, using specific imports rather than wildcard imports, and the dependencies are appropriate for the component's needs.
mdroidian
left a comment
mdroidian
left a comment
There was a problem hiding this comment.
Good work so far. See comments for changes.
2 participants
Summary by CodeRabbit
New Features
Refactor