[ENG-1025] Integrate subtle controls css styling into plugin #540
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
📝 WalkthroughWalkthroughThe pull request adds a streamline styling feature toggle to the Roam application. A new setting key is introduced in userSettings, a checkbox is added to HomePersonalSettings for users to enable/disable the feature, and conditional logic in index.ts loads the styles only when enabled. A new stylesheet module exports the streamline styling CSS. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Settings as HomePersonalSettings
participant Store as Settings Store
participant Index as index.ts
participant DOM
rect rgb(240, 248, 255)
Note over User,DOM: Application Initialization
Index->>Store: getSetting(STREAMLINE_STYLING_KEY)
Store-->>Index: setting value (true/false)
alt Setting Enabled
Index->>Index: Create streamlineStyleElement
Index->>DOM: Append style element
end
end
rect rgb(240, 248, 255)
Note over User,DOM: User Toggles Feature
User->>Settings: Click streamline checkbox
Settings->>Store: setSetting(STREAMLINE_STYLING_KEY, checked)
Store-->>Settings: Setting persisted
alt Checked
Settings->>Settings: addStyle(streamlineStyling)
Settings->>DOM: Inject style#streamline-styling
else Unchecked
Settings->>DOM: Remove style#streamline-styling
end
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Pre-merge checks✅ Passed checks (3 passed)
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: 2
🧹 Nitpick comments (2)
apps/roam/src/styles/streamlineStyling.ts (2)
36-36: Minor: Remove extra space before semicolon.There's an extra space before the semicolon on this line.
Apply this diff:
- border: 1px solid rgba(171, 171, 171, 0.5) ; + border: 1px solid rgba(171, 171, 171, 0.5);
1-98: Consider theme compatibility for future iterations.The CSS uses hardcoded colors (grays, whites) that may not adapt to different Roam themes or dark mode. While acceptable for an opt-in feature, consider using CSS variables or theme-aware colors in future updates to improve compatibility.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/roam/src/components/settings/HomePersonalSettings.tsx(3 hunks)apps/roam/src/data/userSettings.ts(1 hunks)apps/roam/src/index.ts(4 hunks)apps/roam/src/styles/streamlineStyling.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/main.mdc)
**/*.{ts,tsx}: Prefertypeoverinterface
Use explicit return types for functions
Avoidanytypes when possible
Files:
apps/roam/src/data/userSettings.tsapps/roam/src/styles/streamlineStyling.tsapps/roam/src/components/settings/HomePersonalSettings.tsxapps/roam/src/index.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/main.mdc)
**/*.{ts,tsx,js,jsx}: Prefer arrow functions over regular function declarations
Use named parameters (object destructuring) when a function has more than 2 parameters
Use Prettier with the project's configuration
Maintain consistent naming conventions: PascalCase for components and types
Maintain consistent naming conventions: camelCase for variables and functions
Maintain consistent naming conventions: UPPERCASE for constants
Files:
apps/roam/src/data/userSettings.tsapps/roam/src/styles/streamlineStyling.tsapps/roam/src/components/settings/HomePersonalSettings.tsxapps/roam/src/index.ts
apps/roam/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/roam.mdc)
apps/roam/**/*.{js,jsx,ts,tsx}: Use BlueprintJS 3 components and Tailwind CSS for platform-native UI in the Roam Research extension
Use the roamAlphaApi documentation from https://roamresearch.com/#/app/developer-documentation/page/tIaOPdXCj when working with the Roam API
Use Roam Depot/Extension API documentation from https://roamresearch.com/#/app/developer-documentation/page/y31lhjIqU when working with the Roam Extension API
Files:
apps/roam/src/data/userSettings.tsapps/roam/src/styles/streamlineStyling.tsapps/roam/src/components/settings/HomePersonalSettings.tsxapps/roam/src/index.ts
🧠 Learnings (9)
📓 Common learnings
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-07-19T22:34:23.619Z
Learning: Applies to apps/roam/**/*.{js,jsx,ts,tsx} : Use BlueprintJS 3 components and Tailwind CSS for platform-native UI in the Roam Research extension
📚 Learning: 2025-07-08T14:51:55.299Z
Learnt from: maparent
Repo: DiscourseGraphs/discourse-graph PR: 0
File: :0-0
Timestamp: 2025-07-08T14:51:55.299Z
Learning: DiscourseGraphs maintainers prefer not to add narrowly typed definitions for individual `process.env` keys to avoid unnecessary coupling.
Applied to files:
apps/roam/src/data/userSettings.ts
📚 Learning: 2025-07-19T22:34:23.619Z
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-07-19T22:34:23.619Z
Learning: Applies to apps/roam/**/*.{js,jsx,ts,tsx} : Use BlueprintJS 3 components and Tailwind CSS for platform-native UI in the Roam Research extension
Applied to files:
apps/roam/src/styles/streamlineStyling.tsapps/roam/src/components/settings/HomePersonalSettings.tsxapps/roam/src/index.ts
📚 Learning: 2025-07-19T22:34:08.725Z
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/main.mdc:0-0
Timestamp: 2025-07-19T22:34:08.725Z
Learning: Applies to {apps/website,packages/ui}/**/*.{tsx,jsx} : When refactoring inline styles, use tailwind classes
Applied to files:
apps/roam/src/styles/streamlineStyling.tsapps/roam/src/components/settings/HomePersonalSettings.tsxapps/roam/src/index.ts
📚 Learning: 2025-07-19T22:34:08.725Z
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/main.mdc:0-0
Timestamp: 2025-07-19T22:34:08.725Z
Learning: Applies to apps/website/**/*.{tsx,jsx,ts,js} : Use Tailwind CSS for styling where possible
Applied to files:
apps/roam/src/styles/streamlineStyling.ts
📚 Learning: 2025-07-19T22:34:23.619Z
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-07-19T22:34:23.619Z
Learning: Applies to apps/roam/**/*.{js,jsx,ts,tsx} : Use Roam Depot/Extension API documentation from https://roamresearch.com/#/app/developer-documentation/page/y31lhjIqU when working with the Roam Extension API
Applied to files:
apps/roam/src/styles/streamlineStyling.tsapps/roam/src/index.ts
📚 Learning: 2025-07-19T22:34:23.619Z
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-07-19T22:34:23.619Z
Learning: Applies to apps/roam/**/*.{js,jsx,ts,tsx} : Use the roamAlphaApi documentation from https://roamresearch.com/#/app/developer-documentation/page/tIaOPdXCj when working with the Roam API
Applied to files:
apps/roam/src/styles/streamlineStyling.tsapps/roam/src/index.ts
📚 Learning: 2025-07-19T22:34:23.619Z
Learnt from: CR
Repo: DiscourseGraphs/discourse-graph PR: 0
File: .cursor/rules/roam.mdc:0-0
Timestamp: 2025-07-19T22:34:23.619Z
Learning: Applies to apps/roam/package.json : Prefer existing dependencies from package.json when adding or using dependencies in the Roam Research extension
Applied to files:
apps/roam/src/index.ts
📚 Learning: 2025-08-26T01:52:35.443Z
Learnt from: mdroidian
Repo: DiscourseGraphs/discourse-graph PR: 369
File: apps/roam/src/utils/pluginTimer.ts:14-21
Timestamp: 2025-08-26T01:52:35.443Z
Learning: In the discourse-graph codebase, initPluginTimer() is only called once during the runExtension flow in apps/roam/src/index.ts, so re-initialization scenarios are not a concern for this specific usage pattern.
Applied to files:
apps/roam/src/index.ts
🔇 Additional comments (4)
apps/roam/src/data/userSettings.ts (1)
10-10: LGTM!The new setting key follows the established naming convention and export pattern.
apps/roam/src/components/settings/HomePersonalSettings.tsx (1)
232-261: LGTM! Clean DOM management with duplicate prevention.The checkbox implementation correctly:
- Checks for existing style elements before adding
- Assigns a consistent id for later retrieval
- Removes the element when disabled
The pattern matches the existing DISCOURSE_CONTEXT_OVERLAY_IN_CANVAS_KEY approach.
apps/roam/src/index.ts (2)
118-125: Clean conditional feature loading.The streamline styling is properly gated behind the user setting and assigns a consistent id for DOM management.
181-181: Proper cleanup handling via elements array.The conditional spread correctly includes the style element only when enabled, ensuring the extension framework can clean it up on unload.
mdroidian
left a comment
mdroidian
left a comment
There was a problem hiding this comment.
👍 Nice.
I'd also recommend creating a [[Streamline styling test]] (or similar) in the discourse-graphs graph so our team can quickly test it.
eg (just add all elements that are affected/worth seeing)
|
next action: rebase and wait for Matt + Johnny's review |
trangdoan982
force-pushed
the
eng-1025-integrate-subtle-controls-css-styling-into-plugin
branch
from
478777f to
cffcf11
Compare
trangdoan982
deleted the
eng-1025-integrate-subtle-controls-css-styling-into-plugin
branch
Summary by CodeRabbit
Release Notes
Ability for user to select "streamlined styling" in the settings
Streamlined styling includes:
"Subtle block styling" (DIS-562)
Optionally includes the following:
Page links underlined, gray
Make all headers the same color
Todo boxes gray
Right sidebar: white background
Table: no bg colors in rows or body of table
