Conversation
Holocraft
added
enhancement
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughIntroduces shared ChangesUnified focus and keyboard accessibility pass
Estimated code review effort 🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/javascript/doodle-ui/src/components/DataTable/DataTable.tsx`:
- Around line 604-605: The role attribute assignment on the `<tr>` element in
the DataTable component is violating ARIA semantics by conditionally setting
role="button" when onRowClick is true. Table rows must maintain their implicit
role="row" and cannot have role="button" assigned to them. Remove the
`(onRowClick ? 'button' : undefined)` conditional logic from the role assignment
and instead only use the `tableBodyRowRole` value, allowing it to remain
undefined when not explicitly provided. The tabIndex handling on the following
line should remain unchanged as it correctly handles keyboard navigation.
In `@packages/javascript/doodle-ui/src/components/ToggleGroup/ToggleGroup.tsx`:
- Around line 30-35: The ToggleGroup component is setting rovingFocus to false
by default in its destructured parameters, which overrides Radix's default
behavior and disables keyboard navigation. Remove the `= false` default
assignment from the rovingFocus parameter in the arrow function signature so
that Radix's default value of true applies, allowing arrow-key navigation to
work unless explicitly disabled by a caller.
🪄 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: f728ee66-008e-4f5e-a7dc-ef494ad0e17b
📒 Files selected for processing (34)
packages/javascript/bh-shared-ui/src/components/ConfirmationDialog.tsxpackages/javascript/bh-shared-ui/src/components/DropdownSelector/DropdownSelector.tsxpackages/javascript/bh-shared-ui/src/components/DropdownSelector/DropdownTriggerContents.tsxpackages/javascript/bh-shared-ui/src/components/DropdownSelector/constants.tspackages/javascript/bh-shared-ui/src/components/Navigation/MainNav.tsxpackages/javascript/bh-shared-ui/src/components/NodeIcon/NodeIcon.tsxpackages/javascript/bh-shared-ui/src/components/SearchResultItem/SearchResultItem.tsxpackages/javascript/bh-shared-ui/src/components/SimpleEnvironmentSelector/SimpleEnvironmentSelector.tsxpackages/javascript/bh-shared-ui/src/constants.tspackages/javascript/bh-shared-ui/src/views/Explore/ExploreSearch/SavedQueries/SaveQueryActionMenu.tsxpackages/javascript/doodle-ui/src/components/Accordion/Accordion.tsxpackages/javascript/doodle-ui/src/components/Alert/Alert.tsxpackages/javascript/doodle-ui/src/components/Breadcrumbs/Breadcrumbs.tsxpackages/javascript/doodle-ui/src/components/Button/Button.tsxpackages/javascript/doodle-ui/src/components/Carousel/Carousel.tsxpackages/javascript/doodle-ui/src/components/Checkbox/Checkbox.tsxpackages/javascript/doodle-ui/src/components/DataTable/DataTable.tsxpackages/javascript/doodle-ui/src/components/DataTable/StorybookExample/utils.tsxpackages/javascript/doodle-ui/src/components/Dialog/Dialog.tsxpackages/javascript/doodle-ui/src/components/Input/Input.tsxpackages/javascript/doodle-ui/src/components/Link/Link.tsxpackages/javascript/doodle-ui/src/components/Menu/Menu.tsxpackages/javascript/doodle-ui/src/components/Pagination/Pagination.tsxpackages/javascript/doodle-ui/src/components/Popover/Popover.tsxpackages/javascript/doodle-ui/src/components/RadioGroup/RadioGroup.tsxpackages/javascript/doodle-ui/src/components/Select/Select.tsxpackages/javascript/doodle-ui/src/components/Switch/Switch.tsxpackages/javascript/doodle-ui/src/components/Table/Table.tsxpackages/javascript/doodle-ui/src/components/Tabs/Tabs.tsxpackages/javascript/doodle-ui/src/components/Textarea/Textarea.tsxpackages/javascript/doodle-ui/src/components/Toggle/Toggle.tsxpackages/javascript/doodle-ui/src/components/ToggleGroup/ToggleGroup.tsxpackages/javascript/doodle-ui/src/components/Tooltip/Tooltip.tsxpackages/javascript/doodle-ui/src/tailwind/plugin.ts
| 'active:ring-0 active:bg-[#C0C6CB] dark:active:bg-neutral-2', | ||
| 'focus:text-[#121212] dark:focus:text-white', | ||
| 'focus:ring-2 focus:ring-offset-2 focus:ring-secondary dark:focus:ring-offset-[#1F1F1F]', | ||
| 'focus-visible:text-[#121212] dark:focus-visible:text-white', |
Contributor
There was a problem hiding this comment.
Using the new color token of text-main will allow for dark:focus-visible:text-white to be removed.
| <div | ||
| className='flex items-center justify-center border border-neutral-dark-1 rounded-full p-1 size-[22px] text-neutral-dark-1 bg-neutral-light-1 pointer-events-none' | ||
| className={cn( | ||
| 'flex items-center justify-center border border-neutral-dark-1 rounded-full p-1 size-[22px] text-neutral-dark-1 bg-neutral-light-1 pointer-events-none', |
Contributor
There was a problem hiding this comment.
same here with border-neutral-dark-1 and text-neutral-dark-1 - to common-dark.
common-dark will need to be uncommented from preset.ts and plugin.ts
same here with border-neutral-light-1 - to common-white.
common-white will need to be uncommented from preset.ts and plugin.ts
| style={style} | ||
| className={cn( | ||
| 'hover:bg-secondary hover:text-neutral-light-1 hover:dark:bg-secondary-variant-2 hover:dark:text-neutral-dark-1', | ||
| 'group hover:bg-secondary hover:text-neutral-light-1 hover:dark:bg-secondary-variant-2 hover:dark:text-neutral-dark-1 focus:bg-secondary focus:text-neutral-light-1 focus:dark:bg-secondary-variant-2 focus:dark:text-neutral-dark-1 focus-visible:bg-secondary focus-visible:text-neutral-light-1 focus-visible:dark:bg-secondary-variant-2 focus-visible:dark:text-neutral-dark-1', |
Contributor
There was a problem hiding this comment.
Same here - see above
| 'hover:bg-secondary hover:text-neutral-light-1 hover:dark:bg-secondary-variant-2 hover:dark:text-neutral-dark-1', | ||
| 'group hover:bg-secondary hover:text-neutral-light-1 hover:dark:bg-secondary-variant-2 hover:dark:text-neutral-dark-1 focus:bg-secondary focus:text-neutral-light-1 focus:dark:bg-secondary-variant-2 focus:dark:text-neutral-dark-1 focus-visible:bg-secondary focus-visible:text-neutral-light-1 focus-visible:dark:bg-secondary-variant-2 focus-visible:dark:text-neutral-dark-1', | ||
| { | ||
| 'bg-secondary text-neutral-light-1 dark:bg-secondary-variant-2 dark:text-neutral-dark-1': |
Contributor
There was a problem hiding this comment.
also here - use new color tokens
| <PopoverTrigger | ||
| aria-label='Show save query options' | ||
| className='inline-flex items-center justify-center whitespace-nowrap rounded-3xl text-sm ring-offset-background transition-colors hover:underline focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 disabled:pointer-events-none disabled:opacity-50 active:no-underline bg-neutral-light-5 text-neutral-dark-0 shadow-outer-1 hover:bg-secondary hover:text-white h-9 px-4 py-1 text-xs rounded-l-none pl-2 -ml-1 dark:text-neutral-dark-1 dark:hover:text-white' | ||
| className='inline-flex items-center justify-center whitespace-nowrap rounded-3xl text-sm ring-offset-focus-offset transition-colors hover:underline focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-focus focus-visible:ring-offset-2 disabled:pointer-events-none disabled:opacity-50 active:no-underline bg-neutral-light-5 text-neutral-dark-0 shadow-outer-1 hover:bg-secondary hover:text-white focus-visible:bg-secondary focus-visible:text-white h-9 px-4 py-1 text-xs rounded-l-none pl-2 -ml-1 dark:text-neutral-dark-1 dark:hover:text-white' |
Contributor
There was a problem hiding this comment.
dark and light mode styles can be reduced if using the new tailwind presets
|
|
||
| const CheckboxVariants = cva( | ||
| 'peer shrink-0 rounded-sm border-2 border-neutral-dark-1 dark:border-neutral-light-1 ring-offset-background focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-offset-2 disabled:cursor-not-allowed disabled:opacity-50 data-[state=checked]:bg-neutral-dark-1 data-[state=checked]:text-neutral-light-1 dark:data-[state=checked]:bg-neutral-light-1 dark:data-[state=checked]:text-neutral-dark-1', | ||
| 'peer shrink-0 rounded-sm border-2 border-neutral-dark-1 dark:border-neutral-light-1 focus:outline-none focus-visible:focus-ring disabled:cursor-not-allowed disabled:opacity-50 data-[state=checked]:bg-neutral-dark-1 data-[state=checked]:text-neutral-light-1 dark:data-[state=checked]:bg-neutral-light-1 dark:data-[state=checked]:text-neutral-dark-1', |
Contributor
There was a problem hiding this comment.
same here with new color tokens
| '--primary': dark.primary.main, | ||
| '--primary-variant': dark.primary.variant, | ||
| '--secondary': dark.secondary.main, | ||
| '--secondary-main': dark.secondary.main, |
Contributor
There was a problem hiding this comment.
this style is redundant to the style above it.
Contributor
There was a problem hiding this comment.
After a meeting, we decided to keep both variable names as a failsafe - but this requires a change to preset.
| '--primary': light.primary.main, | ||
| '--primary-variant': light.primary.variant, | ||
| '--secondary': light.secondary.main, | ||
| '--secondary-main': light.secondary.main, |
Contributor
There was a problem hiding this comment.
I am noticing a discrepancy in the preset.ts file for secondary.
keep 'secondary', but remove 'secondary-main' and match the same for the below dark mode.
in preset - can you change secondary: 'var(--secondary-main)' to secondary: 'var(--secondary)' ?
Contributor
There was a problem hiding this comment.
After a meeting, we decided to keep both variable names as a failsafe - but this requires a change to preset.
| '.focus-ring-inset': { | ||
| outline: 'none', | ||
| 'box-shadow': 'inset 0 0 0 var(--focus-ring-width) var(--focus-ring), var(--tw-shadow, 0 0 #0000)', | ||
| }, |
| // TODO: Replace hardcoded hex colors with design token CSS variables once the token system is ready. | ||
| const ToggleVariants = cva( | ||
| 'inline-flex items-center justify-center rounded-lg text-sm transition-colors gap-2 [&_svg]:pointer-events-none [&_svg]:size-4 [&_svg]:shrink-0 border border-[#C0C6CB] bg-neutral-1 hover:border-[#4A3BD7] hover:bg-[#4A3BD7] hover:text-neutral-1 hover:underline focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-offset-2 focus-visible:ring-[#4A3BD7] focus-visible:ring-offset-neutral-1 focus-visible:border-[#4A3BD7] focus-visible:bg-[#4A3BD7] focus-visible:text-neutral-1 active:border-[#0D0A30] active:bg-[#0D0A30] active:text-neutral-1 data-[state=on]:border-[#2C2677] data-[state=on]:bg-[#2C2677] data-[state=on]:text-neutral-1 disabled:cursor-not-allowed disabled:border-[#E3E7EA] disabled:bg-[#E3E7EA] disabled:text-[#616161] dark:border-[#121212] dark:bg-[#121212] dark:hover:border-[#66A3FF] dark:hover:bg-[#66A3FF] dark:hover:text-neutral-dark-1 dark:focus-visible:ring-[#66A3FF] dark:focus-visible:ring-offset-neutral-dark-1 dark:focus-visible:border-[#66A3FF] dark:focus-visible:bg-[#66A3FF] dark:focus-visible:text-neutral-dark-1 dark:active:border-[#8D8BF8] dark:active:bg-[#8D8BF8] dark:data-[state=on]:border-[#A1A0FF] dark:data-[state=on]:bg-[#A1A0FF] dark:data-[state=on]:text-neutral-dark-1 dark:disabled:border-[#2E2E2E] dark:disabled:bg-[#2E2E2E] dark:disabled:text-[#A6A6A6]', | ||
| 'inline-flex items-center justify-center rounded text-sm transition-colors gap-2 [&_svg]:pointer-events-none [&_svg]:size-4 [&_svg]:shrink-0 border border-[#C0C6CB] bg-neutral-1 text-neutral-dark-1 hover:border-[#4A3BD7] hover:bg-[#4A3BD7] hover:text-neutral-1 hover:underline focus:outline-none focus-visible:focus-ring focus-visible:border-[#4A3BD7] focus-visible:bg-[#4A3BD7] focus-visible:text-neutral-1 active:border-[#0D0A30] active:bg-[#0D0A30] active:text-neutral-1 data-[state=on]:border-[#2C2677] data-[state=on]:bg-[#2C2677] data-[state=on]:text-neutral-1 disabled:cursor-not-allowed disabled:border-[#E3E7EA] disabled:bg-[#E3E7EA] disabled:text-[#616161] dark:border-[#121212] dark:bg-[#121212] dark:text-neutral-light-1 dark:hover:border-[#66A3FF] dark:hover:bg-[#66A3FF] dark:hover:text-neutral-dark-1 dark:focus-visible:border-[#66A3FF] dark:focus-visible:bg-[#66A3FF] dark:focus-visible:text-neutral-dark-1 dark:active:border-[#8D8BF8] dark:active:bg-[#8D8BF8] dark:active:text-neutral-dark-1 dark:data-[state=on]:border-[#A1A0FF] dark:data-[state=on]:bg-[#A1A0FF] dark:data-[state=on]:text-neutral-dark-1 dark:disabled:border-[#2E2E2E] dark:disabled:bg-[#2E2E2E] dark:disabled:text-[#A6A6A6]', |
Contributor
There was a problem hiding this comment.
Many of these HEX codes and dark mode classing can be replaced with the new tokens
Contributor
|
There's a lot of styling that isn't using the new color tokens - I commented on most of them. First thing I noticed visually was the color contrast and that the focus indicator loses it's border-radius when open: 
|
| variant={variant ?? 'transparent'} | ||
| className={cn( | ||
| 'uppercase', | ||
| 'uppercase group hover:[&_svg]:text-white hover:[&_svg]:fill-current hover:[&_svg_*]:text-white hover:[&_svg_*]:fill-current focus:[&_svg]:text-white focus:[&_svg]:fill-current focus:[&_svg_*]:text-white focus:[&_svg_*]:fill-current focus-visible:[&_svg]:text-white focus-visible:[&_svg]:fill-current focus-visible:[&_svg_*]:text-white focus-visible:[&_svg_*]:fill-current', |
Contributor
There was a problem hiding this comment.
a lot of these styles are included in the triggerStyles - thinking this could be DRYed up.
| '--primary': light.primary.main, | ||
| '--primary-variant': light.primary.variant, | ||
| '--secondary': light.secondary.main, | ||
| '--secondary-main': light.secondary.main, |
Contributor
There was a problem hiding this comment.
After a meeting, we decided to keep both variable names as a failsafe - but this requires a change to preset.
| '--primary': dark.primary.main, | ||
| '--primary-variant': dark.primary.variant, | ||
| '--secondary': dark.secondary.main, | ||
| '--secondary-main': dark.secondary.main, |
Contributor
There was a problem hiding this comment.
After a meeting, we decided to keep both variable names as a failsafe - but this requires a change to preset.
| export const optionStyles = | ||
| 'px-4 py-1 rounded-none w-full justify-normal hover:no-underline hover:bg-neutral-4 disabled:bg-neutral-4'; | ||
| 'px-4 py-1 rounded-none w-full justify-normal hover:no-underline hover:bg-neutral-4 hover:[&_svg]:text-white hover:[&_svg]:fill-current hover:[&_svg_*]:text-white hover:[&_svg_*]:fill-current focus:bg-primary focus:text-white focus:[&_svg]:text-white focus:[&_svg]:fill-current focus:[&_svg_*]:text-white focus:[&_svg_*]:fill-current focus-visible:bg-primary focus-visible:text-white focus-visible:[&_svg]:text-white focus-visible:[&_svg]:fill-current focus-visible:[&_svg_*]:text-white focus-visible:[&_svg_*]:fill-current disabled:bg-neutral-4 group'; | ||
|
|
Contributor
There was a problem hiding this comment.
Many repeating class name here from above - can this be DRYed up?
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
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)
404-423:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThe
outlineproperty doesn't respect border-radius, causing visual inconsistency.The
.focus-ringutility applies bothoutlineandbox-shadow, but CSSoutlinedoes not respectborder-radius. This matches the concern raised in the PR objectives about "border-radius loss" — when applied to rounded elements (buttons, inputs), theoutlinewill remain square while thebox-shadowring follows the border-radius, creating a mismatched appearance.Consider one of these alternatives:
Remove outline, use only box-shadow (simplest, but may be clipped by
overflow: hiddencontainers):'.focus-ring': { '--tw-ring-offset-width': 'var(--focus-ring-offset-width)', '--tw-ring-offset-color': 'var(--focus-ring-offset)', '--tw-ring-color': 'var(--focus-ring)', '--tw-ring-width': 'var(--focus-ring-width)', '--tw-ring-offset-shadow': '0 0 0 var(--tw-ring-offset-width) var(--tw-ring-offset-color)', '--tw-ring-shadow': '0 0 0 calc(var(--focus-ring-width) + var(--tw-ring-offset-width)) var(--tw-ring-color)', 'box-shadow': 'var(--tw-ring-offset-shadow), var(--tw-ring-shadow), var(--tw-shadow, 0 0 `#0000`)', }Increase outline-offset to visually separate it from the element (outline remains square but is pushed out further)
Use a pseudo-element with border instead of outline (more complex but respects border-radius)
[accessibility_and_usability]
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/javascript/doodle-ui/src/tailwind/plugin.ts` around lines 404 - 423, The `.focus-ring` utility combines both outline and box-shadow for focus styling, but CSS outline does not respect border-radius, causing a square outline on rounded elements while the box-shadow follows the curve. Remove the outline and outline-offset properties from the `.focus-ring` utility definition in the addUtilities block, keeping only the box-shadow implementation along with the CSS custom properties that support it. This ensures the focus ring adapts consistently to the element's border-radius for a visually cohesive appearance.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/javascript/doodle-ui/src/tailwind/preset.ts`:
- Around line 33-46: The tertiary color mapping in the colors object is pointing
to an undefined CSS variable `--tertiary-main`. The correct CSS variable defined
in plugin.ts is `--tertiary`. Update the tertiary property in the colors object
to map to 'var(--tertiary)' instead of 'var(--tertiary-main)' to reference the
correct variable definition.
---
Outside diff comments:
In `@packages/javascript/doodle-ui/src/tailwind/plugin.ts`:
- Around line 404-423: The `.focus-ring` utility combines both outline and
box-shadow for focus styling, but CSS outline does not respect border-radius,
causing a square outline on rounded elements while the box-shadow follows the
curve. Remove the outline and outline-offset properties from the `.focus-ring`
utility definition in the addUtilities block, keeping only the box-shadow
implementation along with the CSS custom properties that support it. This
ensures the focus ring adapts consistently to the element's border-radius for a
visually cohesive appearance.
🪄 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: ebc07c5a-ec61-4782-955c-c883bc2f1612
📒 Files selected for processing (2)
packages/javascript/doodle-ui/src/tailwind/plugin.tspackages/javascript/doodle-ui/src/tailwind/preset.ts
Comment on lines
+33
to
+46
| primary: 'var(--primary-main)', | ||
| 'primary-main': | ||
| 'var(--primary-main)' /* failsafe for any classes possibly using the optional '-main' suffix*/, | ||
| 'primary-variant': 'var(--primary-variant)', | ||
|
|
||
| secondary: 'var(--secondary-main)', | ||
| 'secondary-main': | ||
| 'var(--secondary-main)' /* failsafe for any classes possibly using the optional '-main' suffix*/, | ||
| 'secondary-variant': 'var(--secondary-variant)', | ||
| 'secondary-variant-2': 'var(--secondary-variant-2)', | ||
|
|
||
| tertiary: 'var(--tertiary)', | ||
| tertiary: 'var(--tertiary-main)', | ||
| 'tertiary-main': | ||
| 'var(--tertiary-main)' /* failsafe for any classes possibly using the optional '-main' suffix*/, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Search for usage of tertiary color classes
# Search for Tailwind classes using tertiary color
rg -n '\b(bg|text|border|ring)-tertiary\b' packages/javascript/doodle-ui/src --type tsx --type ts -C2
# Check if tertiary is referenced in component styles
ast-grep --pattern 'tertiary'Repository: SpecterOps/BloodHound
Length of output: 252
🏁 Script executed:
#!/bin/bash
# Check the plugin.ts file to see if --tertiary-main is commented out
fd -type f -name "plugin.ts" packages/javascript/doodle-ui/
# Get the specific lines in plugin.ts
sed -n '40,60p' packages/javascript/doodle-ui/src/tailwind/plugin.ts
sed -n '240,250p' packages/javascript/doodle-ui/src/tailwind/plugin.tsRepository: SpecterOps/BloodHound
Length of output: 1775
🏁 Script executed:
#!/bin/bash
# Search for all tertiary color usage across the codebase
rg -n 'tertiary' packages/javascript --type ts --type tsx | head -30Repository: SpecterOps/BloodHound
Length of output: 93
🏁 Script executed:
#!/bin/bash
# Check the ThemeCSSVars definition to understand how tertiary is exposed
rg -n 'ThemeCSSVars' packages/javascript --type ts --type tsx -A 3 -B 1 | head -50Repository: SpecterOps/BloodHound
Length of output: 93
🏁 Script executed:
#!/bin/bash
# Find ThemeCSSVars definition
fd -type f -name "*.ts" -o -name "*.tsx" | xargs grep -l "ThemeCSSVars" | head -5Repository: SpecterOps/BloodHound
Length of output: 236
🏁 Script executed:
#!/bin/bash
# Look at useTheme.tsx to see how tertiary is handled
cat packages/javascript/bh-shared-ui/src/hooks/useTheme.tsx | grep -A 5 -B 5 "tertiary"Repository: SpecterOps/BloodHound
Length of output: 2761
🏁 Script executed:
#!/bin/bash
# Check if there are any other theme color definitions
fd -type f -name "*.ts" packages/javascript/doodle-ui/src/tailwind -exec cat {} \; | grep -n "tertiary" | head -20Repository: SpecterOps/BloodHound
Length of output: 236
🏁 Script executed:
#!/bin/bash
# Look for the ThemeCSSVars enum/object definition
rg -n "const ThemeCSSVars\|enum ThemeCSSVars\|export.*ThemeCSSVars" packages/javascript -A 10 | head -40Repository: SpecterOps/BloodHound
Length of output: 47
🏁 Script executed:
#!/bin/bash
# Search for '--tertiary' variable definition (without -main suffix)
rg -n '\-\-tertiary[^-]' packages/javascript/doodle-ui/Repository: SpecterOps/BloodHound
Length of output: 263
🏁 Script executed:
#!/bin/bash
# Look at the complete preset.ts to understand color mappings
cat packages/javascript/doodle-ui/src/tailwind/preset.ts | head -60Repository: SpecterOps/BloodHound
Length of output: 2369
🏁 Script executed:
#!/bin/bash
# Check how primary color is defined in plugin.ts and preset.ts to compare
rg -n "primary" packages/javascript/doodle-ui/src/tailwind/preset.ts | head -10Repository: SpecterOps/BloodHound
Length of output: 861
🏁 Script executed:
#!/bin/bash
# Check if preset.ts extends or inherits from a base config
head -100 packages/javascript/doodle-ui/src/tailwind/preset.tsRepository: SpecterOps/BloodHound
Length of output: 4068
Critical: tertiary color in preset.ts maps to undefined CSS variable.
Line 44 maps tertiary: 'var(--tertiary-main)', but --tertiary-main is commented out in plugin.ts (lines 51, 244). However, plugin.ts line 205 defines --tertiary: '#02c577', which is the correct variable to use.
Change preset.ts line 44 from tertiary: 'var(--tertiary-main)' to tertiary: 'var(--tertiary)'. Alternatively, uncomment the correct mapping at preset.ts line 85.
Current incorrect code (preset.ts line 44)
tertiary: 'var(--tertiary-main)',
'tertiary-main':
'var(--tertiary-main)' /* failsafe for any classes possibly using the optional '-main' suffix*/,
'tertiary-variant': 'var(--tertiary-variant)',
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/javascript/doodle-ui/src/tailwind/preset.ts` around lines 33 - 46,
The tertiary color mapping in the colors object is pointing to an undefined CSS
variable `--tertiary-main`. The correct CSS variable defined in plugin.ts is
`--tertiary`. Update the tertiary property in the colors object to map to
'var(--tertiary)' instead of 'var(--tertiary-main)' to reference the correct
variable definition.
TheNando
approved these changes
Jun 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This changeset adds focus state styling to all general inputs: buttons, text fields, toggles, etc. to adhere to WCAG standards.
Motivation and Context
Resolves BED-7215
How Has This Been Tested?
Manually
Screenshots (optional):
Types of changes
Checklist:
Summary by CodeRabbit
New Features
.focus-ringand.focus-ring-insetTailwind utilities for consistent keyboard focus indicators.Style / Accessibility
Theme