Update shared UI for dropdown improvements #277
Conversation
|
Important Review skippedAuto incremental 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 📝 WalkthroughWalkthroughUpdates package.json scripts and a dependency version. Adjusts Label component API, styling, and color computation with a new color-utils module. Tweaks Label.css by removing padding and border-radius. Updates LabelsFilter to pass forcedColorScheme, adopt outlined labels with tooltips, adjust opacity handling, and prevent default selection in dropdown items. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant LF as LabelsFilter
participant DM as DropdownMenuContent
participant CI as DropdownMenuCheckboxItem
participant L as Label
participant CU as color-utils
U->>LF: Open labels menu / interact
LF->>DM: Render with forcedColorScheme
loop For each label
DM->>CI: Render checkbox item
CI->>L: Render Label(variant="outlined", showTooltip)
L->>CU: Compute colors (hslToHex, contrast, hslColorToCss)
CU-->>L: Hex/HSL and contrast values
end
U->>CI: Click item
CI->>CI: onSelect(e.preventDefault())
CI->>LF: onCheckedChange(checked)
LF->>LF: Update selection state
LF->>L: Re-render Label with updated classes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for graypaper-reader ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
package.json (1)
10-10: Be cautious with “unsafe” auto-fixes in CI/dev flowsAdding biome’s --unsafe can rewrite code in non-trivial ways. Consider restricting it to local-only usage (e.g., a separate npm script not used by hooks/CI) or scoping to specific paths. Document its intended use in CONTRIBUTING.md.
src/components/Label/color-utils.ts (2)
13-19: Tighten types for contrast to guarantee RGB tuple lengthUse a fixed-size tuple instead of number[] to prevent accidental short/long arrays and improve call-site safety.
Apply:
-export function contrast(rgb1: number[], rgb2: number[]) { +export type RGB = readonly [number, number, number]; +export function contrast(rgb1: RGB, rgb2: RGB) {
1-3: Optionally round HSL values for stable CSS stringsIf hslColorToCss receives high-precision floats, CSS is valid but noisy. Rounding to one decimal improves consistency and caching.
Apply:
-export function hslColorToCss(hsl: [number, number, number]) { - return `hsl(${hsl[0]}, ${hsl[1]}%, ${hsl[2]}%)`; +export type HSL = readonly [number, number, number]; +export function hslColorToCss(hsl: HSL) { + const [h, s, l] = hsl; + const r = (v: number) => Math.round(v * 10) / 10; + return `hsl(${r(h)}, ${r(s)}%, ${r(l)}%)`; }src/components/Label/Label.tsx (3)
25-32: Prefer class merging helper for predictable class compositionUse twMerge to combine base classes with overrides and avoid accidental duplication when consumers pass spacing/rounded utilities.
Apply:
- return ( - <span - style={style} - className={`label truncate ${className} rounded-xl px-2.5 py-0.5`} - title={showTooltip ? label : undefined} - > + return ( + <span + style={style} + className={twMerge("label truncate rounded-xl px-2.5 py-0.5", className)} + title={showTooltip ? label : undefined} + >Also applies to: 34-41
49-54: Revisit dimming formula for outlined background; current lightness/5 may be too darkDividing lightness by 5 (e.g., 65 → 13) produces near-black backgrounds, which can reduce contrast for the colored text, especially for hues with poor contrast at low L. Consider a gentler dim (e.g., clamp to [20–30]) or scale relative to current L.
Example:
- const newDimmedLightness = mainColor.hsl[2] / 5; + const newDimmedLightness = Math.max(22, Math.min(32, mainColor.hsl[2] * 0.5));
56-67: Text color heuristic is simple and works; consider WCAG AA checks if neededThe white path uses #E0E0E0 for reduced glare. If accessibility targets require 4.5:1, consider computing contrast against both #000 and #E0E0E0 and falling back to #FFF if needed to hit AA.
📜 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 by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
package.json(2 hunks)src/components/Label/Label.css(0 hunks)src/components/Label/Label.tsx(4 hunks)src/components/Label/color-utils.ts(1 hunks)src/components/LabelsFilter/LabelsFilter.tsx(1 hunks)
💤 Files with no reviewable changes (1)
- src/components/Label/Label.css
🧰 Additional context used
🧬 Code graph analysis (2)
src/components/LabelsFilter/LabelsFilter.tsx (1)
src/components/Label/Label.tsx (1)
Label(8-43)
src/components/Label/Label.tsx (2)
src/components/NotesProvider/hooks/useLabels.ts (1)
PrefixedLabel(8-8)src/components/Label/color-utils.ts (2)
hslColorToCss(1-3)hslToHex(21-32)
🔇 Additional comments (6)
package.json (1)
22-22: Confirm forcedColorScheme prop support on DropdownMenuContentPlease verify that your installed version of @fluffylabs/shared-ui (^0.1.2) exposes a forcedColorScheme prop on its DropdownMenuContent component. If this prop isn’t explicitly handled by the component (in its implementation or TypeScript declarations), it will be forwarded to the underlying DOM element and trigger React warnings about unknown props.
Recommended verification steps:
- Inspect the source or type definitions of DropdownMenuContent in node_modules/@fluffylabs/shared-ui and ensure forcedColorScheme is listed.
- Run a quick search, for example:
•grep -R "forcedColorScheme" node_modules/@fluffylabs/shared-ui
• Open the component’s.d.ts(or source) and look for a matching prop signature.src/components/Label/color-utils.ts (1)
5-11: Good luminance/contrast implementationsRGB companding and Rec. 709 coefficients look correct. No side effects; nice and testable.
src/components/LabelsFilter/LabelsFilter.tsx (3)
70-70: Prop passthrough: confirm DropdownMenuContent supports forcedColorSchemeSame concern as in package.json: ensure this prop is defined on the shared-ui component and not forwarded to a DOM node.
If needed, gate it with a conditional prop spread:
- <DropdownMenuContent className="w-auto max-w-72" align="end" forcedColorScheme={forcedColorScheme}> + <DropdownMenuContent + className="w-auto max-w-72" + align="end" + {...(forcedColorScheme ? { forcedColorScheme } : {})} + >
77-79: onSelect preventDefault is appropriate to keep the menu open; verify keyboard behaviorPreventing default here keeps the menu open for multi-select, which matches the intent. Please confirm that Space/Enter still toggle via onCheckedChange for both root and child items and that focus doesn’t get trapped.
Manual checks:
- Navigate with Arrow keys, toggle with Space/Enter, ensure the menu stays open.
- Click outside should still close the menu.
- ESC should close the menu.
Also applies to: 92-94
80-85: Nice: tooltip + outlined variant improve scannabilityGood UX tweak. Truncation with title preserves discoverability for long labels.
src/components/Label/Label.tsx (1)
81-87: Good: color generation pipeline is deterministic and stable across sessionsFixed palette size with hash → hue mapping plus hslToHex makes label colors predictable. Nice.
1 participant
759dd60 - feat: Add label tooltips to filter menu
e00d007 - feat: Add outlined variant to Label component
437d922 - fix: labels filter dropdown color scheme and selection
e02c45b - style: update labels style to match design more closely
debca32 - chore: update @fluffylabs/shared-ui
ee17aad - add 0/1 indicator to labels dropdown; add there nice loading UI