-
Notifications
You must be signed in to change notification settings - Fork 4
Render suggestive mode overlay button #465
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
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
@CodeRabbit full review |
✅ Actions performedFull review triggered. |
📝 WalkthroughWalkthroughAdds a new Suggestive Mode overlay feature: implements a React overlay component with a render entry point, wires a new page-ref observer/handler to render it, and exposes a user setting to enable/disable the observer. Integration touches settings UI, observer initialization, and page-ref handler logic. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Settings as Settings UI
participant Init as Observer Init
participant Observer as PageRef Observer
participant Handler as suggestiveOverlayPageRefHandler
participant Renderer as renderSuggestive
participant React as SuggestiveModeOverlay (React)
User->>Settings: Toggle "Suggestive Mode Overlay"
Settings-->>Init: onPageRefObserverChange(handler)
Init->>Observer: addPageRefObserver(getSuggestiveOverlayHandler)
Note over Observer,Handler: On matching page-ref span appears
Observer->>Handler: handle(span, onloadArgs)
Handler->>Handler: Gate checks (parent, tag, data attr)
Handler->>Renderer: renderSuggestive({ tag, parent, onloadArgs })
Renderer->>React: Mount component into parent
React-->>Observer: Overlay mounted
User->>React: Click toggle button
React->>React: Open/close panel, toggle highlights
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 |
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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/roam/src/components/SuggestiveModeOverlay.tsx(1 hunks)apps/roam/src/components/settings/HomePersonalSettings.tsx(2 hunks)apps/roam/src/utils/initializeObserversAndListeners.ts(2 hunks)apps/roam/src/utils/pageRefObserverHandlers.ts(3 hunks)
🔇 Additional comments (3)
apps/roam/src/utils/initializeObserversAndListeners.ts (1)
146-148: Nice alignment with existing observer gatingConditionally adding the suggestive observer before the shared enable step mirrors the other overlay flows and keeps the initialization order intact. Looks good.
apps/roam/src/components/settings/HomePersonalSettings.tsx (1)
80-104: Setting toggle wiring looks solidThe new checkbox mirrors the discourse overlay toggle, persists the setting, and hooks into
onPageRefObserverChangewith the cached handler. No concerns here.apps/roam/src/utils/pageRefObserverHandlers.ts (1)
21-83: Parallel handler implementation looks goodThe suggestive overlay handler mirrors the discourse overlay flow, including tag resolution, data-attribute gating, and parent insertion. This keeps the observer semantics consistent. Nicely done.
mdroidian
left a 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.
* extract suggestive mode personal setting * add home setting * remove console log


refer eng-732-left-side-panel-for-suggestive-mode for main pr