Skip to content

Conversation

@maparent
Copy link
Collaborator

@maparent maparent commented Aug 11, 2025

Create a floating button with a submenu.
Trigger the BirdEatsBugs feedback as a menu element.

Summary by CodeRabbit

  • New Features

    • Added a floating “Discourse Graphs” menu with Send feedback, Docs, and Community entries; installs on startup and removes on unload.
  • Behavior Changes

    • Feedback widget’s default button can be hidden; personal settings toggle now shows/hides the new floating menu.
  • Style

    • New floating menu styles: light/dark themes, responsive panel, refined primary button states, placement presets, and UI utility classes.

Trigger the BirdEatsBugs feedback as a menu element.
@linear
Copy link

linear bot commented Aug 11, 2025

@supabase
Copy link

supabase bot commented Aug 11, 2025

This pull request has been ignored for the connected project zytfjzqyijgagqxrzbmz because there are no changes detected in packages/database/supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@maparent
Copy link
Collaborator Author

@CodeRabbit review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 11, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

1 similar comment
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 11, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 11, 2025

📝 Walkthrough

Walkthrough

Adds a Discourse floating menu UI to Roam, wires its lifecycle into the extension init/unload, adds styles, and changes BirdEatsBugs FeedbackWidget.setOptions so ui.defaultButton accepts { icon?: string } | boolean and is set to false. Also removes legacy hide/show helpers and the old initFeedbackWidget signature.

Changes

Cohort / File(s) Summary of changes
BirdEatsBugs integration
apps/roam/src/components/BirdEatsBugs.tsx
FeedbackWidget.setOptions type changed: ui.defaultButton is now `{ icon?: string }
Discourse floating menu component
apps/roam/src/components/DiscourseFloatingMenu.tsx
New React component DiscourseFloatingMenu (Popover + Menu with Send feedback → window.birdeatsbug?.trigger(), Docs, Community). Exports: installDiscourseFloatingMenu, removeDiscourseFloatingMenu, hideDiscourseFloatingMenu, showDiscourseFloatingMenu. Mount/unmount via anchor #dg-floating-menu-anchor.
App wiring / lifecycle
apps/roam/src/index.ts
Imports floating menu CSS and lifecycle helpers; creates style handle and includes it in render elements; calls installDiscourseFloatingMenu(onloadArgs.extensionAPI) during init and removeDiscourseFloatingMenu() on unload. Updated initFeedbackWidget() usage (no params).
Styles
apps/roam/src/styles/discourseFloatingMenuStyles.css
New stylesheet: CSS variables, placement presets, light/dark theme overrides, popover/screen/button styles, utility classes, responsive adjustments, and inline SVG icon for the menu button.
Settings wiring
apps/roam/src/components/settings/HomePersonalSettings.tsx
Replaced imports/calls of hideFeedbackButton/showFeedbackButton with hideDiscourseFloatingMenu/showDiscourseFloatingMenu and updated the setting toggle handler accordingly.

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant RoamApp
  participant FloatingMenu
  participant BirdEatsBug

  Note over RoamApp: Init
  RoamApp->>FloatingMenu: installDiscourseFloatingMenu(extensionAPI)
  activate FloatingMenu
  FloatingMenu-->>RoamApp: render anchor & button
  deactivate FloatingMenu

  User->>FloatingMenu: Click "Discourse Graph" button
  FloatingMenu->>FloatingMenu: Open Popover (Menu)
  User->>FloatingMenu: Click "Send feedback"
  FloatingMenu->>BirdEatsBug: window.birdeatsbug?.trigger()

  User->>FloatingMenu: Click "Docs"/"Community"
  FloatingMenu->>User: Open external link

  Note over RoamApp: Unload
  RoamApp->>FloatingMenu: removeDiscourseFloatingMenu()
  FloatingMenu-->>RoamApp: unmount & remove anchor
Loading
sequenceDiagram
  participant Caller
  participant BirdEatsBug

  Note over BirdEatsBug: Before
  Caller->>BirdEatsBug: setOptions({ ui: { defaultButton: { icon: undefined } } })

  Note over BirdEatsBug: After
  Caller->>BirdEatsBug: setOptions({ ui: { defaultButton: false } })
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.


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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

🧹 Nitpick comments (3)
apps/roam/src/styles/discourseFloatingMenuStyles.css (1)

63-76: Fix duplicate font name in font stack.

The font stack contains "Segoe UI" twice, which is redundant and should be cleaned up.

Apply this diff to remove the duplicate:

  font-family:
    Inter,
    system-ui,
    -apple-system,
-    Segoe UI,
    Roboto,
    Ubuntu,
    Cantarell,
    Noto Sans,
    sans-serif,
    "Apple Color Emoji",
    "Segoe UI Emoji",
    Segoe UI Symbol,
    "Noto Color Emoji";
apps/roam/src/components/DiscourseFloatingMenu.tsx (2)

6-6: Consider making position and theme configurable.

The positioning (bottom-right) and theme (dark) are hardcoded. Consider making these configurable through props or settings to improve flexibility.

Example implementation:

-export const DiscourseFloatingMenu = () => (
+export const DiscourseFloatingMenu = ({ position = 'bottom-right', theme = 'dark' }) => (
-  <div id="discourse-floating-menu" className="bottom-right dark">
+  <div id="discourse-floating-menu" className={`${position} ${theme}`}>

13-13: Add error handling for birdeatsbug trigger.

Consider adding error handling around the birdeatsbug trigger to gracefully handle cases where the service is unavailable.

Apply this diff to add error handling:

            onClick={() => {
              try {
                window.birdeatsbug?.trigger?.();
              } catch (error) {
                console.error('Failed to trigger feedback widget:', error);
              }
            }}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 448b7d7 and 096af22.

📒 Files selected for processing (4)
  • apps/roam/src/components/BirdEatsBugs.tsx (1 hunks)
  • apps/roam/src/components/DiscourseFloatingMenu.tsx (1 hunks)
  • apps/roam/src/index.ts (4 hunks)
  • apps/roam/src/styles/discourseFloatingMenuStyles.css (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-07-19T22:34:23.619Z
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#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/index.ts
📚 Learning: 2025-07-19T22:34:23.619Z
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#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/index.ts
🪛 Biome (2.1.2)
apps/roam/src/styles/discourseFloatingMenuStyles.css

[error] 75-75: Duplicate font names are redundant and unnecessary: Segoe

Remove duplicate font names within the property

(lint/suspicious/noDuplicateFontNames)

🔇 Additional comments (4)
apps/roam/src/components/BirdEatsBugs.tsx (1)

198-198: LGTM! Correctly hides the default button.

The change from { icon: undefined } to false properly hides the default BirdEatsBug button, allowing the new DiscourseFloatingMenu to handle feedback triggering instead.

apps/roam/src/styles/discourseFloatingMenuStyles.css (1)

1-271: Well-structured CSS with comprehensive theming support.

The CSS implementation is thorough with good use of CSS variables for theming, responsive design considerations, and proper positioning utilities. The adaptation from BirdEatsBugs styling maintains visual consistency.

apps/roam/src/index.ts (2)

20-20: LGTM! Clean integration of the floating menu.

The imports and integration follow the established patterns in the codebase.

Also applies to: 26-29


102-102: Proper lifecycle management implementation.

The floating menu is correctly integrated into the extension lifecycle with proper style handling and cleanup on unload.

Also applies to: 137-137, 144-144, 162-162

@maparent maparent self-assigned this Aug 11, 2025
@maparent maparent requested a review from mdroidian August 11, 2025 19:09
Copy link
Contributor

@mdroidian mdroidian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good start! A few requested changes.

Also, for UI changes, let's add a loom video showing said changes to the PR body.

Here are some examples:
#309
#327

If you don't have a way to record videos, we have a Discourse Graphs loom account. Let me know and I can help you get set up with it.

@maparent maparent requested a review from mdroidian August 14, 2025 13:39
@maparent
Copy link
Collaborator Author

@CodeRabbit review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 14, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

♻️ Duplicate comments (1)
apps/roam/src/components/DiscourseFloatingMenu.tsx (1)

100-102: Unmount React tree before removing the anchor to avoid leaks (React 17).

Removing the DOM node without calling ReactDOM.unmountComponentAtNode can leak event handlers and fibers.

-export const removeDiscourseFloatingMenu = () => {
-  document.getElementById(ANCHOR_ID)?.remove();
-};
+export const removeDiscourseFloatingMenu = () => {
+  const anchor = document.getElementById(ANCHOR_ID);
+  if (anchor) {
+    try {
+      ReactDOM.unmountComponentAtNode(anchor);
+    } catch (e) {
+      // no-op: unmount best-effort
+    }
+    anchor.remove();
+  }
+};
🧹 Nitpick comments (4)
apps/roam/src/styles/discourseFloatingMenuStyles.css (3)

195-197: Remove redundant pseudo-class.

:not(.button-icon) is duplicated.

-#discourse-floating-menu .bp3-button:not(.button-icon):not(.button-icon) > svg {
+#discourse-floating-menu .bp3-button:not(.button-icon) > svg {
   margin-left: calc(var(--base-size-step) * 0.75);
 }

63-76: Address Biome lint: duplicate “Segoe” font-family warning.

Biome flags duplicate font names; while the current stack is defensible, you can silence the warning by removing “Segoe UI Symbol,” which is largely obsolete on modern Windows.

   font-family:
     Inter,
     system-ui,
     -apple-system,
     Segoe UI,
     Roboto,
     Ubuntu,
     Cantarell,
     Noto Sans,
     sans-serif,
     "Apple Color Emoji",
     "Segoe UI Emoji",
-    Segoe UI Symbol,
     "Noto Color Emoji";

229-233: Token duplication: consider consolidating edge-distance tokens.

You reassign the same CSS variables later in the file. Prefer defining them once (or with clear theme/size variants) to avoid surprises.

apps/roam/src/components/DiscourseFloatingMenu.tsx (1)

31-40: Icon name “send-message” may not exist in Blueprint v3.

To ensure the icon renders consistently, consider a known IconName like "chat", "envelope", or "send-to".

-          <MenuItem
-            text="Send feedback"
-            icon="send-message"
+          <MenuItem
+            text="Send feedback"
+            icon="chat"
             onClick={() => {

Please verify against the Blueprint v3 IconName list.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled
  • Linear integration is disabled by default for public repositories

You can enable these settings in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 096af22 and 6f28856.

📒 Files selected for processing (3)
  • apps/roam/src/components/BirdEatsBugs.tsx (4 hunks)
  • apps/roam/src/components/DiscourseFloatingMenu.tsx (1 hunks)
  • apps/roam/src/styles/discourseFloatingMenuStyles.css (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/roam/src/components/BirdEatsBugs.tsx
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2025-08-11T19:09:58.252Z
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#337
File: apps/roam/src/components/DiscourseFloatingMenu.tsx:43-43
Timestamp: 2025-08-11T19:09:58.252Z
Learning: The roam subdirectory (apps/roam) is constrained to React 17 and cannot use React 18+ features like createRoot API. ReactDOM.render should be used instead of createRoot in this subdirectory.

Applied to files:

  • apps/roam/src/components/DiscourseFloatingMenu.tsx
📚 Learning: 2025-07-19T22:34:23.619Z
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#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/components/DiscourseFloatingMenu.tsx
📚 Learning: 2025-07-19T22:34:23.619Z
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#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/components/DiscourseFloatingMenu.tsx
📚 Learning: 2025-07-19T22:34:23.619Z
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#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/components/DiscourseFloatingMenu.tsx
📚 Learning: 2025-06-04T11:41:34.951Z
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#189
File: packages/database/supabase/migrations/20250603144146_account_centric.sql:50-63
Timestamp: 2025-06-04T11:41:34.951Z
Learning: In the discourse-graph database, all accounts currently stored are Roam platform accounts, making platform-specific migration logic safe for global operations.

Applied to files:

  • apps/roam/src/components/DiscourseFloatingMenu.tsx
📚 Learning: 2025-06-23T11:49:45.457Z
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#220
File: apps/roam/src/utils/conceptConversion.ts:11-40
Timestamp: 2025-06-23T11:49:45.457Z
Learning: In the DiscourseGraphs/discourse-graph codebase, direct access to `window.roamAlphaAPI` is the established pattern throughout the codebase. The team prefers to maintain this pattern consistently rather than making piecemeal changes, and plans to address dependency injection as a global refactor when scheduled.

Applied to files:

  • apps/roam/src/components/DiscourseFloatingMenu.tsx
📚 Learning: 2025-06-22T10:40:52.752Z
Learnt from: sid597
PR: DiscourseGraphs/discourse-graph#232
File: apps/roam/src/utils/getAllDiscourseNodesSince.ts:18-31
Timestamp: 2025-06-22T10:40:52.752Z
Learning: In apps/roam/src/utils/getAllDiscourseNodesSince.ts, the user confirmed that querying for `?title` with `:node/title` and mapping it to the `text` field in the DiscourseGraphContent type is the correct implementation for retrieving discourse node content from Roam Research, despite it appearing to query page titles rather than block text content.

Applied to files:

  • apps/roam/src/components/DiscourseFloatingMenu.tsx
🧬 Code Graph Analysis (1)
apps/roam/src/components/DiscourseFloatingMenu.tsx (1)
apps/roam/src/components/BirdEatsBugs.tsx (1)
  • FeedbackWidget (4-39)
🪛 Biome (2.1.2)
apps/roam/src/styles/discourseFloatingMenuStyles.css

[error] 75-75: Duplicate font names are redundant and unnecessary: Segoe

Remove duplicate font names within the property

(lint/suspicious/noDuplicateFontNames)

🔇 Additional comments (3)
apps/roam/src/styles/discourseFloatingMenuStyles.css (1)

112-116: :has() has limited support; ensure acceptable degradation.

The :has() pseudo-class has only fairly recent cross-browser support. If Firefox users are a target, verify that the visual differences are acceptable there or provide a fallback.

Would you like a conditional fallback using non-:has selectors or a class toggle applied via JS to gate these styles?

apps/roam/src/components/DiscourseFloatingMenu.tsx (2)

41-54: Good hardening on external links.

Using rel="noopener noreferrer" with target="_blank" is correct.


33-39: Guarded birdeatsbug trigger looks solid.

Casting to FeedbackWidget and optional chaining with try/catch prevents runtime errors when the SDK isn’t present.

Copy link
Contributor

@mdroidian mdroidian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hover and positioning is silky smooth now! Love it!

Lastly we need to make sure this integrates with the existing "Hide Feedback Button" toggle.
image

@maparent
Copy link
Collaborator Author

@CodeRabbit incremental review

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Aug 15, 2025

@maparent I'll perform an incremental review of the changes in this PR. This will provide a fresh review of all the modifications made to implement the floating action button with the DG logo and submenu functionality.

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

♻️ Duplicate comments (5)
apps/roam/src/components/BirdEatsBugs.tsx (1)

18-22: Type update for ui.defaultButton to accept boolean — LGTM

This aligns the type with actual usage and addresses the prior feedback about allowing false.

apps/roam/src/styles/discourseFloatingMenuStyles.css (2)

156-160: Coarse-pointer selector fixed to target Blueprint buttons — LGTM

Targeting .bp3-button ensures padding applies on touch devices.


175-184: Intent selector corrected to combine classes — LGTM

Using .bp3-button.bp3-intent-primary (same element) ensures intent styles apply.

apps/roam/src/components/DiscourseFloatingMenu.tsx (2)

15-20: Position prop narrowed to placement union — LGTM

Typing position as "top-left" | "top-right" | "bottom-left" | "bottom-right" matches the CSS API and prevents misuse with Blueprint’s Position.


60-63: Use enum for interactionKind — LGTM

Using PopoverInteractionKind.HOVER improves type-safety over string literals.

🧹 Nitpick comments (6)
apps/roam/src/components/BirdEatsBugs.tsx (2)

2-2: Remove unused OnloadArgs import

initFeedbackWidget no longer accepts OnloadArgs; keep imports tidy.

-import { OnloadArgs } from "roamjs-components/types";

167-172: Avoid dead config: text.defaultButton is unused when defaultButton is false

Since the default BirdEatsBugs button is disabled, this label won’t be shown. Dropping it reduces confusion.

       ui: {
         position: "bottom-right",
         defaultButton: false, // hide, will be triggered in DiscourseFloatingMenu
         text: {
-          defaultButton: "Send feedback",
           previewScreen: {
             title: "Discourse Graphs feedback",
           },
         },
apps/roam/src/styles/discourseFloatingMenuStyles.css (2)

229-233: Drop unnecessary !important flags

With the earlier conflicting rule removed, these !important markers are no longer needed.

-#discourse-floating-menu:has(.screen) {
-  box-shadow: none !important;
-  border-radius: 0 !important;
-  border: none !important;
-}
+#discourse-floating-menu:has(.screen) {
+  box-shadow: none;
+  border-radius: 0;
+  border: none;
+}

63-76: Address Biome warning: duplicate/ambiguous Segoe entries in font-family

Biome flags duplicates around "Segoe". Tighten the stack and quote multi-word fonts. Also remove “Segoe UI Symbol” (rarely necessary) to silence the warning.

-  font-family:
-    Inter,
-    system-ui,
-    -apple-system,
-    Segoe UI,
-    Roboto,
-    Ubuntu,
-    Cantarell,
-    Noto Sans,
-    sans-serif,
-    "Apple Color Emoji",
-    "Segoe UI Emoji",
-    Segoe UI Symbol,
-    "Noto Color Emoji";
+  font-family:
+    Inter,
+    system-ui,
+    -apple-system,
+    "Segoe UI",
+    Roboto,
+    Ubuntu,
+    Cantarell,
+    "Noto Sans",
+    sans-serif,
+    "Apple Color Emoji",
+    "Segoe UI Emoji",
+    "Noto Color Emoji";

If Biome still warns after this change, please paste the exact diagnostic so we can tweak the stack further.

apps/roam/src/components/DiscourseFloatingMenu.tsx (2)

34-43: Guarded trigger is fine; consider readiness UX (optional)

Current try/catch + optional chaining is safe. Optionally, you could disable the menu item or show a toast if the SDK hasn’t loaded yet to avoid a no-op click.


70-75: Optional: add accessible name for the FAB

The visible text helps, but an explicit aria-label can aid SRs if the button gets icon-only in the future.

   <Button
     intent={Intent.PRIMARY}
     text="Discourse Graphs"
     id="dg-floating-menu-button"
+    aria-label="Open Discourse Graphs menu"
     className={props.buttonTheme}
   />
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 6f28856 and 96be324.

📒 Files selected for processing (5)
  • apps/roam/src/components/BirdEatsBugs.tsx (5 hunks)
  • apps/roam/src/components/DiscourseFloatingMenu.tsx (1 hunks)
  • apps/roam/src/components/settings/HomePersonalSettings.tsx (2 hunks)
  • apps/roam/src/index.ts (5 hunks)
  • apps/roam/src/styles/discourseFloatingMenuStyles.css (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/roam/src/index.ts
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2025-08-11T19:09:58.252Z
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#337
File: apps/roam/src/components/DiscourseFloatingMenu.tsx:43-43
Timestamp: 2025-08-11T19:09:58.252Z
Learning: The roam subdirectory (apps/roam) is constrained to React 17 and cannot use React 18+ features like createRoot API. ReactDOM.render should be used instead of createRoot in this subdirectory.

Applied to files:

  • apps/roam/src/components/DiscourseFloatingMenu.tsx
📚 Learning: 2025-07-19T22:34:23.619Z
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#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/components/DiscourseFloatingMenu.tsx
📚 Learning: 2025-07-19T22:34:23.619Z
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#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/components/DiscourseFloatingMenu.tsx
📚 Learning: 2025-07-19T22:34:23.619Z
Learnt from: CR
PR: DiscourseGraphs/discourse-graph#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/components/DiscourseFloatingMenu.tsx
📚 Learning: 2025-06-04T11:41:34.951Z
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#189
File: packages/database/supabase/migrations/20250603144146_account_centric.sql:50-63
Timestamp: 2025-06-04T11:41:34.951Z
Learning: In the discourse-graph database, all accounts currently stored are Roam platform accounts, making platform-specific migration logic safe for global operations.

Applied to files:

  • apps/roam/src/components/DiscourseFloatingMenu.tsx
📚 Learning: 2025-06-23T11:49:45.457Z
Learnt from: maparent
PR: DiscourseGraphs/discourse-graph#220
File: apps/roam/src/utils/conceptConversion.ts:11-40
Timestamp: 2025-06-23T11:49:45.457Z
Learning: In the DiscourseGraphs/discourse-graph codebase, direct access to `window.roamAlphaAPI` is the established pattern throughout the codebase. The team prefers to maintain this pattern consistently rather than making piecemeal changes, and plans to address dependency injection as a global refactor when scheduled.

Applied to files:

  • apps/roam/src/components/DiscourseFloatingMenu.tsx
📚 Learning: 2025-06-22T10:40:52.752Z
Learnt from: sid597
PR: DiscourseGraphs/discourse-graph#232
File: apps/roam/src/utils/getAllDiscourseNodesSince.ts:18-31
Timestamp: 2025-06-22T10:40:52.752Z
Learning: In apps/roam/src/utils/getAllDiscourseNodesSince.ts, the user confirmed that querying for `?title` with `:node/title` and mapping it to the `text` field in the DiscourseGraphContent type is the correct implementation for retrieving discourse node content from Roam Research, despite it appearing to query page titles rather than block text content.

Applied to files:

  • apps/roam/src/components/DiscourseFloatingMenu.tsx
🧬 Code Graph Analysis (2)
apps/roam/src/components/settings/HomePersonalSettings.tsx (1)
apps/roam/src/components/DiscourseFloatingMenu.tsx (2)
  • hideDiscourseFloatingMenu (80-83)
  • showDiscourseFloatingMenu (85-88)
apps/roam/src/components/DiscourseFloatingMenu.tsx (1)
apps/roam/src/components/BirdEatsBugs.tsx (1)
  • FeedbackWidget (4-39)
🪛 Biome (2.1.2)
apps/roam/src/styles/discourseFloatingMenuStyles.css

[error] 75-75: Duplicate font names are redundant and unnecessary: Segoe

Remove duplicate font names within the property

(lint/suspicious/noDuplicateFontNames)

🔇 Additional comments (3)
apps/roam/src/components/DiscourseFloatingMenu.tsx (1)

45-56: External links: rel added — LGTM

rel="noopener noreferrer" is present alongside target="_blank".

apps/roam/src/components/settings/HomePersonalSettings.tsx (2)

12-14: Switched to new floating menu visibility APIs — LGTM

Imports align with the new DiscourseFloatingMenu helpers.


129-133: Hide/Show wiring matches persisted setting — LGTM

Behavior is consistent with installDiscourseFloatingMenu honoring the same key.

@maparent maparent requested a review from mdroidian August 15, 2025 03:01
Copy link
Contributor

@mdroidian mdroidian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@maparent maparent merged commit ca2f93b into main Aug 16, 2025
5 checks passed
@github-project-automation github-project-automation bot moved this to Done in General Aug 16, 2025
@maparent maparent deleted the ENG-592-replace-send-feedback-button-with-a-fab-floating-action branch August 16, 2025 02:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants