Progress bar Sizes #1153
Progress bar Sizes #1153
Conversation
WalkthroughSupport for multiple size and variant options was added to the Progress component. This includes extending context and props with Additionally, new UI components Checkbox and Slider were introduced with their contexts, subcomponents, stories, and styles. The Select and SelectPrimitive components were extended with a new Search subcomponent enabling searchable dropdowns. The CodeBlock component was enhanced with scrollable code area support. Several primitive checkbox components and their stories were added. Styling updates were made for checkbox, select search, and slider components. Minor formatting and newline fixes were applied. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ProgressRoot
participant ProgressContext
participant Styles
User->>ProgressRoot: Render with size and variant props
ProgressRoot->>ProgressContext: Provide size and variant in context
ProgressRoot->>ProgressRoot: Generate data-progress-size and data-progress-variant attributes
ProgressRoot->>Styles: Apply styles based on data attributes
sequenceDiagram
participant User
participant SliderRoot
participant SliderContext
participant SliderTrack
participant SliderThumb
participant SliderRange
User->>SliderRoot: Interact (click, pointer events)
SliderRoot->>SliderContext: Update value state
SliderRoot->>SliderTrack: Render track
SliderRoot->>SliderRange: Render filled range based on value
SliderRoot->>SliderThumb: Render thumb positioned by value
sequenceDiagram
participant User
participant SelectPrimitiveRoot
participant SelectPrimitiveSearch
participant SelectPrimitiveContent
participant SelectPrimitiveItem
User->>SelectPrimitiveSearch: Type search query
SelectPrimitiveSearch->>SelectPrimitiveContent: Filter items
User->>SelectPrimitiveContent: Navigate with keyboard
SelectPrimitiveContent->>SelectPrimitiveItem: Highlight active item
User->>SelectPrimitiveContent: Press Enter
SelectPrimitiveContent->>SelectPrimitiveRoot: Trigger handleSelect with active item
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
| minValue: number; | ||
| maxValue: number; | ||
| children: React.ReactNode; | ||
| customRootClass?: string; | ||
| } | ||
|
|
||
| const ProgressRoot = ({ value = 0, minValue = 0, maxValue = 100, children, customRootClass }: ProgressRootProps) => { | ||
| const ProgressRoot = ({ value = 0, minValue = 0, maxValue = 100,size='', children, customRootClass }: ProgressRootProps) => { |
There was a problem hiding this comment.
size should have default value
There was a problem hiding this comment.
no, it wont be headless if it has defaults
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/ui/Progress/contexts/ProgressContext.tsx (1)
17-18: Empty-string defaults conflict with the “optional” contract
sizeandvariantare declared optional but the default context value sets them to''. Components that rely on truthiness (if (size) …) will treat''as “absent” but TypeScript distinguishesundefinedfrom the empty string, so the runtime behaviour and the type contract diverge.Consider either
- removing those keys from the default object altogether, or
- initialising them with
undefined:- size: '', - variant: '', + // size and variant intentionally left undefined; they will be supplied by <ProgressRoot>This keeps the semantics aligned and avoids accidental styling overrides caused by an empty data-attribute value.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/components/ui/Progress/contexts/ProgressContext.tsx(1 hunks)src/components/ui/Progress/fragments/ProgressRoot.tsx(1 hunks)src/components/ui/Progress/tests/Progress.test.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/ui/Progress/fragments/ProgressRoot.tsx
- src/components/ui/Progress/tests/Progress.test.tsx
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
| <ProgressContext.Provider value={sendValues}> | ||
| <Primitive.div | ||
| className={clsx(rootClass)} | ||
| data-testid={`${COMPONENT_NAME.toLowerCase()}-bar-component`} |
There was a problem hiding this comment.
what are we using this for ?
There was a problem hiding this comment.
to write test cases to check the attributes
There was a problem hiding this comment.
The main component shouldn't have any extra attributes, the tests should pass this attribute if required.
There was a problem hiding this comment.
Ok will make that change
There was a problem hiding this comment.
I have to remove the test cases for the component as I have to access the first element in the progress component but neither does it have a role or a key that I can access it with. Thats why I have added the testId, the progressbar role accesses the inside element where the data attributes are not present.
There was a problem hiding this comment.
you shouldnt be adding attributes that will help your tests pass
every attribute that is shipped with a component carries value and meaning - arbitrary/low value attributes cannot be afforded to be shipped with the library
what you might find useful for testing, the user will have absolutely no use for it and components need to be added keeping the user/dev in mind
There was a problem hiding this comment.
Got it I will remove them.
| &[data-progress-size="x-large"] { | ||
| height: 16px; | ||
|
|
||
| .rad-ui-progress-indicator { |
There was a problem hiding this comment.
I will change that
|
I have removed the prev testId attribute and simplified the recurring code in scss. |
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
|
lint issues |
There was a problem hiding this comment.
Actionable comments posted: 23
🔭 Outside diff range comments (2)
docs/components/layout/Documentation/helpers/CodeBlock.js (1)
45-61: Remove unused height/maxHeight logic.The height and maxHeight variables are calculated but no longer used in the ScrollArea implementation, making this code redundant.
Apply this diff to remove the unused logic:
-let height = 'auto'; -let maxHeight = 'auto'; - -if(expanded) { - if(!inline) { - height = 'auto'; - maxHeight = 640; - } - -} -else{ - if(!inline) { - height = 180; - maxHeight = 640; - } - -}src/core/primitives/Select/fragments/SelectPrimitiveContent.tsx (1)
2-2: Remove unused imports.The linting pipeline correctly identifies unused imports:
useEffect,Primitive,setIsOpen, andselectedValue.-import React, { useContext, useEffect } from 'react'; +import React, { useContext } from 'react';Also remove the unused
Primitiveimport:-import Primitive from '../../Primitive';
🧹 Nitpick comments (7)
docs/components/layout/Documentation/helpers/CodeBlock.js (1)
101-102: Remove unnecessary blank lines.These extra blank lines at the end of the component are unnecessary and should be removed for cleaner code.
Apply this diff to remove the extra blank lines:
-src/components/ui/Slider/fragments/SliderRange.tsx (1)
6-10: Consider if children prop is necessary for this component.The
SliderRangecomponent appears to be a visual indicator for the filled portion of the slider. Consider whether thechildrenprop is actually needed for this use case, as range indicators typically don't contain child elements.-const SliderRange = ({ children }: { children: React.ReactNode }) => { +const SliderRange = () => { const { rootClass, value } = React.useContext(SliderContext); - return <div className={`${rootClass}-range`} style={{ left: `calc(${value}% - 16px)`, width: `calc(${value}%)` }}>{children}</div>; + return <div className={`${rootClass}-range`} style={{ left: `calc(${value}% - 16px)`, width: `calc(${value}%)` }} />; };src/components/ui/Slider/fragments/SliderThumb.tsx (1)
3-3: Fix unused import warning.The linting warning about
useRefbeing unused is a false positive since you're usingReact.useRefdirectly. Update the import to fix this warning.-import React, { useRef } from 'react'; +import React from 'react';src/core/primitives/Select/fragments/SelectPrimitiveContent.tsx (1)
38-47: Enhance keyboard handling robustness.The keyboard handling logic works but could be improved for better reliability and error handling.
onKeyDownCapture={(e) => { if (e.key === 'Enter') { e.preventDefault(); - const itemValue = refs.floating.current.querySelector('[data-active="true"]'); + const itemValue = refs.floating.current?.querySelector('[data-active="true"]'); if (itemValue) { - handleSelect((itemValue.getAttribute('data-value'))) + const value = itemValue.getAttribute('data-value'); + if (value) { + handleSelect(value); + } } } }}src/core/primitives/Select/fragments/SelectPrimitiveSearch.tsx (1)
57-57: Remove console.log statements.Debug console.log statements should be removed from production code.
- console.log('arrow input taken');- console.log('activeItemValue', activeItemValue);Also applies to: 81-81
src/components/ui/Checkbox/stories/Checkbox.stories.tsx (2)
11-13: Consider extracting the SVG icon for better maintainability.The inline SVG implementation is correct but could be improved for maintainability. Consider extracting the TickIcon to a shared icons directory or constants file if it's used elsewhere in the codebase.
-const TickIcon = () => { - return <svg width="15" height="15" viewBox="0 0 15 15" fill="none" xmlns="http://www.w3.org/2000/svg"><path d="M11.4669 3.72684C11.7558 3.91574 11.8369 4.30308 11.648 4.59198L7.39799 11.092C7.29783 11.2452 7.13556 11.3467 6.95402 11.3699C6.77247 11.3931 6.58989 11.3355 6.45446 11.2124L3.70446 8.71241C3.44905 8.48022 3.43023 8.08494 3.66242 7.82953C3.89461 7.57412 4.28989 7.55529 4.5453 7.78749L6.75292 9.79441L10.6018 3.90792C10.7907 3.61902 11.178 3.53795 11.4669 3.72684Z" fill="currentColor" fill-rule="evenodd" clip-rule="evenodd"></path></svg>; -}; +const TickIcon = () => ( + <svg width="15" height="15" viewBox="0 0 15 15" fill="none" xmlns="http://www.w3.org/2000/svg"> + <path + d="M11.4669 3.72684C11.7558 3.91574 11.8369 4.30308 11.648 4.59198L7.39799 11.092C7.29783 11.2452 7.13556 11.3467 6.95402 11.3699C6.77247 11.3931 6.58989 11.3355 6.45446 11.2124L3.70446 8.71241C3.44905 8.48022 3.43023 8.08494 3.66242 7.82953C3.89461 7.57412 4.28989 7.55529 4.5453 7.78749L6.75292 9.79441L10.6018 3.90792C10.7907 3.61902 11.178 3.53795 11.4669 3.72684Z" + fill="currentColor" + fillRule="evenodd" + clipRule="evenodd" + /> + </svg> +);
15-23: Consider removing unnecessary wrapper div.The CheckboxStory component structure is correct and demonstrates the compound component pattern well. However, the outer div wrapper appears unnecessary and could be removed for cleaner markup.
const CheckboxStory = () => { - return <div> - <Checkbox.Root> - <Checkbox.Indicator> - <TickIcon /> - </Checkbox.Indicator> - </Checkbox.Root> - </div>; + return ( + <Checkbox.Root> + <Checkbox.Indicator> + <TickIcon /> + </Checkbox.Indicator> + </Checkbox.Root> + ); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (35)
docs/app/docs/layout.tsx(1 hunks)docs/components/layout/Documentation/helpers/CodeBlock.js(3 hunks)src/components/ui/Checkbox/Checkbox.tsx(1 hunks)src/components/ui/Checkbox/context/CheckboxContext.tsx(1 hunks)src/components/ui/Checkbox/fragments/CheckboxIndicator.tsx(1 hunks)src/components/ui/Checkbox/fragments/CheckboxRoot.tsx(1 hunks)src/components/ui/Checkbox/stories/Checkbox.stories.tsx(1 hunks)src/components/ui/Select/Select.tsx(2 hunks)src/components/ui/Select/fragments/SelectSearch.tsx(1 hunks)src/components/ui/Select/stories/Select.stories.tsx(1 hunks)src/components/ui/Slider/Slider.tsx(1 hunks)src/components/ui/Slider/context/SliderContext.tsx(1 hunks)src/components/ui/Slider/fragments/SliderRange.tsx(1 hunks)src/components/ui/Slider/fragments/SliderRoot.tsx(1 hunks)src/components/ui/Slider/fragments/SliderThumb.tsx(1 hunks)src/components/ui/Slider/fragments/SliderTrack.tsx(1 hunks)src/components/ui/Slider/stories/Slider.stories.tsx(1 hunks)src/core/primitives/Checkbox/context/CheckboxPrimitiveContext.tsx(1 hunks)src/core/primitives/Checkbox/fragments/CheckboxPrimitiveIndicator.tsx(1 hunks)src/core/primitives/Checkbox/fragments/CheckboxPrimitiveRoot.tsx(1 hunks)src/core/primitives/Checkbox/fragments/CheckboxPrimitiveTrigger.tsx(1 hunks)src/core/primitives/Checkbox/index.tsx(1 hunks)src/core/primitives/Checkbox/stories/CheckboxPrimitive.stories.tsx(1 hunks)src/core/primitives/Select/Select.tsx(2 hunks)src/core/primitives/Select/contexts/SelectPrimitiveContext.tsx(1 hunks)src/core/primitives/Select/fragments/SelectPrimitiveContent.tsx(2 hunks)src/core/primitives/Select/fragments/SelectPrimitiveItem.tsx(2 hunks)src/core/primitives/Select/fragments/SelectPrimitiveRoot.tsx(4 hunks)src/core/primitives/Select/fragments/SelectPrimitiveSearch.tsx(1 hunks)src/core/primitives/Select/stories/Select.stories.tsx(1 hunks)styles/jsTokens/colors.tokens.js(1 hunks)styles/themes/components/checkbox.scss(1 hunks)styles/themes/components/select.scss(2 hunks)styles/themes/components/slider.scss(1 hunks)styles/themes/default.scss(2 hunks)
✅ Files skipped from review due to trivial changes (16)
- docs/app/docs/layout.tsx
- src/core/primitives/Checkbox/context/CheckboxPrimitiveContext.tsx
- src/components/ui/Checkbox/context/CheckboxContext.tsx
- styles/themes/default.scss
- src/components/ui/Select/Select.tsx
- src/core/primitives/Select/Select.tsx
- src/components/ui/Slider/fragments/SliderTrack.tsx
- src/core/primitives/Checkbox/index.tsx
- styles/themes/components/slider.scss
- styles/themes/components/checkbox.scss
- src/components/ui/Slider/Slider.tsx
- src/components/ui/Checkbox/fragments/CheckboxRoot.tsx
- src/components/ui/Slider/context/SliderContext.tsx
- src/components/ui/Slider/stories/Slider.stories.tsx
- src/core/primitives/Checkbox/stories/CheckboxPrimitive.stories.tsx
- styles/jsTokens/colors.tokens.js
🧰 Additional context used
🧠 Learnings (18)
📓 Common learnings
Learnt from: decipher-cs
PR: rad-ui/ui#417
File: src/components/ui/Dropdown/Dropdown.stories.tsx:43-50
Timestamp: 2024-12-12T08:34:33.079Z
Learning: Ensure to verify existing ARIA attributes in components before suggesting additions during code reviews, especially in the `Dropdown.Trigger` component in `src/components/ui/Dropdown/Dropdown.stories.tsx`.
src/components/ui/Select/stories/Select.stories.tsx (2)
Learnt from: decipher-cs
PR: rad-ui/ui#417
File: src/components/ui/Dropdown/Dropdown.stories.tsx:43-50
Timestamp: 2024-12-12T08:34:33.079Z
Learning: Ensure to verify existing ARIA attributes in components before suggesting additions during code reviews, especially in the `Dropdown.Trigger` component in `src/components/ui/Dropdown/Dropdown.stories.tsx`.
Learnt from: decipher-cs
PR: rad-ui/ui#417
File: src/components/ui/Dropdown/Dropdown.tsx:0-0
Timestamp: 2024-12-12T08:22:59.375Z
Learning: The `Dropdown.Trigger` component is customizable and needs to be used with `Dropdown.Root`.
src/core/primitives/Checkbox/fragments/CheckboxPrimitiveIndicator.tsx (2)
Learnt from: kotAPI
PR: rad-ui/ui#576
File: src/core/primitives/Toggle/index.tsx:15-22
Timestamp: 2024-11-24T06:43:42.194Z
Learning: In the `TogglePrimitive` component (`src/core/primitives/Toggle/index.tsx`), when the component becomes controlled, it's acceptable to not sync the internal `isPressed` state with the external `pressed` prop.
Learnt from: decipher-cs
PR: rad-ui/ui#417
File: src/components/ui/Dropdown/Dropdown.stories.tsx:43-50
Timestamp: 2024-12-12T08:34:33.079Z
Learning: Ensure to verify existing ARIA attributes in components before suggesting additions during code reviews, especially in the `Dropdown.Trigger` component in `src/components/ui/Dropdown/Dropdown.stories.tsx`.
src/components/ui/Checkbox/fragments/CheckboxIndicator.tsx (2)
Learnt from: kotAPI
PR: rad-ui/ui#576
File: src/core/primitives/Toggle/index.tsx:15-22
Timestamp: 2024-11-24T06:43:42.194Z
Learning: In the `TogglePrimitive` component (`src/core/primitives/Toggle/index.tsx`), when the component becomes controlled, it's acceptable to not sync the internal `isPressed` state with the external `pressed` prop.
Learnt from: decipher-cs
PR: rad-ui/ui#417
File: src/components/ui/Dropdown/Dropdown.stories.tsx:43-50
Timestamp: 2024-12-12T08:34:33.079Z
Learning: Ensure to verify existing ARIA attributes in components before suggesting additions during code reviews, especially in the `Dropdown.Trigger` component in `src/components/ui/Dropdown/Dropdown.stories.tsx`.
src/components/ui/Checkbox/Checkbox.tsx (3)
Learnt from: decipher-cs
PR: rad-ui/ui#417
File: src/components/ui/Dropdown/Dropdown.stories.tsx:43-50
Timestamp: 2024-12-12T08:34:33.079Z
Learning: Ensure to verify existing ARIA attributes in components before suggesting additions during code reviews, especially in the `Dropdown.Trigger` component in `src/components/ui/Dropdown/Dropdown.stories.tsx`.
Learnt from: kotAPI
PR: rad-ui/ui#576
File: src/core/primitives/Toggle/index.tsx:15-22
Timestamp: 2024-11-24T06:43:42.194Z
Learning: In the `TogglePrimitive` component (`src/core/primitives/Toggle/index.tsx`), when the component becomes controlled, it's acceptable to not sync the internal `isPressed` state with the external `pressed` prop.
Learnt from: decipher-cs
PR: rad-ui/ui#417
File: src/components/ui/Dropdown/Dropdown.tsx:0-0
Timestamp: 2024-12-12T08:22:59.375Z
Learning: The `Dropdown.Trigger` component is customizable and needs to be used with `Dropdown.Root`.
src/core/primitives/Select/fragments/SelectPrimitiveSearch.tsx (2)
Learnt from: kotAPI
PR: rad-ui/ui#576
File: src/core/primitives/Toggle/index.tsx:15-22
Timestamp: 2024-11-24T06:43:42.194Z
Learning: In the `TogglePrimitive` component (`src/core/primitives/Toggle/index.tsx`), when the component becomes controlled, it's acceptable to not sync the internal `isPressed` state with the external `pressed` prop.
Learnt from: decipher-cs
PR: rad-ui/ui#417
File: src/components/ui/Dropdown/Dropdown.stories.tsx:43-50
Timestamp: 2024-12-12T08:34:33.079Z
Learning: Ensure to verify existing ARIA attributes in components before suggesting additions during code reviews, especially in the `Dropdown.Trigger` component in `src/components/ui/Dropdown/Dropdown.stories.tsx`.
src/core/primitives/Checkbox/fragments/CheckboxPrimitiveTrigger.tsx (3)
Learnt from: kotAPI
PR: rad-ui/ui#576
File: src/core/primitives/Toggle/index.tsx:15-22
Timestamp: 2024-11-24T06:43:42.194Z
Learning: In the `TogglePrimitive` component (`src/core/primitives/Toggle/index.tsx`), when the component becomes controlled, it's acceptable to not sync the internal `isPressed` state with the external `pressed` prop.
Learnt from: decipher-cs
PR: rad-ui/ui#417
File: src/components/ui/Dropdown/Dropdown.stories.tsx:43-50
Timestamp: 2024-12-12T08:34:33.079Z
Learning: Ensure to verify existing ARIA attributes in components before suggesting additions during code reviews, especially in the `Dropdown.Trigger` component in `src/components/ui/Dropdown/Dropdown.stories.tsx`.
Learnt from: decipher-cs
PR: rad-ui/ui#417
File: src/components/ui/Dropdown/Dropdown.tsx:0-0
Timestamp: 2024-12-12T08:22:59.375Z
Learning: The `Dropdown.Trigger` component is customizable and needs to be used with `Dropdown.Root`.
src/core/primitives/Select/contexts/SelectPrimitiveContext.tsx (1)
Learnt from: kotAPI
PR: rad-ui/ui#576
File: src/core/primitives/Toggle/index.tsx:15-22
Timestamp: 2024-11-24T06:43:42.194Z
Learning: In the `TogglePrimitive` component (`src/core/primitives/Toggle/index.tsx`), when the component becomes controlled, it's acceptable to not sync the internal `isPressed` state with the external `pressed` prop.
src/core/primitives/Select/fragments/SelectPrimitiveRoot.tsx (3)
Learnt from: kotAPI
PR: rad-ui/ui#576
File: src/core/primitives/Toggle/index.tsx:15-22
Timestamp: 2024-11-24T06:43:42.194Z
Learning: In the `TogglePrimitive` component (`src/core/primitives/Toggle/index.tsx`), when the component becomes controlled, it's acceptable to not sync the internal `isPressed` state with the external `pressed` prop.
Learnt from: kotAPI
PR: rad-ui/ui#1031
File: src/components/ui/Accordion/fragments/AccordionRoot.tsx:41-44
Timestamp: 2025-04-07T04:38:34.864Z
Learning: The Accordion component in rad-ui/ui supports both controlled and uncontrolled modes through props like `value`, `defaultValue`, and `onValueChange`. When implementing controlled components, remember to: 1) Initialize state from defaultValue, 2) Update internal state when value changes (controlled mode), 3) Call onValueChange callback, and 4) Prevent internal state updates when in controlled mode.
Learnt from: decipher-cs
PR: rad-ui/ui#417
File: src/components/ui/Dropdown/Dropdown.tsx:0-0
Timestamp: 2024-12-12T08:22:59.375Z
Learning: The `Dropdown.Trigger` component is customizable and needs to be used with `Dropdown.Root`.
styles/themes/components/select.scss (1)
Learnt from: decipher-cs
PR: rad-ui/ui#417
File: src/components/ui/Dropdown/Dropdown.stories.tsx:43-50
Timestamp: 2024-12-12T08:34:33.079Z
Learning: Ensure to verify existing ARIA attributes in components before suggesting additions during code reviews, especially in the `Dropdown.Trigger` component in `src/components/ui/Dropdown/Dropdown.stories.tsx`.
src/components/ui/Slider/fragments/SliderThumb.tsx (1)
Learnt from: decipher-cs
PR: rad-ui/ui#417
File: src/components/ui/Dropdown/Dropdown.stories.tsx:43-50
Timestamp: 2024-12-12T08:34:33.079Z
Learning: Ensure to verify existing ARIA attributes in components before suggesting additions during code reviews, especially in the `Dropdown.Trigger` component in `src/components/ui/Dropdown/Dropdown.stories.tsx`.
src/core/primitives/Checkbox/fragments/CheckboxPrimitiveRoot.tsx (3)
Learnt from: kotAPI
PR: rad-ui/ui#576
File: src/core/primitives/Toggle/index.tsx:15-22
Timestamp: 2024-11-24T06:43:42.194Z
Learning: In the `TogglePrimitive` component (`src/core/primitives/Toggle/index.tsx`), when the component becomes controlled, it's acceptable to not sync the internal `isPressed` state with the external `pressed` prop.
Learnt from: decipher-cs
PR: rad-ui/ui#417
File: src/components/ui/Dropdown/Dropdown.tsx:0-0
Timestamp: 2024-12-12T08:22:59.375Z
Learning: The `Dropdown.Trigger` component is customizable and needs to be used with `Dropdown.Root`.
Learnt from: decipher-cs
PR: rad-ui/ui#417
File: src/components/ui/Dropdown/Dropdown.stories.tsx:43-50
Timestamp: 2024-12-12T08:34:33.079Z
Learning: Ensure to verify existing ARIA attributes in components before suggesting additions during code reviews, especially in the `Dropdown.Trigger` component in `src/components/ui/Dropdown/Dropdown.stories.tsx`.
src/components/ui/Checkbox/stories/Checkbox.stories.tsx (2)
Learnt from: decipher-cs
PR: rad-ui/ui#417
File: src/components/ui/Dropdown/Dropdown.stories.tsx:43-50
Timestamp: 2024-12-12T08:34:33.079Z
Learning: Ensure to verify existing ARIA attributes in components before suggesting additions during code reviews, especially in the `Dropdown.Trigger` component in `src/components/ui/Dropdown/Dropdown.stories.tsx`.
Learnt from: kotAPI
PR: rad-ui/ui#484
File: src/components/ui/Kbd/stories/Kbd.stories.js:6-6
Timestamp: 2024-10-27T07:20:10.457Z
Learning: In the Storybook files, variations in the title prefixes like 'Components/', 'WIP/', 'Tools/', and 'Examples/' are intentional. These naming patterns are acceptable and should not be flagged as inconsistencies in future code reviews.
src/core/primitives/Select/fragments/SelectPrimitiveItem.tsx (3)
Learnt from: kotAPI
PR: rad-ui/ui#576
File: src/core/primitives/Toggle/index.tsx:15-22
Timestamp: 2024-11-24T06:43:42.194Z
Learning: In the `TogglePrimitive` component (`src/core/primitives/Toggle/index.tsx`), when the component becomes controlled, it's acceptable to not sync the internal `isPressed` state with the external `pressed` prop.
Learnt from: decipher-cs
PR: rad-ui/ui#417
File: src/components/ui/Dropdown/Dropdown.stories.tsx:43-50
Timestamp: 2024-12-12T08:34:33.079Z
Learning: Ensure to verify existing ARIA attributes in components before suggesting additions during code reviews, especially in the `Dropdown.Trigger` component in `src/components/ui/Dropdown/Dropdown.stories.tsx`.
Learnt from: kotAPI
PR: rad-ui/ui#1031
File: src/components/ui/Accordion/fragments/AccordionRoot.tsx:41-44
Timestamp: 2025-04-07T04:38:34.864Z
Learning: The Accordion component in rad-ui/ui supports both controlled and uncontrolled modes through props like `value`, `defaultValue`, and `onValueChange`. When implementing controlled components, remember to: 1) Initialize state from defaultValue, 2) Update internal state when value changes (controlled mode), 3) Call onValueChange callback, and 4) Prevent internal state updates when in controlled mode.
src/core/primitives/Select/stories/Select.stories.tsx (1)
Learnt from: decipher-cs
PR: rad-ui/ui#417
File: src/components/ui/Dropdown/Dropdown.stories.tsx:43-50
Timestamp: 2024-12-12T08:34:33.079Z
Learning: Ensure to verify existing ARIA attributes in components before suggesting additions during code reviews, especially in the `Dropdown.Trigger` component in `src/components/ui/Dropdown/Dropdown.stories.tsx`.
docs/components/layout/Documentation/helpers/CodeBlock.js (2)
Learnt from: GoldGroove06
PR: rad-ui/ui#889
File: docs/app/docsv2/components/aspect-ratio/docs/codeUsage.js:16-17
Timestamp: 2025-02-22T18:53:12.639Z
Learning: The code object in docs/app/docsv2/components/aspect-ratio/docs/codeUsage.js requires two closing braces - one for the inner `javascript` object and another for the outer `code` object.
Learnt from: decipher-cs
PR: rad-ui/ui#417
File: src/components/ui/Dropdown/Dropdown.stories.tsx:43-50
Timestamp: 2024-12-12T08:34:33.079Z
Learning: Ensure to verify existing ARIA attributes in components before suggesting additions during code reviews, especially in the `Dropdown.Trigger` component in `src/components/ui/Dropdown/Dropdown.stories.tsx`.
src/core/primitives/Select/fragments/SelectPrimitiveContent.tsx (2)
Learnt from: kotAPI
PR: rad-ui/ui#576
File: src/core/primitives/Toggle/index.tsx:15-22
Timestamp: 2024-11-24T06:43:42.194Z
Learning: In the `TogglePrimitive` component (`src/core/primitives/Toggle/index.tsx`), when the component becomes controlled, it's acceptable to not sync the internal `isPressed` state with the external `pressed` prop.
Learnt from: decipher-cs
PR: rad-ui/ui#417
File: src/components/ui/Dropdown/Dropdown.stories.tsx:43-50
Timestamp: 2024-12-12T08:34:33.079Z
Learning: Ensure to verify existing ARIA attributes in components before suggesting additions during code reviews, especially in the `Dropdown.Trigger` component in `src/components/ui/Dropdown/Dropdown.stories.tsx`.
src/components/ui/Slider/fragments/SliderRoot.tsx (2)
Learnt from: decipher-cs
PR: rad-ui/ui#417
File: src/components/ui/Dropdown/Dropdown.tsx:0-0
Timestamp: 2024-12-12T08:22:59.375Z
Learning: The `Dropdown.Trigger` component is customizable and needs to be used with `Dropdown.Root`.
Learnt from: kotAPI
PR: rad-ui/ui#1031
File: src/components/ui/Accordion/fragments/AccordionRoot.tsx:41-44
Timestamp: 2025-04-07T04:38:34.864Z
Learning: The Accordion component in rad-ui/ui supports both controlled and uncontrolled modes through props like `value`, `defaultValue`, and `onValueChange`. When implementing controlled components, remember to: 1) Initialize state from defaultValue, 2) Update internal state when value changes (controlled mode), 3) Call onValueChange callback, and 4) Prevent internal state updates when in controlled mode.
🧬 Code Graph Analysis (3)
src/components/ui/Select/fragments/SelectSearch.tsx (1)
src/components/ui/Select/contexts/SelectRootContext.tsx (1)
SelectRootContext(7-9)
src/components/ui/Slider/fragments/SliderThumb.tsx (1)
src/components/ui/Slider/context/SliderContext.tsx (1)
SliderContext(13-21)
src/core/primitives/Select/fragments/SelectPrimitiveItem.tsx (1)
src/core/primitives/Select/contexts/SelectPrimitiveContext.tsx (1)
SelectPrimitiveContext(26-26)
🪛 GitHub Actions: Lint
src/core/primitives/Select/fragments/SelectPrimitiveSearch.tsx
[warning] 8-8: ESLint: 'selectedItemRef' is assigned a value but never used. (no-unused-vars)
src/core/primitives/Select/fragments/SelectPrimitiveRoot.tsx
[warning] 2-2: ESLint: 'useEffect' is defined but never used. (no-unused-vars)
src/components/ui/Slider/fragments/SliderThumb.tsx
[warning] 3-19: ESLint: 'useRef' is defined but never used; jsx-a11y/click-events-have-key-events and jsx-a11y/no-static-element-interactions warnings at line 19.
src/core/primitives/Select/fragments/SelectPrimitiveContent.tsx
[warning] 2-13: ESLint: 'useEffect', 'Primitive', 'setIsOpen', and 'selectedValue' are defined or assigned but never used. (no-unused-vars)
src/components/ui/Slider/fragments/SliderRoot.tsx
[warning] 92-92: ESLint jsx-a11y/click-events-have-key-events and jsx-a11y/no-static-element-interactions: Visible non-interactive elements with click handlers must have keyboard listeners and appropriate roles.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (21)
docs/components/layout/Documentation/helpers/CodeBlock.js (1)
11-12: LGTM: Clean imports for the ScrollArea refactor.The addition of
clsxandScrollAreaimports supports the component modernization effectively.src/components/ui/Slider/fragments/SliderRoot.tsx (1)
1-97: Inconsistency between PR objectives and actual changes.The PR objectives mention "Progress bar Sizes" but the files being reviewed are for a Slider component implementation. Please verify if these files are correctly associated with PR #1153.
#!/bin/bash # Verify if there are any Progress component changes in this PR echo "Searching for Progress component files..." fd -t f -e tsx -e ts | grep -i progress | head -10 echo "Searching for any progress-related changes..." rg -i "progress" --type ts --type tsx -A 3 -B 3 | head -20Likely an incorrect or invalid review comment.
src/components/ui/Select/stories/Select.stories.tsx (1)
176-176: Clean integration of search functionality.The addition of
<Select.Search/>appropriately demonstrates the new search feature within the story example.src/components/ui/Select/fragments/SelectSearch.tsx (1)
5-14: Well-structured component implementation.The
SelectSearchcomponent correctly uses theSelectRootContextand follows the established pattern for fragment components in this codebase.src/core/primitives/Select/contexts/SelectPrimitiveContext.tsx (1)
22-23: Context extension looks correct.The addition of
activeItemValueandsetActiveItemValueproperties to the context type is properly typed and follows the established pattern.src/core/primitives/Select/fragments/SelectPrimitiveRoot.tsx (3)
27-27: Good addition of active item state.The
activeItemValuestate is properly initialized and will be used by the search functionality.
64-73: Appropriate use of useLayoutEffect for DOM measurements.The switch from
useEffecttouseLayoutEffectis correct for synchronous DOM measurements and positioning calculations.
75-75: Context values properly extended.The context values correctly include the new
activeItemValueandsetActiveItemValuefor the search functionality.src/core/primitives/Select/stories/Select.stories.tsx (1)
145-175: PR objectives don't match the actual changes.The PR objectives mention "Progress bar Sizes" but all the code changes are about adding search functionality to the Select component. Please verify that this is the correct PR and update the objectives accordingly.
Likely an incorrect or invalid review comment.
styles/themes/components/select.scss (2)
85-108: LGTM! Well-structured search input styling.The search input styles are well-implemented with:
- Consistent use of CSS custom properties for theming
- Proper accessibility support with focus states
- Good user experience with smooth transitions
- Appropriate box-sizing and responsive behavior
163-167: LGTM! Consistent active item highlighting.The active item styling appropriately uses the existing accent color scheme and maintains consistency with other item states.
src/core/primitives/Select/fragments/SelectPrimitiveItem.tsx (3)
15-16: LGTM! Proper context consumption and state management.The addition of
activeItemValueto the context consumption and the localisFocusedstate properly supports the new active item tracking functionality.
18-27: LGTM! Clean focus management implementation.The focus/blur handlers properly maintain the local focused state which is used for the data-active attribute.
28-37: LGTM! Proper component transition and attribute logic.The transition from
ButtonPrimitivetoPrimitive.divis clean, and thedata-activeattribute logic correctly combines both the context's active item value and the local focus state.src/core/primitives/Checkbox/fragments/CheckboxPrimitiveTrigger.tsx (1)
1-13: Inconsistency: PR objectives mention Progress bar but code implements Checkbox components.The PR objectives describe "Progress bar Sizes" but all the provided files implement Checkbox components. Please verify that the correct files are being reviewed.
Likely an incorrect or invalid review comment.
src/core/primitives/Checkbox/fragments/CheckboxPrimitiveIndicator.tsx (1)
6-12: Implementation looks correct and follows conditional rendering pattern.The component correctly uses context to conditionally render the indicator based on the checked state, which is the expected behavior for a checkbox indicator.
src/components/ui/Checkbox/fragments/CheckboxIndicator.tsx (1)
4-8: Component structure follows good composition pattern.The component correctly wraps the primitive indicator and passes props appropriately, maintaining a clean separation between primitive and UI layers.
src/components/ui/Checkbox/Checkbox.tsx (1)
4-12: Excellent compound component pattern implementation.The component correctly implements the compound component pattern with clear developer guidance. The warning message helps prevent misuse, and the static property assignment enables clean API usage like
Checkbox.RootandCheckbox.Indicator.src/components/ui/Checkbox/stories/Checkbox.stories.tsx (3)
1-28: Inconsistency between PR objectives and file content.The PR objectives mention "Progress bar Sizes" and adding different sizes for the progress bar component, but this file is implementing a Checkbox component story. This appears to be unrelated to the stated PR objectives.
Please clarify if this file was included in the PR by mistake or if the PR description needs to be updated to reflect multiple component additions.
Likely an incorrect or invalid review comment.
1-9: LGTM!The imports and Storybook configuration follow standard patterns. The "Components/" title prefix is appropriate as confirmed by the project's established conventions.
25-27: LGTM!The Default story export follows proper Storybook patterns and correctly wraps the component in SandboxEditor for interactive functionality.
| } | ||
| return ( | ||
| <pre className="relative mb-8"> | ||
| <div className="relative "> | ||
| <code | ||
| <div className="relative height maxHeight"> |
There was a problem hiding this comment.
Fix invalid CSS class names.
The class string "relative height maxHeight" contains invalid CSS class names. The height and maxHeight appear to be variable names rather than actual CSS classes.
Apply this diff to fix the CSS classes:
-<div className="relative height maxHeight">
+<div className="relative">🤖 Prompt for AI Agents
In docs/components/layout/Documentation/helpers/CodeBlock.js at line 64, the
className string "relative height maxHeight" incorrectly includes "height" and
"maxHeight" which are not valid CSS class names but likely variable names.
Replace these with appropriate CSS class names or remove them if they are not
CSS classes, ensuring only valid CSS class names are included in the className
attribute.
| <ScrollArea.Root className = { clsx("transition-all", expanded ? "max-h-[640px]": "max-h-[180px]")}> | ||
| <ScrollArea.Viewport style={{height: inline ? 'auto' : height, maxHeight: maxHeight, overflowY: 'auto'}}> | ||
| <code className={`language-${language} whitespace-pre-wrap block`}> | ||
| {code} | ||
| </code> | ||
| </ScrollArea.Viewport> | ||
| <ScrollArea.Scrollbar> | ||
| <ScrollArea.Thumb /> | ||
| </ScrollArea.Scrollbar> | ||
| </ScrollArea.Root> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve the ScrollArea implementation consistency.
The ScrollArea implementation has some inconsistencies with the existing height/maxHeight logic and styling approach:
- The
heightandmaxHeightvariables are calculated but not used in the ScrollArea classes - Mixing inline styles with conditional classes creates inconsistent styling patterns
- The ScrollArea.Viewport inline styles may not align with the component's design system
Apply this diff to improve consistency:
-<ScrollArea.Root className = { clsx("transition-all", expanded ? "max-h-[640px]": "max-h-[180px]")}>
- <ScrollArea.Viewport style={{height: inline ? 'auto' : height, maxHeight: maxHeight, overflowY: 'auto'}}>
- <code className={`language-${language} whitespace-pre-wrap block`}>
- {code}
- </code>
- </ScrollArea.Viewport>
- <ScrollArea.Scrollbar>
- <ScrollArea.Thumb />
- </ScrollArea.Scrollbar>
-</ScrollArea.Root>
+<ScrollArea.Root className={clsx(
+ "transition-all",
+ !inline && (expanded ? "max-h-[640px]" : "max-h-[180px]"),
+ inline && "h-auto"
+)}>
+ <ScrollArea.Viewport className="overflow-auto">
+ <code className={`language-${language} whitespace-pre-wrap block`}>
+ {code}
+ </code>
+ </ScrollArea.Viewport>
+ <ScrollArea.Scrollbar>
+ <ScrollArea.Thumb />
+ </ScrollArea.Scrollbar>
+</ScrollArea.Root>📝 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.
| <ScrollArea.Root className = { clsx("transition-all", expanded ? "max-h-[640px]": "max-h-[180px]")}> | |
| <ScrollArea.Viewport style={{height: inline ? 'auto' : height, maxHeight: maxHeight, overflowY: 'auto'}}> | |
| <code className={`language-${language} whitespace-pre-wrap block`}> | |
| {code} | |
| </code> | |
| </ScrollArea.Viewport> | |
| <ScrollArea.Scrollbar> | |
| <ScrollArea.Thumb /> | |
| </ScrollArea.Scrollbar> | |
| </ScrollArea.Root> | |
| <ScrollArea.Root className={clsx( | |
| "transition-all", | |
| !inline && (expanded ? "max-h-[640px]" : "max-h-[180px]"), | |
| inline && "h-auto" | |
| )}> | |
| <ScrollArea.Viewport className="overflow-auto"> | |
| <code className={`language-${language} whitespace-pre-wrap block`}> | |
| {code} | |
| </code> | |
| </ScrollArea.Viewport> | |
| <ScrollArea.Scrollbar> | |
| <ScrollArea.Thumb /> | |
| </ScrollArea.Scrollbar> | |
| </ScrollArea.Root> |
🤖 Prompt for AI Agents
In docs/components/layout/Documentation/helpers/CodeBlock.js around lines 75 to
84, the ScrollArea component inconsistently applies height and maxHeight by
mixing inline styles and conditional classes, and does not use the calculated
height and maxHeight variables in the class names. To fix this, remove the
inline styles on ScrollArea.Viewport and instead apply height and maxHeight
through conditional class names or styles consistent with the design system,
ensuring the calculated height and maxHeight variables are properly used in the
ScrollArea classes for consistent styling.
| const handleChange = (e: React.ChangeEvent<HTMLInputElement>) => { | ||
| console.log(e.target.value); | ||
| }; |
There was a problem hiding this comment.
Remove console.log from production code.
The console.log statement should be removed from production code. Consider using the setValue function from context to properly update the slider value.
const handleChange = (e: React.ChangeEvent<HTMLInputElement>) => {
- console.log(e.target.value);
+ // TODO: Implement proper value update logic using setValue from context
};📝 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.
| const handleChange = (e: React.ChangeEvent<HTMLInputElement>) => { | |
| console.log(e.target.value); | |
| }; | |
| const handleChange = (e: React.ChangeEvent<HTMLInputElement>) => { | |
| - console.log(e.target.value); | |
| + // TODO: Implement proper value update logic using setValue from context | |
| }; |
🤖 Prompt for AI Agents
In src/components/ui/Slider/fragments/SliderThumb.tsx around lines 15 to 17,
remove the console.log statement from the handleChange function to avoid logging
in production. Instead, use the setValue function from the context to update the
slider value properly when the input changes.
| return <div className={`${rootClass}-thumb`} onClick={handleClick} style={{ left: `calc(${value}% - 16px)` }}> | ||
| <span className={`${rootClass}-thumb-value`}></span> | ||
| <input onChange={handleChange} value={value} style={{ display: 'none' }} ref={sliderInputRef} /> | ||
| </div>; |
There was a problem hiding this comment.
Add keyboard event handlers for accessibility.
The component has accessibility issues due to missing keyboard event handlers. Interactive elements should support keyboard navigation.
- return <div className={`${rootClass}-thumb`} onClick={handleClick} style={{ left: `calc(${value}% - 16px)` }}>
+ return <div
+ className={`${rootClass}-thumb`}
+ onClick={handleClick}
+ onKeyDown={(e) => {
+ if (e.key === 'Enter' || e.key === ' ') {
+ e.preventDefault();
+ handleClick(e as any);
+ }
+ }}
+ role="slider"
+ tabIndex={0}
+ aria-valuemin={0}
+ aria-valuemax={100}
+ aria-valuenow={value}
+ style={{ left: `calc(${value}% - 16px)` }}
+ >
<span className={`${rootClass}-thumb-value`}></span>
<input onChange={handleChange} value={value} style={{ display: 'none' }} ref={sliderInputRef} />
</div>;📝 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.
| return <div className={`${rootClass}-thumb`} onClick={handleClick} style={{ left: `calc(${value}% - 16px)` }}> | |
| <span className={`${rootClass}-thumb-value`}></span> | |
| <input onChange={handleChange} value={value} style={{ display: 'none' }} ref={sliderInputRef} /> | |
| </div>; | |
| return <div | |
| className={`${rootClass}-thumb`} | |
| onClick={handleClick} | |
| onKeyDown={(e) => { | |
| if (e.key === 'Enter' || e.key === ' ') { | |
| e.preventDefault(); | |
| handleClick(e as any); | |
| } | |
| }} | |
| role="slider" | |
| tabIndex={0} | |
| aria-valuemin={0} | |
| aria-valuemax={100} | |
| aria-valuenow={value} | |
| style={{ left: `calc(${value}% - 16px)` }} | |
| > | |
| <span className={`${rootClass}-thumb-value`}></span> | |
| <input | |
| onChange={handleChange} | |
| value={value} | |
| style={{ display: 'none' }} | |
| ref={sliderInputRef} | |
| /> | |
| </div>; |
🤖 Prompt for AI Agents
In src/components/ui/Slider/fragments/SliderThumb.tsx around lines 19 to 22, the
div acting as an interactive element lacks keyboard event handlers, causing
accessibility issues. Add appropriate keyboard event handlers such as onKeyDown
to handle key presses like Enter and Space, enabling keyboard navigation and
interaction. Ensure the handlers trigger the same behavior as the onClick event
to maintain consistent functionality for keyboard users.
|
|
||
| setSliderValue(e); | ||
|
|
||
| console.log('moving'); |
There was a problem hiding this comment.
Remove console.log from production code.
Debug console.log statements should be removed from production code.
- console.log('moving');📝 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.
| console.log('moving'); |
🤖 Prompt for AI Agents
In src/components/ui/Slider/fragments/SliderRoot.tsx at line 88, remove the
console.log('moving') statement to eliminate debug output from production code.
| import React from 'react'; | ||
| import CheckboxPrimitiveIndicator from '~/core/primitives/Checkbox/fragments/CheckboxPrimitiveIndicator'; | ||
|
|
||
| const CheckboxIndicator = ({ children, className = '', ...props }: CheckboxIndicatorProps) => ( |
There was a problem hiding this comment.
Define missing TypeScript interface.
The component references CheckboxIndicatorProps but this type is not defined or imported, which will cause a TypeScript compilation error.
+interface CheckboxIndicatorProps {
+ children: React.ReactNode;
+ className?: string;
+}
+
const CheckboxIndicator = ({ children, className = '', ...props }: CheckboxIndicatorProps) => (🤖 Prompt for AI Agents
In src/components/ui/Checkbox/fragments/CheckboxIndicator.tsx at line 4, the
type CheckboxIndicatorProps is used but not defined or imported, causing a
TypeScript error. Define the CheckboxIndicatorProps interface to specify the
expected props for the CheckboxIndicator component, including children and
className, or import it if it exists elsewhere. This will resolve the missing
type reference and ensure proper type checking.
| <CheckboxPrimitiveTrigger className={className}> | ||
| {children} | ||
| </CheckboxPrimitiveTrigger> | ||
| <input type="checkbox" style={{ display: 'none' }} /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Connect hidden input to component state and add proper attributes.
The hidden input should be synchronized with the component's checked state and include proper form attributes.
- <input type="checkbox" style={{ display: 'none' }} />
+ <input
+ type="checkbox"
+ checked={checked}
+ onChange={() => {}} // Controlled by trigger
+ style={{ display: 'none' }}
+ tabIndex={-1}
+ aria-hidden="true"
+ />Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/core/primitives/Checkbox/fragments/CheckboxPrimitiveRoot.tsx at line 19,
the hidden input element is not connected to the component's checked state and
lacks necessary form attributes. Update the input to have its checked attribute
bound to the component's state, add an onChange handler to update the state, and
include relevant attributes like name and id to ensure proper form behavior and
accessibility.
| const contextValues = { | ||
| checked, | ||
| setChecked | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize context value creation to prevent unnecessary re-renders.
The context values object is recreated on every render, which can cause performance issues in child components.
- const contextValues = {
- checked,
- setChecked
- };
+ const handleCheckedChange = React.useCallback((newChecked: boolean) => {
+ if (controlledChecked === undefined) {
+ setInternalChecked(newChecked);
+ }
+ onCheckedChange?.(newChecked);
+ }, [controlledChecked, onCheckedChange]);
+
+ const contextValues = React.useMemo(() => ({
+ checked,
+ setChecked: handleCheckedChange
+ }), [checked, handleCheckedChange]);📝 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.
| const contextValues = { | |
| checked, | |
| setChecked | |
| }; | |
| const handleCheckedChange = React.useCallback((newChecked: boolean) => { | |
| if (controlledChecked === undefined) { | |
| setInternalChecked(newChecked); | |
| } | |
| onCheckedChange?.(newChecked); | |
| }, [controlledChecked, onCheckedChange]); | |
| const contextValues = React.useMemo(() => ({ | |
| checked, | |
| setChecked: handleCheckedChange | |
| }), [checked, handleCheckedChange]); |
🤖 Prompt for AI Agents
In src/core/primitives/Checkbox/fragments/CheckboxPrimitiveRoot.tsx around lines
10 to 13, the contextValues object is recreated on every render causing
unnecessary re-renders in child components. To fix this, memoize the
contextValues object using React's useMemo hook, specifying checked and
setChecked as dependencies, so the object is only recreated when these values
change.
| import CheckboxPrimitiveContext from '../context/CheckboxPrimitiveContext'; | ||
| import CheckboxPrimitiveTrigger from './CheckboxPrimitiveTrigger'; | ||
|
|
||
| const CheckboxPrimitiveRoot = ({ children, className = '' }: { children: React.ReactNode, className?: string }) => { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add support for controlled component pattern.
The component should support both controlled and uncontrolled usage patterns with checked, defaultChecked, and onCheckedChange props.
+interface CheckboxPrimitiveRootProps {
+ children: React.ReactNode;
+ className?: string;
+ checked?: boolean;
+ defaultChecked?: boolean;
+ onCheckedChange?: (checked: boolean) => void;
+}
+
-const CheckboxPrimitiveRoot = ({ children, className = '' }: { children: React.ReactNode, className?: string }) => {
+const CheckboxPrimitiveRoot = ({
+ children,
+ className = '',
+ checked: controlledChecked,
+ defaultChecked = false,
+ onCheckedChange
+}: CheckboxPrimitiveRootProps) => {Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/core/primitives/Checkbox/fragments/CheckboxPrimitiveRoot.tsx at line 7,
the CheckboxPrimitiveRoot component currently only accepts children and
className props and does not support controlled usage. Modify the component to
accept checked, defaultChecked, and onCheckedChange props to enable both
controlled and uncontrolled usage patterns. Implement internal state management
for uncontrolled mode and use the checked prop for controlled mode, calling
onCheckedChange when the checked state changes.
| import CheckboxPrimitiveTrigger from './CheckboxPrimitiveTrigger'; | ||
|
|
||
| const CheckboxPrimitiveRoot = ({ children, className = '' }: { children: React.ReactNode, className?: string }) => { | ||
| const [checked, setChecked] = React.useState(false); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Initialize state with defaultChecked value.
The state should be initialized with the defaultChecked value when provided.
- const [checked, setChecked] = React.useState(false);
+ const [internalChecked, setInternalChecked] = React.useState(defaultChecked);
+ const checked = controlledChecked !== undefined ? controlledChecked : internalChecked;📝 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.
| const [checked, setChecked] = React.useState(false); | |
| const [internalChecked, setInternalChecked] = React.useState(defaultChecked); | |
| const checked = controlledChecked !== undefined ? controlledChecked : internalChecked; |
🤖 Prompt for AI Agents
In src/core/primitives/Checkbox/fragments/CheckboxPrimitiveRoot.tsx at line 8,
the state variable 'checked' is initialized with a fixed value 'false'. Update
the initialization to use the 'defaultChecked' prop value if it is provided, so
the initial state reflects the default checked status passed to the component.
lint does not recognize @each in scss
Added different sizes for the progress bar component in master list for issue #937
Summary by CodeRabbit
New Features
Tests
Style
Documentation
Chores