[ENG-197] Fix creating link with invalid chars #121
Merged
Conversation
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
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
|
@coderabbitai review |
|
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 📝 WalkthroughWalkthroughThe changes add a new filename validation method in the modal component and a regex constant in the validation utility. The modal now uses the new method in its suggestion handling flow to check for invalid characters before proceeding with node creation. If invalid characters are detected, a notice is shown and the process stops. In the utility, the new regex ensures that filename formats match acceptable characters. These modifications improve the error handling during node creation. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant M as NodeTypeModal
participant V as Validation (validateTextForInvalidChars)
participant N as Discourse Node Creator
U->>M: Selects suggestion (onChooseSuggestion)
M->>V: Validates text for invalid characters
V-->>M: Returns { valid: boolean, invalidChar? }
alt Filename is valid
M->>N: Create discourse node
else Filename invalid
M->>U: Display invalid character notice
end
Poem
🪧 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 (
|
✅ Actions performedReview 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: 0
🧹 Nitpick comments (2)
apps/obsidian/src/components/NodeTypeModal.tsx (2)
30-39: Well-structured validation method with clear return typeThe
validateTextForInvalidCharsmethod is well-implemented with a clear purpose and helpful return type that includes both validation status and the specific invalid character found.However, consider moving this validation logic to the
validateNodeType.tsfile to centralize all validation-related functionality in one place.
84-92: Effective validation with user-friendly error messagingThe validation before node creation prevents invalid filenames and provides a clear error message to the user. The 5000ms timeout for the notice gives users enough time to read the message.
For consistency, consider aligning the error message format with the one in
validateNodeType.tsor extracting these messages to shared constants.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/obsidian/src/components/NodeTypeModal.tsx(3 hunks)apps/obsidian/src/utils/validateNodeType.ts(2 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
apps/obsidian/src/components/NodeTypeModal.tsx (1)
apps/obsidian/src/utils/validateNodeType.ts (1)
INVALID_FILENAME_CHARS_REGEX(3-3)
🔇 Additional comments (3)
apps/obsidian/src/utils/validateNodeType.ts (2)
3-3: Good addition of a well-defined regex constantThe addition of
INVALID_FILENAME_CHARS_REGEXis a good approach to centralizing the validation pattern for invalid filename characters. Exporting it allows for consistent reuse across the codebase.
33-39: Well-implemented validation with clear error messagingThe validation logic correctly identifies invalid characters in the format string and provides a helpful error message that includes the specific problematic character. This makes it easier for users to understand and fix the issue.
apps/obsidian/src/components/NodeTypeModal.tsx (1)
4-4: Good import of shared validation regexCorrectly importing the
INVALID_FILENAME_CHARS_REGEXconstant ensures consistency in validation across the application.
trangdoan982
force-pushed
the
eng-197-link-bug
branch
from
a673053 to
721a226
Compare
mdroidian
pushed a commit
that referenced
this pull request
May 16, 2025
author Trang Doan <[email protected]> 1744314525 -0400 committer Michael Gartner <[email protected]> 1747354088 -0600 ENG-96 Create new relationship between nodes (#115) * instantiate new relationship worked * fix * address PR comments * fix bi-directional update issues * show only compatible node type options * small fix * breakdown the components. use datacore * working * address PR comments * improve search by only allowing compatible node results * . * rm dataview --------- Co-authored-by: Michael Gartner <[email protected]> Move llm-api endpoints to vercel serverless (#102) * testing gemini * move endgoint to website * open ai endpoint * added anthropic endpoint * pass env vars * add cors handdling and options * . * using centralised cors middleware * only adding bypass cookie * use right key * remove the bypass token requirement * sanitize, fix routes * remove server action config * DRY * remove unused * address review * adress review Roam: Add feedback toggle (#118) * add settings to hide or show button, also works when disabled or enabled midway * review * . --------- Co-authored-by: Michael Gartner <[email protected]> [ENG-197] Fix creating link with invalid chars (#121) * fix creating link with invalid chars * placeholder update --------- Co-authored-by: Michael Gartner <[email protected]> Roam: Add feedback button to settings menu - ENG-147 (#122) * add button to bottom right, don't hide sdk css, tested * remove intent not working git * remove ts-ignore and use a better type def * remove styling Update NodeConfig to use new UIDs for DiscourseNodeIndex and DiscourseNodeSpecification components (#126) Roam: Add PostHog user identification for enhanced analytics tracking using user's roam UID as the unique identifier - ENG-177 (#123) * add posthog identify * remove username and email to keep it anonymus * double userUid and best practice for js Roam: Discourse Context Overlay - remove queue and arbitrary delay (#127) * Refactor getOverlayInfo to use async/await and improve error handling. Update cache key from title to tag and remove overlayQueue logic for cleaner implementation. * Remove experimental getOverlayInfo function * Remove unused refreshUi logic [ENG-44] Display relations (#116) * instantiate new relationship worked * add display relations * remove dv * sm fix [ENG-198] Filtered out related file in search (#125) * filtered out related file * fix some naming [ENG-97] Use TailwindCSS in obsidian app (#128) * Update Obsidian app to integrate Tailwind CSS with PostCSS and Autoprefixer support - Added Tailwind CSS, PostCSS, and Autoprefixer to package dependencies - Configured styles.css to include Tailwind directives - Enhanced compile script to process styles using PostCSS with Tailwind and Autoprefixer * delete irrelevant package [ENG-192] Change all existing styles to using tw (#129) * Update Obsidian app to integrate Tailwind CSS with PostCSS and Autoprefixer support - Added Tailwind CSS, PostCSS, and Autoprefixer to package dependencies - Configured styles.css to include Tailwind directives - Enhanced compile script to process styles using PostCSS with Tailwind and Autoprefixer * delete irrelevant package * changing all styles to tailwindcss * Update Obsidian app to integrate Tailwind CSS with PostCSS and Autoprefixer support - Added Tailwind CSS, PostCSS, and Autoprefixer to package dependencies - Configured styles.css to include Tailwind directives - Enhanced compile script to process styles using PostCSS with Tailwind and Autoprefixer * delete irrelevant package * changing all styles to tailwindcss --------- Co-authored-by: Michael Gartner <[email protected]> Roam: Bug-fix: Don't let user create discourse nodes with empty text using node context menu - ENG-171 (#130) * functional covering all three cases tested locally * apply coderabbit review suggestion * better approach one that I understand and can reason about * accidental removal of onClose Update Roam app version to 0.13.0 in package.json and package-lock.json (#134) [ENG-204] Move from localStorage to extensionAPI.settings (#133) * cur progress * address PR comments * kinda works. need to test more * small fix * address PR comments . Create publish-obsidian.yml Update publish-obsidian.yml Update publish-obsidian.yml Update publish-obsidian.yml Enhance DiscourseContextOverlay: Update button styles to include loading state and improve score/ref display during loading. Use placeholders for score and refs when loading. (#136) . Update publish-obsidian.yml Update publish-obsidian.yml Update publish-obsidian.yml Update publish-obsidian.yml Refactor ExportDialog: Remove discourseGraphEnabled state and simplify FormGroup visibility logic. Set includeDiscourseContext to false by default. (#139) Enhance LabelDialog: Add confirmText to return object for improved button text handling based on action type. (#141) Additional styles / cursor rules (#142) * Update STYLE_GUIDE.md and main.mdc: Add guideline for utilizing utility functions for reusable logic and common operations. * Update STYLE_GUIDE.md and main.mdc: Add guideline to prefer arrow functions over regular function declarations. * Update main.mdc: Add guideline to prefer Tailwind classes when refactoring inline styles. * Update STYLE_GUIDE.md and main.mdc: Add guideline to prefer early returns over nested conditionals for improved readability. Roam: When a user deletes a node also delete all the corresponding relations to the node - ENG-26 (#149) * ask user for confirmation, delete corresponding relations * address review * address review * address comments [ENG-301] Create node in right-click menu (#152) * create node in right-click menu * small fix * address PR comments * address PR comments add readme and remove sample commands remove sample editor command rm space minor fixes Roam: Bug fix - Insert Discourse Node after creation (#154) * remove focus after menu select to allow updateBlock to work * add clarifying comment [ENG-308] Add command to open DG settings (#158) * add command to open DG settings * edit comment ENG-322 - Switch from MIT to Apache 2.0 license (#156) * Switch from MIT to Apache 2.0 license * copyright discourse graphs * rm liscense from apps/roam --------- Co-authored-by: Michael Gartner <[email protected]> initial port [ENG-207] Move Github sync setting to individual nodes (#124) * current progress * improve in UI: if sync is turned off then also turn off the comments configuration * address PR comments * revert graphOverviewUid bug * revert graphOverviewUid bug - getDiscourseNodes * avoid racing conditions for github sync * nested settings * temp fix to race condition * remove unecessary DOM and match existing styles --------- Co-authored-by: Michael Gartner <[email protected]> Eng 286 show when GitHub sync is disabled globally (#143) * Refactor GitHub Sync settings in NodeConfig and GeneralSettings components - Updated the onChange handler for GitHub Sync to use async/await and added a timeout for refreshing the config tree. - Introduced a global settings check in NodeConfig to conditionally render the GitHub Sync checkbox and comments configuration. - Passed setMainTab prop to NodeConfig for better navigation control. This improves the user experience by ensuring that settings are updated correctly and provides clear feedback when global settings are disabled. * matchingNode fix . Refactor Export components to use getSetting for consistent settings retrieval - Updated ExportDialog and ExportGithub components to replace localStorageGet with getSetting for fetching GitHub OAuth and repository settings. - Modified extensionSettings utility functions to use arrow functions and provide a default value for getSetting. - Improved code readability and maintainability by standardizing the method of accessing settings. Eng 286 show when GitHub sync is disabled globally (#143) * Refactor GitHub Sync settings in NodeConfig and GeneralSettings components - Updated the onChange handler for GitHub Sync to use async/await and added a timeout for refreshing the config tree. - Introduced a global settings check in NodeConfig to conditionally render the GitHub Sync checkbox and comments configuration. - Passed setMainTab prop to NodeConfig for better navigation control. This improves the user experience by ensuring that settings are updated correctly and provides clear feedback when global settings are disabled. * matchingNode fix .
mdroidian
pushed a commit
that referenced
this pull request
May 16, 2025
author Trang Doan <[email protected]> 1744314525 -0400 committer Michael Gartner <[email protected]> 1747354088 -0600 ENG-96 Create new relationship between nodes (#115) * instantiate new relationship worked * fix * address PR comments * fix bi-directional update issues * show only compatible node type options * small fix * breakdown the components. use datacore * working * address PR comments * improve search by only allowing compatible node results * . * rm dataview --------- Co-authored-by: Michael Gartner <[email protected]> Move llm-api endpoints to vercel serverless (#102) * testing gemini * move endgoint to website * open ai endpoint * added anthropic endpoint * pass env vars * add cors handdling and options * . * using centralised cors middleware * only adding bypass cookie * use right key * remove the bypass token requirement * sanitize, fix routes * remove server action config * DRY * remove unused * address review * adress review Roam: Add feedback toggle (#118) * add settings to hide or show button, also works when disabled or enabled midway * review * . --------- Co-authored-by: Michael Gartner <[email protected]> [ENG-197] Fix creating link with invalid chars (#121) * fix creating link with invalid chars * placeholder update --------- Co-authored-by: Michael Gartner <[email protected]> Roam: Add feedback button to settings menu - ENG-147 (#122) * add button to bottom right, don't hide sdk css, tested * remove intent not working git * remove ts-ignore and use a better type def * remove styling Update NodeConfig to use new UIDs for DiscourseNodeIndex and DiscourseNodeSpecification components (#126) Roam: Add PostHog user identification for enhanced analytics tracking using user's roam UID as the unique identifier - ENG-177 (#123) * add posthog identify * remove username and email to keep it anonymus * double userUid and best practice for js Roam: Discourse Context Overlay - remove queue and arbitrary delay (#127) * Refactor getOverlayInfo to use async/await and improve error handling. Update cache key from title to tag and remove overlayQueue logic for cleaner implementation. * Remove experimental getOverlayInfo function * Remove unused refreshUi logic [ENG-44] Display relations (#116) * instantiate new relationship worked * add display relations * remove dv * sm fix [ENG-198] Filtered out related file in search (#125) * filtered out related file * fix some naming [ENG-97] Use TailwindCSS in obsidian app (#128) * Update Obsidian app to integrate Tailwind CSS with PostCSS and Autoprefixer support - Added Tailwind CSS, PostCSS, and Autoprefixer to package dependencies - Configured styles.css to include Tailwind directives - Enhanced compile script to process styles using PostCSS with Tailwind and Autoprefixer * delete irrelevant package [ENG-192] Change all existing styles to using tw (#129) * Update Obsidian app to integrate Tailwind CSS with PostCSS and Autoprefixer support - Added Tailwind CSS, PostCSS, and Autoprefixer to package dependencies - Configured styles.css to include Tailwind directives - Enhanced compile script to process styles using PostCSS with Tailwind and Autoprefixer * delete irrelevant package * changing all styles to tailwindcss * Update Obsidian app to integrate Tailwind CSS with PostCSS and Autoprefixer support - Added Tailwind CSS, PostCSS, and Autoprefixer to package dependencies - Configured styles.css to include Tailwind directives - Enhanced compile script to process styles using PostCSS with Tailwind and Autoprefixer * delete irrelevant package * changing all styles to tailwindcss --------- Co-authored-by: Michael Gartner <[email protected]> Roam: Bug-fix: Don't let user create discourse nodes with empty text using node context menu - ENG-171 (#130) * functional covering all three cases tested locally * apply coderabbit review suggestion * better approach one that I understand and can reason about * accidental removal of onClose Update Roam app version to 0.13.0 in package.json and package-lock.json (#134) [ENG-204] Move from localStorage to extensionAPI.settings (#133) * cur progress * address PR comments * kinda works. need to test more * small fix * address PR comments . Create publish-obsidian.yml Update publish-obsidian.yml Update publish-obsidian.yml Update publish-obsidian.yml Enhance DiscourseContextOverlay: Update button styles to include loading state and improve score/ref display during loading. Use placeholders for score and refs when loading. (#136) . Update publish-obsidian.yml Update publish-obsidian.yml Update publish-obsidian.yml Update publish-obsidian.yml Refactor ExportDialog: Remove discourseGraphEnabled state and simplify FormGroup visibility logic. Set includeDiscourseContext to false by default. (#139) Enhance LabelDialog: Add confirmText to return object for improved button text handling based on action type. (#141) Additional styles / cursor rules (#142) * Update STYLE_GUIDE.md and main.mdc: Add guideline for utilizing utility functions for reusable logic and common operations. * Update STYLE_GUIDE.md and main.mdc: Add guideline to prefer arrow functions over regular function declarations. * Update main.mdc: Add guideline to prefer Tailwind classes when refactoring inline styles. * Update STYLE_GUIDE.md and main.mdc: Add guideline to prefer early returns over nested conditionals for improved readability. Roam: When a user deletes a node also delete all the corresponding relations to the node - ENG-26 (#149) * ask user for confirmation, delete corresponding relations * address review * address review * address comments [ENG-301] Create node in right-click menu (#152) * create node in right-click menu * small fix * address PR comments * address PR comments add readme and remove sample commands remove sample editor command rm space minor fixes Roam: Bug fix - Insert Discourse Node after creation (#154) * remove focus after menu select to allow updateBlock to work * add clarifying comment [ENG-308] Add command to open DG settings (#158) * add command to open DG settings * edit comment ENG-322 - Switch from MIT to Apache 2.0 license (#156) * Switch from MIT to Apache 2.0 license * copyright discourse graphs * rm liscense from apps/roam --------- Co-authored-by: Michael Gartner <[email protected]> initial port [ENG-207] Move Github sync setting to individual nodes (#124) * current progress * improve in UI: if sync is turned off then also turn off the comments configuration * address PR comments * revert graphOverviewUid bug * revert graphOverviewUid bug - getDiscourseNodes * avoid racing conditions for github sync * nested settings * temp fix to race condition * remove unecessary DOM and match existing styles --------- Co-authored-by: Michael Gartner <[email protected]> Eng 286 show when GitHub sync is disabled globally (#143) * Refactor GitHub Sync settings in NodeConfig and GeneralSettings components - Updated the onChange handler for GitHub Sync to use async/await and added a timeout for refreshing the config tree. - Introduced a global settings check in NodeConfig to conditionally render the GitHub Sync checkbox and comments configuration. - Passed setMainTab prop to NodeConfig for better navigation control. This improves the user experience by ensuring that settings are updated correctly and provides clear feedback when global settings are disabled. * matchingNode fix . Refactor Export components to use getSetting for consistent settings retrieval - Updated ExportDialog and ExportGithub components to replace localStorageGet with getSetting for fetching GitHub OAuth and repository settings. - Modified extensionSettings utility functions to use arrow functions and provide a default value for getSetting. - Improved code readability and maintainability by standardizing the method of accessing settings. Eng 286 show when GitHub sync is disabled globally (#143) * Refactor GitHub Sync settings in NodeConfig and GeneralSettings components - Updated the onChange handler for GitHub Sync to use async/await and added a timeout for refreshing the config tree. - Introduced a global settings check in NodeConfig to conditionally render the GitHub Sync checkbox and comments configuration. - Passed setMainTab prop to NodeConfig for better navigation control. This improves the user experience by ensuring that settings are updated correctly and provides clear feedback when global settings are disabled. * matchingNode fix .
mdroidian
pushed a commit
that referenced
this pull request
Jun 3, 2025
author Trang Doan <[email protected]> 1744314525 -0400 committer Michael Gartner <[email protected]> 1747354088 -0600 ENG-96 Create new relationship between nodes (#115) * instantiate new relationship worked * fix * address PR comments * fix bi-directional update issues * show only compatible node type options * small fix * breakdown the components. use datacore * working * address PR comments * improve search by only allowing compatible node results * . * rm dataview --------- Co-authored-by: Michael Gartner <[email protected]> Move llm-api endpoints to vercel serverless (#102) * testing gemini * move endgoint to website * open ai endpoint * added anthropic endpoint * pass env vars * add cors handdling and options * . * using centralised cors middleware * only adding bypass cookie * use right key * remove the bypass token requirement * sanitize, fix routes * remove server action config * DRY * remove unused * address review * adress review Roam: Add feedback toggle (#118) * add settings to hide or show button, also works when disabled or enabled midway * review * . --------- Co-authored-by: Michael Gartner <[email protected]> [ENG-197] Fix creating link with invalid chars (#121) * fix creating link with invalid chars * placeholder update --------- Co-authored-by: Michael Gartner <[email protected]> Roam: Add feedback button to settings menu - ENG-147 (#122) * add button to bottom right, don't hide sdk css, tested * remove intent not working git * remove ts-ignore and use a better type def * remove styling Update NodeConfig to use new UIDs for DiscourseNodeIndex and DiscourseNodeSpecification components (#126) Roam: Add PostHog user identification for enhanced analytics tracking using user's roam UID as the unique identifier - ENG-177 (#123) * add posthog identify * remove username and email to keep it anonymus * double userUid and best practice for js Roam: Discourse Context Overlay - remove queue and arbitrary delay (#127) * Refactor getOverlayInfo to use async/await and improve error handling. Update cache key from title to tag and remove overlayQueue logic for cleaner implementation. * Remove experimental getOverlayInfo function * Remove unused refreshUi logic [ENG-44] Display relations (#116) * instantiate new relationship worked * add display relations * remove dv * sm fix [ENG-198] Filtered out related file in search (#125) * filtered out related file * fix some naming [ENG-97] Use TailwindCSS in obsidian app (#128) * Update Obsidian app to integrate Tailwind CSS with PostCSS and Autoprefixer support - Added Tailwind CSS, PostCSS, and Autoprefixer to package dependencies - Configured styles.css to include Tailwind directives - Enhanced compile script to process styles using PostCSS with Tailwind and Autoprefixer * delete irrelevant package [ENG-192] Change all existing styles to using tw (#129) * Update Obsidian app to integrate Tailwind CSS with PostCSS and Autoprefixer support - Added Tailwind CSS, PostCSS, and Autoprefixer to package dependencies - Configured styles.css to include Tailwind directives - Enhanced compile script to process styles using PostCSS with Tailwind and Autoprefixer * delete irrelevant package * changing all styles to tailwindcss * Update Obsidian app to integrate Tailwind CSS with PostCSS and Autoprefixer support - Added Tailwind CSS, PostCSS, and Autoprefixer to package dependencies - Configured styles.css to include Tailwind directives - Enhanced compile script to process styles using PostCSS with Tailwind and Autoprefixer * delete irrelevant package * changing all styles to tailwindcss --------- Co-authored-by: Michael Gartner <[email protected]> Roam: Bug-fix: Don't let user create discourse nodes with empty text using node context menu - ENG-171 (#130) * functional covering all three cases tested locally * apply coderabbit review suggestion * better approach one that I understand and can reason about * accidental removal of onClose Update Roam app version to 0.13.0 in package.json and package-lock.json (#134) [ENG-204] Move from localStorage to extensionAPI.settings (#133) * cur progress * address PR comments * kinda works. need to test more * small fix * address PR comments . Create publish-obsidian.yml Update publish-obsidian.yml Update publish-obsidian.yml Update publish-obsidian.yml Enhance DiscourseContextOverlay: Update button styles to include loading state and improve score/ref display during loading. Use placeholders for score and refs when loading. (#136) . Update publish-obsidian.yml Update publish-obsidian.yml Update publish-obsidian.yml Update publish-obsidian.yml Refactor ExportDialog: Remove discourseGraphEnabled state and simplify FormGroup visibility logic. Set includeDiscourseContext to false by default. (#139) Enhance LabelDialog: Add confirmText to return object for improved button text handling based on action type. (#141) Additional styles / cursor rules (#142) * Update STYLE_GUIDE.md and main.mdc: Add guideline for utilizing utility functions for reusable logic and common operations. * Update STYLE_GUIDE.md and main.mdc: Add guideline to prefer arrow functions over regular function declarations. * Update main.mdc: Add guideline to prefer Tailwind classes when refactoring inline styles. * Update STYLE_GUIDE.md and main.mdc: Add guideline to prefer early returns over nested conditionals for improved readability. Roam: When a user deletes a node also delete all the corresponding relations to the node - ENG-26 (#149) * ask user for confirmation, delete corresponding relations * address review * address review * address comments [ENG-301] Create node in right-click menu (#152) * create node in right-click menu * small fix * address PR comments * address PR comments add readme and remove sample commands remove sample editor command rm space minor fixes Roam: Bug fix - Insert Discourse Node after creation (#154) * remove focus after menu select to allow updateBlock to work * add clarifying comment [ENG-308] Add command to open DG settings (#158) * add command to open DG settings * edit comment ENG-322 - Switch from MIT to Apache 2.0 license (#156) * Switch from MIT to Apache 2.0 license * copyright discourse graphs * rm liscense from apps/roam --------- Co-authored-by: Michael Gartner <[email protected]> initial port [ENG-207] Move Github sync setting to individual nodes (#124) * current progress * improve in UI: if sync is turned off then also turn off the comments configuration * address PR comments * revert graphOverviewUid bug * revert graphOverviewUid bug - getDiscourseNodes * avoid racing conditions for github sync * nested settings * temp fix to race condition * remove unecessary DOM and match existing styles --------- Co-authored-by: Michael Gartner <[email protected]> Eng 286 show when GitHub sync is disabled globally (#143) * Refactor GitHub Sync settings in NodeConfig and GeneralSettings components - Updated the onChange handler for GitHub Sync to use async/await and added a timeout for refreshing the config tree. - Introduced a global settings check in NodeConfig to conditionally render the GitHub Sync checkbox and comments configuration. - Passed setMainTab prop to NodeConfig for better navigation control. This improves the user experience by ensuring that settings are updated correctly and provides clear feedback when global settings are disabled. * matchingNode fix . Refactor Export components to use getSetting for consistent settings retrieval - Updated ExportDialog and ExportGithub components to replace localStorageGet with getSetting for fetching GitHub OAuth and repository settings. - Modified extensionSettings utility functions to use arrow functions and provide a default value for getSetting. - Improved code readability and maintainability by standardizing the method of accessing settings. Eng 286 show when GitHub sync is disabled globally (#143) * Refactor GitHub Sync settings in NodeConfig and GeneralSettings components - Updated the onChange handler for GitHub Sync to use async/await and added a timeout for refreshing the config tree. - Introduced a global settings check in NodeConfig to conditionally render the GitHub Sync checkbox and comments configuration. - Passed setMainTab prop to NodeConfig for better navigation control. This improves the user experience by ensuring that settings are updated correctly and provides clear feedback when global settings are disabled. * matchingNode fix .
mdroidian
pushed a commit
that referenced
this pull request
Jun 9, 2025
author Trang Doan <[email protected]> 1744314525 -0400 committer Michael Gartner <[email protected]> 1747354088 -0600 ENG-96 Create new relationship between nodes (#115) * instantiate new relationship worked * fix * address PR comments * fix bi-directional update issues * show only compatible node type options * small fix * breakdown the components. use datacore * working * address PR comments * improve search by only allowing compatible node results * . * rm dataview --------- Co-authored-by: Michael Gartner <[email protected]> Move llm-api endpoints to vercel serverless (#102) * testing gemini * move endgoint to website * open ai endpoint * added anthropic endpoint * pass env vars * add cors handdling and options * . * using centralised cors middleware * only adding bypass cookie * use right key * remove the bypass token requirement * sanitize, fix routes * remove server action config * DRY * remove unused * address review * adress review Roam: Add feedback toggle (#118) * add settings to hide or show button, also works when disabled or enabled midway * review * . --------- Co-authored-by: Michael Gartner <[email protected]> [ENG-197] Fix creating link with invalid chars (#121) * fix creating link with invalid chars * placeholder update --------- Co-authored-by: Michael Gartner <[email protected]> Roam: Add feedback button to settings menu - ENG-147 (#122) * add button to bottom right, don't hide sdk css, tested * remove intent not working git * remove ts-ignore and use a better type def * remove styling Update NodeConfig to use new UIDs for DiscourseNodeIndex and DiscourseNodeSpecification components (#126) Roam: Add PostHog user identification for enhanced analytics tracking using user's roam UID as the unique identifier - ENG-177 (#123) * add posthog identify * remove username and email to keep it anonymus * double userUid and best practice for js Roam: Discourse Context Overlay - remove queue and arbitrary delay (#127) * Refactor getOverlayInfo to use async/await and improve error handling. Update cache key from title to tag and remove overlayQueue logic for cleaner implementation. * Remove experimental getOverlayInfo function * Remove unused refreshUi logic [ENG-44] Display relations (#116) * instantiate new relationship worked * add display relations * remove dv * sm fix [ENG-198] Filtered out related file in search (#125) * filtered out related file * fix some naming [ENG-97] Use TailwindCSS in obsidian app (#128) * Update Obsidian app to integrate Tailwind CSS with PostCSS and Autoprefixer support - Added Tailwind CSS, PostCSS, and Autoprefixer to package dependencies - Configured styles.css to include Tailwind directives - Enhanced compile script to process styles using PostCSS with Tailwind and Autoprefixer * delete irrelevant package [ENG-192] Change all existing styles to using tw (#129) * Update Obsidian app to integrate Tailwind CSS with PostCSS and Autoprefixer support - Added Tailwind CSS, PostCSS, and Autoprefixer to package dependencies - Configured styles.css to include Tailwind directives - Enhanced compile script to process styles using PostCSS with Tailwind and Autoprefixer * delete irrelevant package * changing all styles to tailwindcss * Update Obsidian app to integrate Tailwind CSS with PostCSS and Autoprefixer support - Added Tailwind CSS, PostCSS, and Autoprefixer to package dependencies - Configured styles.css to include Tailwind directives - Enhanced compile script to process styles using PostCSS with Tailwind and Autoprefixer * delete irrelevant package * changing all styles to tailwindcss --------- Co-authored-by: Michael Gartner <[email protected]> Roam: Bug-fix: Don't let user create discourse nodes with empty text using node context menu - ENG-171 (#130) * functional covering all three cases tested locally * apply coderabbit review suggestion * better approach one that I understand and can reason about * accidental removal of onClose Update Roam app version to 0.13.0 in package.json and package-lock.json (#134) [ENG-204] Move from localStorage to extensionAPI.settings (#133) * cur progress * address PR comments * kinda works. need to test more * small fix * address PR comments . Create publish-obsidian.yml Update publish-obsidian.yml Update publish-obsidian.yml Update publish-obsidian.yml Enhance DiscourseContextOverlay: Update button styles to include loading state and improve score/ref display during loading. Use placeholders for score and refs when loading. (#136) . Update publish-obsidian.yml Update publish-obsidian.yml Update publish-obsidian.yml Update publish-obsidian.yml Refactor ExportDialog: Remove discourseGraphEnabled state and simplify FormGroup visibility logic. Set includeDiscourseContext to false by default. (#139) Enhance LabelDialog: Add confirmText to return object for improved button text handling based on action type. (#141) Additional styles / cursor rules (#142) * Update STYLE_GUIDE.md and main.mdc: Add guideline for utilizing utility functions for reusable logic and common operations. * Update STYLE_GUIDE.md and main.mdc: Add guideline to prefer arrow functions over regular function declarations. * Update main.mdc: Add guideline to prefer Tailwind classes when refactoring inline styles. * Update STYLE_GUIDE.md and main.mdc: Add guideline to prefer early returns over nested conditionals for improved readability. Roam: When a user deletes a node also delete all the corresponding relations to the node - ENG-26 (#149) * ask user for confirmation, delete corresponding relations * address review * address review * address comments [ENG-301] Create node in right-click menu (#152) * create node in right-click menu * small fix * address PR comments * address PR comments add readme and remove sample commands remove sample editor command rm space minor fixes Roam: Bug fix - Insert Discourse Node after creation (#154) * remove focus after menu select to allow updateBlock to work * add clarifying comment [ENG-308] Add command to open DG settings (#158) * add command to open DG settings * edit comment ENG-322 - Switch from MIT to Apache 2.0 license (#156) * Switch from MIT to Apache 2.0 license * copyright discourse graphs * rm liscense from apps/roam --------- Co-authored-by: Michael Gartner <[email protected]> initial port [ENG-207] Move Github sync setting to individual nodes (#124) * current progress * improve in UI: if sync is turned off then also turn off the comments configuration * address PR comments * revert graphOverviewUid bug * revert graphOverviewUid bug - getDiscourseNodes * avoid racing conditions for github sync * nested settings * temp fix to race condition * remove unecessary DOM and match existing styles --------- Co-authored-by: Michael Gartner <[email protected]> Eng 286 show when GitHub sync is disabled globally (#143) * Refactor GitHub Sync settings in NodeConfig and GeneralSettings components - Updated the onChange handler for GitHub Sync to use async/await and added a timeout for refreshing the config tree. - Introduced a global settings check in NodeConfig to conditionally render the GitHub Sync checkbox and comments configuration. - Passed setMainTab prop to NodeConfig for better navigation control. This improves the user experience by ensuring that settings are updated correctly and provides clear feedback when global settings are disabled. * matchingNode fix . Refactor Export components to use getSetting for consistent settings retrieval - Updated ExportDialog and ExportGithub components to replace localStorageGet with getSetting for fetching GitHub OAuth and repository settings. - Modified extensionSettings utility functions to use arrow functions and provide a default value for getSetting. - Improved code readability and maintainability by standardizing the method of accessing settings. Eng 286 show when GitHub sync is disabled globally (#143) * Refactor GitHub Sync settings in NodeConfig and GeneralSettings components - Updated the onChange handler for GitHub Sync to use async/await and added a timeout for refreshing the config tree. - Introduced a global settings check in NodeConfig to conditionally render the GitHub Sync checkbox and comments configuration. - Passed setMainTab prop to NodeConfig for better navigation control. This improves the user experience by ensuring that settings are updated correctly and provides clear feedback when global settings are disabled. * matchingNode fix .
mdroidian
pushed a commit
that referenced
this pull request
Jun 14, 2025
author Trang Doan <[email protected]> 1744314525 -0400 committer Michael Gartner <[email protected]> 1747354088 -0600 ENG-96 Create new relationship between nodes (#115) * instantiate new relationship worked * fix * address PR comments * fix bi-directional update issues * show only compatible node type options * small fix * breakdown the components. use datacore * working * address PR comments * improve search by only allowing compatible node results * . * rm dataview --------- Co-authored-by: Michael Gartner <[email protected]> Move llm-api endpoints to vercel serverless (#102) * testing gemini * move endgoint to website * open ai endpoint * added anthropic endpoint * pass env vars * add cors handdling and options * . * using centralised cors middleware * only adding bypass cookie * use right key * remove the bypass token requirement * sanitize, fix routes * remove server action config * DRY * remove unused * address review * adress review Roam: Add feedback toggle (#118) * add settings to hide or show button, also works when disabled or enabled midway * review * . --------- Co-authored-by: Michael Gartner <[email protected]> [ENG-197] Fix creating link with invalid chars (#121) * fix creating link with invalid chars * placeholder update --------- Co-authored-by: Michael Gartner <[email protected]> Roam: Add feedback button to settings menu - ENG-147 (#122) * add button to bottom right, don't hide sdk css, tested * remove intent not working git * remove ts-ignore and use a better type def * remove styling Update NodeConfig to use new UIDs for DiscourseNodeIndex and DiscourseNodeSpecification components (#126) Roam: Add PostHog user identification for enhanced analytics tracking using user's roam UID as the unique identifier - ENG-177 (#123) * add posthog identify * remove username and email to keep it anonymus * double userUid and best practice for js Roam: Discourse Context Overlay - remove queue and arbitrary delay (#127) * Refactor getOverlayInfo to use async/await and improve error handling. Update cache key from title to tag and remove overlayQueue logic for cleaner implementation. * Remove experimental getOverlayInfo function * Remove unused refreshUi logic [ENG-44] Display relations (#116) * instantiate new relationship worked * add display relations * remove dv * sm fix [ENG-198] Filtered out related file in search (#125) * filtered out related file * fix some naming [ENG-97] Use TailwindCSS in obsidian app (#128) * Update Obsidian app to integrate Tailwind CSS with PostCSS and Autoprefixer support - Added Tailwind CSS, PostCSS, and Autoprefixer to package dependencies - Configured styles.css to include Tailwind directives - Enhanced compile script to process styles using PostCSS with Tailwind and Autoprefixer * delete irrelevant package [ENG-192] Change all existing styles to using tw (#129) * Update Obsidian app to integrate Tailwind CSS with PostCSS and Autoprefixer support - Added Tailwind CSS, PostCSS, and Autoprefixer to package dependencies - Configured styles.css to include Tailwind directives - Enhanced compile script to process styles using PostCSS with Tailwind and Autoprefixer * delete irrelevant package * changing all styles to tailwindcss * Update Obsidian app to integrate Tailwind CSS with PostCSS and Autoprefixer support - Added Tailwind CSS, PostCSS, and Autoprefixer to package dependencies - Configured styles.css to include Tailwind directives - Enhanced compile script to process styles using PostCSS with Tailwind and Autoprefixer * delete irrelevant package * changing all styles to tailwindcss --------- Co-authored-by: Michael Gartner <[email protected]> Roam: Bug-fix: Don't let user create discourse nodes with empty text using node context menu - ENG-171 (#130) * functional covering all three cases tested locally * apply coderabbit review suggestion * better approach one that I understand and can reason about * accidental removal of onClose Update Roam app version to 0.13.0 in package.json and package-lock.json (#134) [ENG-204] Move from localStorage to extensionAPI.settings (#133) * cur progress * address PR comments * kinda works. need to test more * small fix * address PR comments . Create publish-obsidian.yml Update publish-obsidian.yml Update publish-obsidian.yml Update publish-obsidian.yml Enhance DiscourseContextOverlay: Update button styles to include loading state and improve score/ref display during loading. Use placeholders for score and refs when loading. (#136) . Update publish-obsidian.yml Update publish-obsidian.yml Update publish-obsidian.yml Update publish-obsidian.yml Refactor ExportDialog: Remove discourseGraphEnabled state and simplify FormGroup visibility logic. Set includeDiscourseContext to false by default. (#139) Enhance LabelDialog: Add confirmText to return object for improved button text handling based on action type. (#141) Additional styles / cursor rules (#142) * Update STYLE_GUIDE.md and main.mdc: Add guideline for utilizing utility functions for reusable logic and common operations. * Update STYLE_GUIDE.md and main.mdc: Add guideline to prefer arrow functions over regular function declarations. * Update main.mdc: Add guideline to prefer Tailwind classes when refactoring inline styles. * Update STYLE_GUIDE.md and main.mdc: Add guideline to prefer early returns over nested conditionals for improved readability. Roam: When a user deletes a node also delete all the corresponding relations to the node - ENG-26 (#149) * ask user for confirmation, delete corresponding relations * address review * address review * address comments [ENG-301] Create node in right-click menu (#152) * create node in right-click menu * small fix * address PR comments * address PR comments add readme and remove sample commands remove sample editor command rm space minor fixes Roam: Bug fix - Insert Discourse Node after creation (#154) * remove focus after menu select to allow updateBlock to work * add clarifying comment [ENG-308] Add command to open DG settings (#158) * add command to open DG settings * edit comment ENG-322 - Switch from MIT to Apache 2.0 license (#156) * Switch from MIT to Apache 2.0 license * copyright discourse graphs * rm liscense from apps/roam --------- Co-authored-by: Michael Gartner <[email protected]> initial port [ENG-207] Move Github sync setting to individual nodes (#124) * current progress * improve in UI: if sync is turned off then also turn off the comments configuration * address PR comments * revert graphOverviewUid bug * revert graphOverviewUid bug - getDiscourseNodes * avoid racing conditions for github sync * nested settings * temp fix to race condition * remove unecessary DOM and match existing styles --------- Co-authored-by: Michael Gartner <[email protected]> Eng 286 show when GitHub sync is disabled globally (#143) * Refactor GitHub Sync settings in NodeConfig and GeneralSettings components - Updated the onChange handler for GitHub Sync to use async/await and added a timeout for refreshing the config tree. - Introduced a global settings check in NodeConfig to conditionally render the GitHub Sync checkbox and comments configuration. - Passed setMainTab prop to NodeConfig for better navigation control. This improves the user experience by ensuring that settings are updated correctly and provides clear feedback when global settings are disabled. * matchingNode fix . Refactor Export components to use getSetting for consistent settings retrieval - Updated ExportDialog and ExportGithub components to replace localStorageGet with getSetting for fetching GitHub OAuth and repository settings. - Modified extensionSettings utility functions to use arrow functions and provide a default value for getSetting. - Improved code readability and maintainability by standardizing the method of accessing settings. Eng 286 show when GitHub sync is disabled globally (#143) * Refactor GitHub Sync settings in NodeConfig and GeneralSettings components - Updated the onChange handler for GitHub Sync to use async/await and added a timeout for refreshing the config tree. - Introduced a global settings check in NodeConfig to conditionally render the GitHub Sync checkbox and comments configuration. - Passed setMainTab prop to NodeConfig for better navigation control. This improves the user experience by ensuring that settings are updated correctly and provides clear feedback when global settings are disabled. * matchingNode fix .
mdroidian
pushed a commit
that referenced
this pull request
Jun 16, 2025
author Trang Doan <[email protected]> 1744314525 -0400 committer Michael Gartner <[email protected]> 1747354088 -0600 ENG-96 Create new relationship between nodes (#115) * instantiate new relationship worked * fix * address PR comments * fix bi-directional update issues * show only compatible node type options * small fix * breakdown the components. use datacore * working * address PR comments * improve search by only allowing compatible node results * . * rm dataview --------- Co-authored-by: Michael Gartner <[email protected]> Move llm-api endpoints to vercel serverless (#102) * testing gemini * move endgoint to website * open ai endpoint * added anthropic endpoint * pass env vars * add cors handdling and options * . * using centralised cors middleware * only adding bypass cookie * use right key * remove the bypass token requirement * sanitize, fix routes * remove server action config * DRY * remove unused * address review * adress review Roam: Add feedback toggle (#118) * add settings to hide or show button, also works when disabled or enabled midway * review * . --------- Co-authored-by: Michael Gartner <[email protected]> [ENG-197] Fix creating link with invalid chars (#121) * fix creating link with invalid chars * placeholder update --------- Co-authored-by: Michael Gartner <[email protected]> Roam: Add feedback button to settings menu - ENG-147 (#122) * add button to bottom right, don't hide sdk css, tested * remove intent not working git * remove ts-ignore and use a better type def * remove styling Update NodeConfig to use new UIDs for DiscourseNodeIndex and DiscourseNodeSpecification components (#126) Roam: Add PostHog user identification for enhanced analytics tracking using user's roam UID as the unique identifier - ENG-177 (#123) * add posthog identify * remove username and email to keep it anonymus * double userUid and best practice for js Roam: Discourse Context Overlay - remove queue and arbitrary delay (#127) * Refactor getOverlayInfo to use async/await and improve error handling. Update cache key from title to tag and remove overlayQueue logic for cleaner implementation. * Remove experimental getOverlayInfo function * Remove unused refreshUi logic [ENG-44] Display relations (#116) * instantiate new relationship worked * add display relations * remove dv * sm fix [ENG-198] Filtered out related file in search (#125) * filtered out related file * fix some naming [ENG-97] Use TailwindCSS in obsidian app (#128) * Update Obsidian app to integrate Tailwind CSS with PostCSS and Autoprefixer support - Added Tailwind CSS, PostCSS, and Autoprefixer to package dependencies - Configured styles.css to include Tailwind directives - Enhanced compile script to process styles using PostCSS with Tailwind and Autoprefixer * delete irrelevant package [ENG-192] Change all existing styles to using tw (#129) * Update Obsidian app to integrate Tailwind CSS with PostCSS and Autoprefixer support - Added Tailwind CSS, PostCSS, and Autoprefixer to package dependencies - Configured styles.css to include Tailwind directives - Enhanced compile script to process styles using PostCSS with Tailwind and Autoprefixer * delete irrelevant package * changing all styles to tailwindcss * Update Obsidian app to integrate Tailwind CSS with PostCSS and Autoprefixer support - Added Tailwind CSS, PostCSS, and Autoprefixer to package dependencies - Configured styles.css to include Tailwind directives - Enhanced compile script to process styles using PostCSS with Tailwind and Autoprefixer * delete irrelevant package * changing all styles to tailwindcss --------- Co-authored-by: Michael Gartner <[email protected]> Roam: Bug-fix: Don't let user create discourse nodes with empty text using node context menu - ENG-171 (#130) * functional covering all three cases tested locally * apply coderabbit review suggestion * better approach one that I understand and can reason about * accidental removal of onClose Update Roam app version to 0.13.0 in package.json and package-lock.json (#134) [ENG-204] Move from localStorage to extensionAPI.settings (#133) * cur progress * address PR comments * kinda works. need to test more * small fix * address PR comments . Create publish-obsidian.yml Update publish-obsidian.yml Update publish-obsidian.yml Update publish-obsidian.yml Enhance DiscourseContextOverlay: Update button styles to include loading state and improve score/ref display during loading. Use placeholders for score and refs when loading. (#136) . Update publish-obsidian.yml Update publish-obsidian.yml Update publish-obsidian.yml Update publish-obsidian.yml Refactor ExportDialog: Remove discourseGraphEnabled state and simplify FormGroup visibility logic. Set includeDiscourseContext to false by default. (#139) Enhance LabelDialog: Add confirmText to return object for improved button text handling based on action type. (#141) Additional styles / cursor rules (#142) * Update STYLE_GUIDE.md and main.mdc: Add guideline for utilizing utility functions for reusable logic and common operations. * Update STYLE_GUIDE.md and main.mdc: Add guideline to prefer arrow functions over regular function declarations. * Update main.mdc: Add guideline to prefer Tailwind classes when refactoring inline styles. * Update STYLE_GUIDE.md and main.mdc: Add guideline to prefer early returns over nested conditionals for improved readability. Roam: When a user deletes a node also delete all the corresponding relations to the node - ENG-26 (#149) * ask user for confirmation, delete corresponding relations * address review * address review * address comments [ENG-301] Create node in right-click menu (#152) * create node in right-click menu * small fix * address PR comments * address PR comments add readme and remove sample commands remove sample editor command rm space minor fixes Roam: Bug fix - Insert Discourse Node after creation (#154) * remove focus after menu select to allow updateBlock to work * add clarifying comment [ENG-308] Add command to open DG settings (#158) * add command to open DG settings * edit comment ENG-322 - Switch from MIT to Apache 2.0 license (#156) * Switch from MIT to Apache 2.0 license * copyright discourse graphs * rm liscense from apps/roam --------- Co-authored-by: Michael Gartner <[email protected]> initial port [ENG-207] Move Github sync setting to individual nodes (#124) * current progress * improve in UI: if sync is turned off then also turn off the comments configuration * address PR comments * revert graphOverviewUid bug * revert graphOverviewUid bug - getDiscourseNodes * avoid racing conditions for github sync * nested settings * temp fix to race condition * remove unecessary DOM and match existing styles --------- Co-authored-by: Michael Gartner <[email protected]> Eng 286 show when GitHub sync is disabled globally (#143) * Refactor GitHub Sync settings in NodeConfig and GeneralSettings components - Updated the onChange handler for GitHub Sync to use async/await and added a timeout for refreshing the config tree. - Introduced a global settings check in NodeConfig to conditionally render the GitHub Sync checkbox and comments configuration. - Passed setMainTab prop to NodeConfig for better navigation control. This improves the user experience by ensuring that settings are updated correctly and provides clear feedback when global settings are disabled. * matchingNode fix . Refactor Export components to use getSetting for consistent settings retrieval - Updated ExportDialog and ExportGithub components to replace localStorageGet with getSetting for fetching GitHub OAuth and repository settings. - Modified extensionSettings utility functions to use arrow functions and provide a default value for getSetting. - Improved code readability and maintainability by standardizing the method of accessing settings. Eng 286 show when GitHub sync is disabled globally (#143) * Refactor GitHub Sync settings in NodeConfig and GeneralSettings components - Updated the onChange handler for GitHub Sync to use async/await and added a timeout for refreshing the config tree. - Introduced a global settings check in NodeConfig to conditionally render the GitHub Sync checkbox and comments configuration. - Passed setMainTab prop to NodeConfig for better navigation control. This improves the user experience by ensuring that settings are updated correctly and provides clear feedback when global settings are disabled. * matchingNode fix .
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
https://www.loom.com/share/ca568a6dd6784cacbdc001f59d4038d8
Summary by CodeRabbit