ENG-836 make button like css for node tag #443

sid597 · 2025-09-15T05:18:23Z

Summary by CodeRabbit

New Features
- Tag popups and canvas nodes now use an improved dynamic color theming that derives coordinated background, text, border, and contrast for better readability.
Style
- Visual refinements to tag popups: improved padding, rounded corners, subtle shadow, refined font sizing/weight, spacing, and cursor cues for clearer interaction.

linear · 2025-09-15T05:18:26Z

ENG-836 "button-like" CSS styling for node tag

supabase · 2025-09-15T05:18:30Z

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 ↗︎.

sid597 · 2025-09-15T05:18:38Z

ENG-836 make button like css for node tag #443 👈 (View in Graphite)
ENG-842 change node creation behaviour from node tags #442
main

This stack of pull requests is managed by Graphite. Learn more about stacking.

sid597 · 2025-09-15T05:19:38Z

@coderabbitai full review

coderabbitai · 2025-09-15T05:19:44Z

✅ Actions performed

Full review triggered.

coderabbitai · 2025-09-15T05:26:22Z

📝 Walkthrough

Walkthrough

Replaces prior simple tag-color assignment with a new color-theming flow using colord + a new getPleasingColors utility to compute accessible background/text/border colors, wires that scheme into tag popup inline styles, and updates discourse node color derivation to use the pleasing-color results. Adds colord dependency.

Changes

Cohort / File(s)	Summary
Tag popup theming `apps/roam/src/utils/initializeObserversAndListeners.ts`	Replace single text-color assignment with full inline styling when `node.canvasSettings.color` exists; import and use `colord` + `getPleasingColors` to derive background, text, border, and other style properties for tag popups.
Node color calculation `apps/roam/src/components/canvas/DiscourseNodeUtil.tsx`	Replace previous ContrastColor-based contrast logic with `colord` and `getPleasingColors` flow; introduce `canvasSelectedColor`, compute `backgroundColor` and `textColor` from pleasing colors; update imports.
New color utility `packages/utils/src/getPleasingColors.ts`	Add `getPleasingColors` function and `PleasingColorScheme` type exporting an algorithm that generates accessible primary/background/text/border colors and contrast ratio using `colord` (with a11y and mix logic).
Dependency update `package.json`	Add runtime dependency `colord` to project `dependencies`.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Node as DiscourseNode
  participant Util as getPleasingColors
  participant Popup as TagPopupRenderer

  Node->>Util: provide node.canvasSettings.color (hex / formatted)
  note right of Util #D6F5D6: compute pleasing color scheme\n(primary, background, text, border, contrast)
  Util-->>Node: PleasingColorScheme
  Node->>Popup: render tag popup with inline styles (background, color, border, ...)
  Popup-->>Node: UI updated

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Roam: ENG-622: Node tag to node conversions #304 — Modifies tag popup rendering/listener code paths that this change themes, likely overlapping styling logic.
Roam: ENG-798 Node Tags breaks Color Highlighter #402 — Also changes node-tag popup rendering and invocation flow in initializeObserversAndListeners.ts, touching the same rendering/styling area.

Pre-merge checks

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Title Check	⚠️ Warning	The current title “ENG-836 make button like css for node tag” is vague and does not clearly convey that the primary change implements dynamic color theming and contrast logic for node tag popups using the Colord library and a custom pleasing-color algorithm. It includes a ticket reference and informal phrasing that obscures the main functional update. A concise, descriptive title should highlight the introduction of the getPleasingColors flow and colord integration for tag styling.	Please rename the pull request to clearly describe the main change, for example: “Add dynamic color theming for node tag popups using Colord and getPleasingColors.”

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

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

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 3

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e4d5c93 and c08ba49.

📒 Files selected for processing (1)

apps/roam/src/utils/initializeObserversAndListeners.ts (3 hunks)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

GitHub Check: Graphite / mergeability_check
GitHub Check: Graphite / mergeability_check
GitHub Check: Graphite / mergeability_check
GitHub Check: Graphite / mergeability_check

apps/roam/src/utils/initializeObserversAndListeners.ts

mdroidian

I think many colors will still fail using this.

Try the contrast-color package (we use it in DiscourseNodeUtil) or a package like it to generate the corresponding background that is WCAG compliant.

Make sure to test them with https://webaim.org/resources/contrastchecker/

apps/roam/src/utils/initializeObserversAndListeners.ts

mdroidian

While this does look good on our discourse-graphs graph

The default shipped node colors turn out to be mostly black or white backgrounds :(

Can you think of any tweaks we could make so that these are compliant colored backgrounds?

If not, a couple other options could be

manually set the foreground/background for the default shipped nodes (not ideal)
change the default shipped colors (also not ideal)

Swap the main color

Another idea would be to use the node color (the color that is set in the node setting) as the background color instead of the text color. Then calculate the text color based on that.

That way the fallback would be that the text color is black or white, not the background color. This would be much cleaner than a bunch of black/white background tags.

I think we should do both:

swap the main color to be background instead of text
test discourse-graphs and the default shipped node colors and aim for 75% to not be black/white

Let me know what you think

packages/utils/src/getContrastingColor.ts

apps/roam/src/components/canvas/DiscourseNodeUtil.tsx

sid597 · 2025-09-28T07:52:03Z

@mdroidian can we change the default text color?

this is what the colors look like (in plugin-testing-akamatsulab) when we use the text color as background (swapped from before)

and in default plugin

mdroidian · 2025-09-28T07:56:30Z

@mdroidian can we change the default text color?

this is what the colors look like (in plugin-testing-akamatsulab) when we use the text color as background (swapped from before)

and in default plugin

This looks better. Have you tried tweaking the algorithm at all?

sid597 · 2025-09-28T08:01:23Z

no the tweak we can do is make the steps smaller, currently we use 0.1 I changed it to 0.01 but it looked the same. I just swapped the text with background.

By steps I mean if we don't find a contrasting color then either lighten or darken the current color by x amount smaller number means its more granular but more iterations

mdroidian · 2025-09-29T03:00:40Z

ENG-836 make button like css for node tag - 28 September 2025

https://github.com/mdroidian/color-contrast-button-test

mdroidian

Looks like this PR needs a rebase and the new "pleasing" algorithm, is that correct @sid597 ?

sid597 · 2025-10-06T03:22:53Z

@mdroidian now using the pleasing algorithm but there are lint issues related to colord

import { colord, extend, type Colord } from "colord";
import a11yPlugin from "colord/plugins/a11y";
import mixPlugin from "colord/plugins/mix";
extend([a11yPlugin, mixPlugin]);

Unsafe call of an `error` type typed value.eslint[@typescript-eslint/no-unsafe-call](https://typescript-eslint.io/rules/no-unsafe-call)

....
const t = colord({ h: 0, s: 0, l: mid }); // neutral gray
const cr = bg.contrast(t);

Unsafe call of an `error` type typed value.eslint[@typescript-eslint/no-unsafe-call](https://typescript-eslint.io/rules/no-unsafe-call)

sid597 · 2025-10-08T05:37:18Z

@mdroidian I did not rename the function or removed the comments from the pleasing algorithm file I think for this case the all comments make sense.

mdroidian

Just move colord to apps/roam and packages/util and remove contrast-color then good to go! 👍

package.json

mdroidian · 2025-10-12T21:49:38Z

Actually let's run the pleasing algo through code rabbit too.

@coderabbitai full review

coderabbitai · 2025-10-12T21:50:01Z

@mdroidian: I'll trigger a full review of the PR, paying special attention to the pleasing algorithm implementation and the colord integration. The review will cover the color contrast logic, TypeScript type safety issues, and the overall implementation.

✅ Actions performed

Full review triggered.

coderabbitai

Actionable comments posted: 1

♻️ Duplicate comments (2)

apps/roam/src/utils/initializeObserversAndListeners.ts (1)
122-143: Prefer CSS variables + class pattern to avoid style thrashing.

The inline style application remains unaddressed from previous reviews. Repeatedly applying inline styles on observer callbacks can cause style thrashing and complicates theming. The suggested CSS variable pattern from prior reviews would make this more maintainable and performant.

Consider applying the previously suggested pattern:
  if (node.canvasSettings?.color) {
    const formattedColor = formatHexColor(node.canvasSettings.color);
    if (!formattedColor) {
      break;
    }
    const contrastingColor = getPleasingColors(
      colord(formattedColor),
    );

-   Object.assign(s.style, {
-     backgroundColor: contrastingColor.background,
-     color: contrastingColor.text,
-     border: `1px solid ${contrastingColor.border}`,
-     fontWeight: "500",
-     padding: "2px 6px",
-     borderRadius: "12px",
-     margin: "0 2px",
-     fontSize: "0.9em",
-     whiteSpace: "nowrap",
-     boxShadow: "0 1px 2px rgba(0, 0, 0, 0.05)",
-     display: "inline-block",
-     cursor: "pointer",
-   });
+   if (s.dataset.dgTagStyled !== "1") {
+     s.dataset.dgTagStyled = "1";
+     s.classList.add("dg-node-tag-btn");
+   }
+   s.style.setProperty("--dg-tag-bg", contrastingColor.background);
+   s.style.setProperty("--dg-tag-fg", contrastingColor.text);
+   s.style.setProperty("--dg-tag-border", contrastingColor.border);
  }
Then add corresponding CSS to your stylesheet:
.dg-node-tag-btn {
  background-color: var(--dg-tag-bg);
  color: var(--dg-tag-fg);
  border: 1px solid var(--dg-tag-border);
  font-weight: 500;
  padding: 2px 6px;
  border-radius: 12px;
  margin: 0 2px;
  font-size: 0.9em;
  white-space: nowrap;
  box-shadow: 0 1px 2px rgba(0,0,0,0.05);
  display: inline-block;
  cursor: pointer;
}
package.json (1)
33-34: Move colord to workspace-specific dependencies.

Adding colord to the root package.json violates the turbo convention mentioned in previous reviews. Since colord is only used in apps/roam and packages/utils, add it to those specific package.json files instead.

Based on learnings.

Apply this change:

Remove colord from root package.json

Add to apps/roam/package.json:
"dependencies": {
  "colord": "^2.9.3"
}
Add to packages/utils/package.json:
"dependencies": {
  "colord": "^2.9.3"
}

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a69b94e and 71f75d1.

⛔ Files ignored due to path filters (1)

pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml

📒 Files selected for processing (4)

apps/roam/src/components/canvas/DiscourseNodeUtil.tsx (3 hunks)
apps/roam/src/utils/initializeObserversAndListeners.ts (2 hunks)
package.json (1 hunks)
packages/utils/src/getPleasingColors.ts (1 hunks)

🔇 Additional comments (4)

apps/roam/src/utils/initializeObserversAndListeners.ts (1)

53-54: LGTM! Proper integration of color utilities.

The imports of getPleasingColors and colord are correct and enable the new WCAG-compliant color theming flow.

apps/roam/src/components/canvas/DiscourseNodeUtil.tsx (2)

34-34: LGTM! Proper imports for the new color utility.

The imports of colord and getPleasingColors are correctly placed and support the refactored color calculation flow.

Also applies to: 47-47

352-361: LGTM! Clean refactor to use accessible color algorithm.

The getColors() method now properly:

Formats and validates the color input

Falls back to palette color when needed

Derives accessible background and text colors via getPleasingColors

This is a significant improvement over the previous approach.

packages/utils/src/getPleasingColors.ts (1)

1-191: Validate getPleasingColors on edge-case colors for WCAG compliance.

Manually test locally with these inputs:
Bright Yellow (#ffff00), Bright Cyan (#00ffff), Very Dark Blue (#33), Pure Black (#000000), Pure White (#ffffff), Mid Gray (#808080), Saturated Red (#ff0000).
Ensure returned background/text colors meet at least AA (4.5:1) and AAA (7:1) thresholds.

packages/utils/src/getPleasingColors.ts

sid597 · 2025-10-19T18:48:29Z

cc @mdroidian merging

github-project-automation bot added this to General Sep 15, 2025

sid597 mentioned this pull request Sep 15, 2025

ENG-842 change node creation behaviour from node tags #442

Merged

sid597 changed the title ~~make button like css for node tag~~ ENG-836 make button like css for node tag Sep 15, 2025

sid597 marked this pull request as ready for review September 15, 2025 05:19

coderabbitai bot reviewed Sep 15, 2025

View reviewed changes

apps/roam/src/utils/initializeObserversAndListeners.ts Outdated Show resolved Hide resolved

apps/roam/src/utils/initializeObserversAndListeners.ts Outdated Show resolved Hide resolved

apps/roam/src/utils/initializeObserversAndListeners.ts Outdated Show resolved Hide resolved

mdroidian changed the base branch from eng-842-change-behaviorlocation-of-moved-text to graphite-base/443 September 15, 2025 08:48

mdroidian force-pushed the graphite-base/443 branch from e4d5c93 to 9a87a36 Compare September 15, 2025 08:48

mdroidian force-pushed the eng-836-button-like-css-styling-for-node-tag branch from c08ba49 to 006d860 Compare September 15, 2025 08:48

graphite-app bot changed the base branch from graphite-base/443 to main September 15, 2025 08:49

mdroidian force-pushed the eng-836-button-like-css-styling-for-node-tag branch from 006d860 to 5be0c75 Compare September 15, 2025 08:49

mdroidian mentioned this pull request Sep 16, 2025

Nodetag documentation #433

Merged

sid597 force-pushed the eng-836-button-like-css-styling-for-node-tag branch from 5be0c75 to cc93d09 Compare September 21, 2025 07:12

sid597 requested a review from mdroidian September 21, 2025 07:14

mdroidian requested changes Sep 22, 2025

View reviewed changes

apps/roam/src/utils/initializeObserversAndListeners.ts Outdated Show resolved Hide resolved

apps/roam/src/utils/initializeObserversAndListeners.ts Outdated Show resolved Hide resolved

sid597 force-pushed the eng-836-button-like-css-styling-for-node-tag branch from cc93d09 to 20aac2b Compare September 27, 2025 19:58

sid597 requested a review from mdroidian September 27, 2025 20:01

mdroidian requested changes Sep 27, 2025

View reviewed changes

packages/utils/src/getContrastingColor.ts Outdated Show resolved Hide resolved

apps/roam/src/components/canvas/DiscourseNodeUtil.tsx Outdated Show resolved Hide resolved

sid597 requested a review from mdroidian September 28, 2025 07:54

mdroidian requested changes Oct 5, 2025

View reviewed changes

sid597 force-pushed the eng-836-button-like-css-styling-for-node-tag branch from 2ee1869 to 3906aa6 Compare October 6, 2025 01:50

sid597 requested a review from mdroidian October 8, 2025 05:37

mdroidian approved these changes Oct 12, 2025

View reviewed changes

package.json Outdated Show resolved Hide resolved

coderabbitai bot reviewed Oct 12, 2025

View reviewed changes

packages/utils/src/getPleasingColors.ts Show resolved Hide resolved

sid597 added 8 commits October 19, 2025 23:36

make button like css for node tag

daba4fc

fix color schemes

bcc1cdb

use colord

d83ef9a

address review

06d8b65

remove package lock

5f9821f

use pleasing algorithm

4d9432f

fix lint

e431999

fix package imports

1f4b964

sid597 force-pushed the eng-836-button-like-css-styling-for-node-tag branch from 71f75d1 to 1f4b964 Compare October 19, 2025 18:44

sid597 merged commit 510ac31 into main Oct 19, 2025
5 of 6 checks passed

github-project-automation bot moved this to Done in General Oct 19, 2025

coderabbitai bot mentioned this pull request Oct 19, 2025

ENG-986 Quick UI refactor on Canvas Drawer #507

Merged

coderabbitai bot mentioned this pull request Dec 1, 2025

ENG-1107 Update drag and drop image for nodes in discourse toolbar #578

Merged

ENG-836 make button like css for node tag #443

ENG-836 make button like css for node tag #443

Uh oh!

Conversation

sid597 commented Sep 15, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

linear bot commented Sep 15, 2025

Uh oh!

supabase bot commented Sep 15, 2025

Uh oh!

sid597 commented Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sid597 commented Sep 15, 2025

Uh oh!

coderabbitai bot commented Sep 15, 2025

Uh oh!

coderabbitai bot commented Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Pre-merge checks

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mdroidian left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mdroidian left a comment

Choose a reason for hiding this comment

Swap the main color

Uh oh!

Uh oh!

Uh oh!

sid597 commented Sep 28, 2025

Uh oh!

mdroidian commented Sep 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sid597 commented Sep 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mdroidian commented Sep 29, 2025

Uh oh!

mdroidian left a comment

Choose a reason for hiding this comment

Uh oh!

sid597 commented Oct 6, 2025

Uh oh!

sid597 commented Oct 8, 2025

Uh oh!

mdroidian left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mdroidian commented Oct 12, 2025

Uh oh!

coderabbitai bot commented Oct 12, 2025

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sid597 commented Oct 19, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

sid597 commented Sep 15, 2025 •

edited by coderabbitai bot

Loading

sid597 commented Sep 15, 2025 •

edited

Loading

coderabbitai bot commented Sep 15, 2025 •

edited

Loading

mdroidian commented Sep 28, 2025 •

edited

Loading

sid597 commented Sep 28, 2025 •

edited

Loading