feat(Alert): add Doodle UI Alert component - BED-5807#2680
feat(Alert): add Doodle UI Alert component - BED-5807#2680
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (12)
✅ Files skipped from review due to trivial changes (4)
🚧 Files skipped from review as they are similar to previous changes (7)
📝 WalkthroughWalkthroughAdds a new Alert React component with Storybook stories, three new SVG icon components and an updated Info icon, a comprehensive color palette with light/dark tokens, and integration of those status colors into the Tailwind plugin and preset; component exports were updated accordingly. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/javascript/doodle-ui/src/tailwind/plugin.ts (1)
22-74:⚠️ Potential issue | 🔴 CriticalRemoving
--contrastfrom:rootbreaks text color in light theme.
--contrastis no longer defined in the:root/light scope — only inside.dark(line 77). Perpackages/javascript/doodle-ui/src/input.css(lines 22–25), the base layer applies@apply text-contrasttohtml, andpreset.tsline 43 mapscontrast: 'var(--contrast)'. In light mode,var(--contrast)will now resolve to its fallback (unset →currentcolor), leaving global text color undefined until.darkis active.The TODO at lines 31-32 references
palette.common.white, butpaletteisn't imported here and the line is commented out, so the variable is effectively gone. Either restore the previous literal value or land the BED-8069 follow-up in the same PR.🛠️ Suggested fix
' :root': { '--primary': '#33318f', @@ - // TODO: Update in https://specterops.atlassian.net/browse/BED-8069 - //'--contrast': palette.common.white, + '--contrast': '#ffffff', + // TODO: Update in https://specterops.atlassian.net/browse/BED-8069 to use palette.common.white🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/javascript/doodle-ui/src/tailwind/plugin.ts` around lines 22 - 74, The :root block in plugin.ts removed the --contrast variable causing var(--contrast) to fall back to currentcolor in light mode; re-add --contrast to the :root styles (the same block where --primary/--secondary are defined) with a sensible light-theme text color (e.g. '--contrast': '#121212' or 'rgba(0,0,0,0.87)') so preset.ts's contrast: 'var(--contrast)' and input.css's text-contrast resolve in light mode (do not rely on the .dark scope or an uncommented palette import).
🧹 Nitpick comments (1)
packages/javascript/doodle-ui/src/styleguide/components/AppIcons/components/Warning.tsx (1)
22-29: Consider using a 16×16 warning icon footprint.The Alert variant icons otherwise use a 16×16 footprint, but
Warningrenders as 16×13. This can make the warning variant sit visually shorter/misaligned next toInfo,Error, andCircleCheckin the same component set.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/javascript/doodle-ui/src/styleguide/components/AppIcons/components/Warning.tsx` around lines 22 - 29, The Warning icon currently renders in a 16×13 canvas causing visual misalignment; update the Warning component's BaseSVG props (width, height, viewBox) to use a 16×16 footprint and adjust the BasePath d coordinates so the triangle and glyph scale/position correctly within the new 0 0 16 16 viewBox (i.e., modify the BaseSVG props in the Warning component and re-layout the path in BasePath to preserve the original shape but fit the taller canvas).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/javascript/doodle-ui/src/components/Alert/Alert.stories.tsx`:
- Around line 120-136: The anonymous render callback contains a useState hook
which violates react-hooks rules; refactor the story by creating a named React
component (e.g., DismissibleAlertStory) that uses useState and returns the
current JSX, then have render return that component (render: () =>
<DismissibleAlertStory />). Ensure the component name is PascalCase (e.g.,
DismissibleAlertStory), move the useState and setVisible logic into that
component, and keep Alert, Button and their props unchanged.
In `@packages/javascript/doodle-ui/src/components/Alert/Alert.tsx`:
- Around line 68-75: In Alert.tsx the action button currently uses a hard-coded
aria-label ('Dismiss alert') which overrides the visible label and causes
incorrect accessible name; update the button in the Alert component (the JSX
that renders when action is truthy) to remove the static aria-label (or set it
dynamically to action.label) so the visible action.label is used as the
accessible name for the element.
- Around line 20-58: The file uses namespaced React types (React.HTMLAttributes,
React.ReactNode) but only imports forwardRef; add an explicit import of the
React namespace (import * as React from 'react') alongside the existing
forwardRef import so the Alert component's type annotations (seen in the Alert
forwardRef declaration and props using React.HTMLAttributes/React.ReactNode)
resolve correctly.
In `@packages/javascript/doodle-ui/src/tailwind/colors.ts`:
- Around line 140-151: The color scale for 'light-green' in colors.ts has
duplicate values at 400 and 500; update the 'light-green' object so that the key
400 is set to the correct Material Design value (`#9CCC65`) instead of the current
`#8BC34A`, ensuring the scale is continuous between 300->400->500; locate the
'light-green' mapping in the colors export and change the 400 entry to
'#9CCC65'.
- Around line 54-65: Update the cyan color palette by changing the value for the
key 300 in the cyan object from '#44DDDD' to the correct Material Design hex
'#4DD0E1'; locate the cyan object in colors.ts (the cyan: { ... } map) and
replace the 300 entry so the sequence (200, 300, 400) reads '#80DEEA',
'#4DD0E1', '#26C6DA'.
- Around line 18-29: The amber color scale has a typo: both amber.600 and
amber.700 are set to '#FFB300'; update the amber.700 entry to the correct
Material Design value '#FFA000' (locate the amber object and change the 700
property) so the shades read 600:'#FFB300', 700:'#FFA000', 800:'#FF8F00'.
---
Outside diff comments:
In `@packages/javascript/doodle-ui/src/tailwind/plugin.ts`:
- Around line 22-74: The :root block in plugin.ts removed the --contrast
variable causing var(--contrast) to fall back to currentcolor in light mode;
re-add --contrast to the :root styles (the same block where
--primary/--secondary are defined) with a sensible light-theme text color (e.g.
'--contrast': '#121212' or 'rgba(0,0,0,0.87)') so preset.ts's contrast:
'var(--contrast)' and input.css's text-contrast resolve in light mode (do not
rely on the .dark scope or an uncommented palette import).
---
Nitpick comments:
In
`@packages/javascript/doodle-ui/src/styleguide/components/AppIcons/components/Warning.tsx`:
- Around line 22-29: The Warning icon currently renders in a 16×13 canvas
causing visual misalignment; update the Warning component's BaseSVG props
(width, height, viewBox) to use a 16×16 footprint and adjust the BasePath d
coordinates so the triangle and glyph scale/position correctly within the new 0
0 16 16 viewBox (i.e., modify the BaseSVG props in the Warning component and
re-layout the path in BasePath to preserve the original shape but fit the taller
canvas).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 81e7193f-4f64-46bf-b3f4-9e5a61488931
📒 Files selected for processing (12)
packages/javascript/doodle-ui/src/components/Alert/Alert.stories.tsxpackages/javascript/doodle-ui/src/components/Alert/Alert.tsxpackages/javascript/doodle-ui/src/components/Alert/index.tspackages/javascript/doodle-ui/src/components/index.tspackages/javascript/doodle-ui/src/styleguide/components/AppIcons/components/CircleCheck.tsxpackages/javascript/doodle-ui/src/styleguide/components/AppIcons/components/Error.tsxpackages/javascript/doodle-ui/src/styleguide/components/AppIcons/components/Info.tsxpackages/javascript/doodle-ui/src/styleguide/components/AppIcons/components/Warning.tsxpackages/javascript/doodle-ui/src/styleguide/components/AppIcons/components/index.tspackages/javascript/doodle-ui/src/tailwind/colors.tspackages/javascript/doodle-ui/src/tailwind/plugin.tspackages/javascript/doodle-ui/src/tailwind/preset.ts
| cyan: { | ||
| 50: '#E0F7FA', | ||
| 100: '#B2EBF2', | ||
| 200: '#80DEEA', | ||
| 300: '#44DDDD', | ||
| 400: '#26C6DA', | ||
| 500: '#00BCD4', | ||
| 600: '#00ACC1', | ||
| 700: '#0097A7', | ||
| 800: '#00838F', | ||
| 900: '#006064', | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Material Design cyan 300 hex
💡 Result:
The hex code for Material Design Cyan 300 is #4DD0E1.
Citations:
- 1: https://materialui.co/colors/cyan/300
- 2: https://material.io/archive/guidelines/style/color.html
- 3: https://m1.material.io/style/color.html
🏁 Script executed:
cat -n packages/javascript/doodle-ui/src/tailwind/colors.ts | sed -n '50,70p'Repository: SpecterOps/BloodHound
Length of output: 664
Update cyan.300 to #4DD0E1.
The current value #44DDDD is incorrect. Material Design specifies Cyan 300 as #4DD0E1, which also maintains the proper color progression relative to 200 (#80DEEA) and 400 (#26C6DA).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/javascript/doodle-ui/src/tailwind/colors.ts` around lines 54 - 65,
Update the cyan color palette by changing the value for the key 300 in the cyan
object from '#44DDDD' to the correct Material Design hex '#4DD0E1'; locate the
cyan object in colors.ts (the cyan: { ... } map) and replace the 300 entry so
the sequence (200, 300, 400) reads '#80DEEA', '#4DD0E1', '#26C6DA'.
| 'light-green': { | ||
| 50: '#F1F8E9', | ||
| 100: '#DCEDC8', | ||
| 200: '#C5E1A5', | ||
| 300: '#AED581', | ||
| 400: '#8BC34A', | ||
| 500: '#8BC34A', | ||
| 600: '#7CB342', | ||
| 700: '#689F38', | ||
| 800: '#558B2F', | ||
| 900: '#33691E', | ||
| }, |
There was a problem hiding this comment.
light-green.400 duplicates light-green.500.
Both are #8BC34A; Material Design's light-green.400 is #9CCC65. This flattens the scale at the 400/500 step.
🛠️ Suggested fix
- 400: '#8BC34A',
+ 400: '#9CCC65',📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 'light-green': { | |
| 50: '#F1F8E9', | |
| 100: '#DCEDC8', | |
| 200: '#C5E1A5', | |
| 300: '#AED581', | |
| 400: '#8BC34A', | |
| 500: '#8BC34A', | |
| 600: '#7CB342', | |
| 700: '#689F38', | |
| 800: '#558B2F', | |
| 900: '#33691E', | |
| }, | |
| 'light-green': { | |
| 50: '#F1F8E9', | |
| 100: '#DCEDC8', | |
| 200: '#C5E1A5', | |
| 300: '#AED581', | |
| 400: '#9CCC65', | |
| 500: '#8BC34A', | |
| 600: '#7CB342', | |
| 700: '#689F38', | |
| 800: '#558B2F', | |
| 900: '#33691E', | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/javascript/doodle-ui/src/tailwind/colors.ts` around lines 140 - 151,
The color scale for 'light-green' in colors.ts has duplicate values at 400 and
500; update the 'light-green' object so that the key 400 is set to the correct
Material Design value (`#9CCC65`) instead of the current `#8BC34A`, ensuring the
scale is continuous between 300->400->500; locate the 'light-green' mapping in
the colors export and change the 400 entry to '#9CCC65'.
There was a problem hiding this comment.
These colors changes will also be applied in the Color Tokens story Leah is working on. We'll resolve any conflicts we hit. Just needed this mapped out for my story.
d1d0895 to
c3fe080
Compare
Description
Add Alert component and Storybook stories to Doodle UI
Motivation and Context
Resolves BED-5807
Why is this change required? What problem does it solve?
How Has This Been Tested?
Storybook stories and manual testing
Screenshots (optional):
Types of changes
Checklist:
Summary by CodeRabbit
New Features
Documentation