feat(toggle): new TEDI-Ready component #305#594
Merged
Conversation
21 tasks
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 53 minutes and 28 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a new TEDI-Ready Toggle component (implementation, SCSS module, tests, and Storybook stories), re-exports it from the TEDI index, and marks the existing community Toggle story as deprecated. No public API breaking changes. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ToggleComponent
participant NativeInput
participant ExternalHandler
User->>ToggleComponent: Click / keyboard toggle
ToggleComponent->>NativeInput: update checked (prevent if disabled/isLoading)
NativeInput-->>ToggleComponent: change event (checked state)
ToggleComponent->>ToggleComponent: update internal state if uncontrolled
ToggleComponent->>ExternalHandler: call onChange(checked) (unless isLoading)
ExternalHandler-->>ToggleComponent: (optional) controlled checked prop update
ToggleComponent->>ToggleComponent: render spinner/icon/slider based on state
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
src/tedi/components/form/toggle/toggle.stories.tsx (2)
287-295: Missingidprop in WithFeedbackText story.Same as above—adding an explicit
idimproves story clarity.✨ Suggested improvement
export const WithFeedbackText: Story = { args: { + id: 'toggle-feedback', label: 'Label', helper: { text: 'Something went wrong', type: 'error', }, }, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tedi/components/form/toggle/toggle.stories.tsx` around lines 287 - 295, The WithFeedbackText Story definition is missing an explicit id arg; update the Story object named WithFeedbackText in toggle.stories.tsx to include an id property in its args (e.g., args.id = '<unique-id>') so the story renders with a stable identifier and clearer documentation; ensure the id is unique across stories and matches the expected prop type used by the Toggle component.
132-137: Missingidprop in Default story.The
Defaultstory doesn't provide anidprop. While the component generates a fallback, explicitly providing anidin stories helps with testing and demonstrates best practices to consumers.✨ Suggested improvement
export const Default: Story = { args: { + id: 'toggle-default', label: 'Toggle', hideLabel: true, }, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tedi/components/form/toggle/toggle.stories.tsx` around lines 132 - 137, The Default story for the Toggle storybook entry is missing an explicit id prop; update the Default Story object (export const Default: Story) to include a stable id string (e.g., id: 'toggle-default') in its args so tests and consumers have a deterministic identifier rather than relying on the component's fallback id generation.src/tedi/components/form/toggle/toggle.spec.tsx (1)
30-36: Test assertion may not verifyhideLabelbehavior correctly.The test checks that the label element has class
tedi-toggle__label, but this class is always applied regardless ofhideLabel. Consider asserting that a visually-hidden class or style is applied whenhideLabelis true.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tedi/components/form/toggle/toggle.spec.tsx` around lines 30 - 36, The test "renders hidden label when hideLabel is true" incorrectly asserts the presence of the always-present class tedi-toggle__label; update the assertion to verify the actual hidden behavior of the Toggle when hideLabel is passed: render <Toggle {...defaultProps} hideLabel /> and assert that the label element (found via screen.getByText(/enable notifications/i)) has the visually-hidden class or attribute your component uses (e.g., 'tedi-toggle__label--visually-hidden' or 'sr-only'), or alternatively assert an appropriate inline style/aria attribute (e.g., style.display === 'none' or aria-hidden="true") that Toggle applies when hideLabel is true; reference the Toggle component, the hideLabel prop, and defaultProps to locate the test to change.src/tedi/components/form/toggle/toggle.module.scss (1)
109-112: Consider: Focus outline always uses primary color.The focus outline uses
--form-toggl-primary-active-defaultregardless of thecolorprop (primary vs colored). This may be intentional for accessibility consistency, but if the colored variant should have a matching focus ring, consider using a variable that respects the color variant.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/tedi/components/form/toggle/toggle.module.scss` around lines 109 - 112, The focus ring in toggle.module.scss is hard-coded to --form-toggl-primary-active-default in the &:focus-visible rule, so it ignores the component's color variant; update the rule to use a variant-aware CSS variable (e.g. --form-toggle-focus-color) or switch to the colored variable when the colored variant class is present (e.g. .colored { --form-toggle-focus-color: var(--form-toggl-colored-active-default); }) so the focus outline matches the color prop while keeping accessibility styles consistent; adjust the Toggle component or variant classes to set that custom property for primary vs colored variants.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/tedi/components/form/toggle/toggle.module.scss`:
- Around line 160-166: The two SCSS modifier classes &--label-left and
&--label-right currently both set flex-direction: row so they produce identical
layouts; update the toggle.module.scss so that one modifier (e.g.,
&--label-right) uses flex-direction: row-reverse (or vice versa depending on
intended default) to actually flip the label position while leaving the other as
row.
In `@src/tedi/components/form/toggle/toggle.spec.tsx`:
- Around line 141-146: The test "applies active class when checked" renders the
controlled Toggle with a checked prop but omits an onChange handler, which can
cause React console warnings; update the test that calls render(<Toggle
{...defaultProps} checked />) to pass a no-op/mock onChange (e.g.,
onChange={jest.fn()}) so the controlled Toggle receives both checked and
onChange—locate the test block with that description and modify the render call
accordingly (refer to Toggle, defaultProps, and the test name).
In `@src/tedi/components/form/toggle/toggle.stories.tsx`:
- Around line 69-124: TemplateStates renders multiple Toggle components sharing
the same id={value}, causing duplicate DOM IDs; update each Toggle to receive a
unique id (e.g., derive from value plus a suffix or index) so IDs are unique per
instance. Locate the Toggle usages in TemplateStates (the blocks passing
{...toggleProps}, label, size, defaultChecked, disabled/isLoading and
id={value}) and replace id={value} with a generated unique id string (for
example `${value}-small-1`, `${value}-large-2`, or `${value}-${index}`) or use a
helper that composes value with a position/variant identifier to guarantee
uniqueness across the row. Ensure the same unique id is used where needed for
label association.
In `@src/tedi/components/form/toggle/toggle.tsx`:
- Line 15: The Toggle component declares id as required but generates a fallback
via useId(), so update the prop type and prop handling to make id optional:
change the prop/interface from id: string to id?: string and adjust any
references (propId, id, and the Toggle function/component signature where propId
is destructured) to reflect optionality; ensure the component still computes the
final id using useId() when propId is undefined (referencing propId and useId()
in toggle.tsx).
- Around line 156-165: The outer <label htmlFor={id} className={controlClass}>
is wrapping two FormLabel components (used when labelPosition === 'left' and the
other instance for the opposite position), which causes nested <label> elements;
update both FormLabel usages (the ones with props id,
className={styles['tedi-toggle__label']}, hideLabel, label, tooltip) to pass
renderWithoutLabel={true} so they render as non-label elements (e.g., <span>)
and avoid invalid nested labels.
---
Nitpick comments:
In `@src/tedi/components/form/toggle/toggle.module.scss`:
- Around line 109-112: The focus ring in toggle.module.scss is hard-coded to
--form-toggl-primary-active-default in the &:focus-visible rule, so it ignores
the component's color variant; update the rule to use a variant-aware CSS
variable (e.g. --form-toggle-focus-color) or switch to the colored variable when
the colored variant class is present (e.g. .colored { --form-toggle-focus-color:
var(--form-toggl-colored-active-default); }) so the focus outline matches the
color prop while keeping accessibility styles consistent; adjust the Toggle
component or variant classes to set that custom property for primary vs colored
variants.
In `@src/tedi/components/form/toggle/toggle.spec.tsx`:
- Around line 30-36: The test "renders hidden label when hideLabel is true"
incorrectly asserts the presence of the always-present class tedi-toggle__label;
update the assertion to verify the actual hidden behavior of the Toggle when
hideLabel is passed: render <Toggle {...defaultProps} hideLabel /> and assert
that the label element (found via screen.getByText(/enable notifications/i)) has
the visually-hidden class or attribute your component uses (e.g.,
'tedi-toggle__label--visually-hidden' or 'sr-only'), or alternatively assert an
appropriate inline style/aria attribute (e.g., style.display === 'none' or
aria-hidden="true") that Toggle applies when hideLabel is true; reference the
Toggle component, the hideLabel prop, and defaultProps to locate the test to
change.
In `@src/tedi/components/form/toggle/toggle.stories.tsx`:
- Around line 287-295: The WithFeedbackText Story definition is missing an
explicit id arg; update the Story object named WithFeedbackText in
toggle.stories.tsx to include an id property in its args (e.g., args.id =
'<unique-id>') so the story renders with a stable identifier and clearer
documentation; ensure the id is unique across stories and matches the expected
prop type used by the Toggle component.
- Around line 132-137: The Default story for the Toggle storybook entry is
missing an explicit id prop; update the Default Story object (export const
Default: Story) to include a stable id string (e.g., id: 'toggle-default') in
its args so tests and consumers have a deterministic identifier rather than
relying on the component's fallback id generation.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: bf25e0f8-f73b-46e2-9cd5-5400db8fbf52
📒 Files selected for processing (6)
src/community/components/form/toggle/toggle.stories.tsxsrc/tedi/components/form/toggle/toggle.module.scsssrc/tedi/components/form/toggle/toggle.spec.tsxsrc/tedi/components/form/toggle/toggle.stories.tsxsrc/tedi/components/form/toggle/toggle.tsxsrc/tedi/index.ts
Comment on lines
+69
to
+124
| <Toggle | ||
| {...toggleProps} | ||
| label="Toggle" | ||
| disabled={value === 'Disabled'} | ||
| isLoading={value === 'Loading'} | ||
| id={value} | ||
| /> | ||
| <Toggle | ||
| {...toggleProps} | ||
| label="Toggle" | ||
| defaultChecked | ||
| disabled={value === 'Disabled'} | ||
| isLoading={value === 'Loading'} | ||
| id={value} | ||
| /> | ||
| </Col> | ||
| <Col width="auto" className="display-flex align-items-center gap-3"> | ||
| <Toggle | ||
| {...toggleProps} | ||
| label="Toggle" | ||
| size="large" | ||
| disabled={value === 'Disabled'} | ||
| isLoading={value === 'Loading'} | ||
| id={value} | ||
| /> | ||
| <Toggle | ||
| {...toggleProps} | ||
| label="Toggle" | ||
| defaultChecked | ||
| size="large" | ||
| disabled={value === 'Disabled'} | ||
| isLoading={value === 'Loading'} | ||
| id={value} | ||
| /> | ||
| </Col> | ||
| <Col width="auto" className="display-flex align-items-center gap-3"> | ||
| <Toggle | ||
| {...toggleProps} | ||
| label="Toggle" | ||
| size="large" | ||
| disabled={value === 'Disabled'} | ||
| isLoading={value === 'Loading'} | ||
| id={value} | ||
| icon | ||
| /> | ||
| <Toggle | ||
| {...toggleProps} | ||
| label="Toggle" | ||
| defaultChecked | ||
| size="large" | ||
| disabled={value === 'Disabled'} | ||
| isLoading={value === 'Loading'} | ||
| id={value} | ||
| icon | ||
| /> | ||
| </Col> |
There was a problem hiding this comment.
Duplicate id attributes in TemplateStates.
All toggles in the same row share id={value} (e.g., all "Hover" row toggles have id="Hover"). This creates duplicate IDs in the DOM, which is invalid HTML and can cause accessibility issues. Each toggle should have a unique ID.
🐛 Proposed fix: Use unique IDs per toggle
<Col width="auto" className="display-flex align-items-center gap-3">
<Toggle
{...toggleProps}
label="Toggle"
disabled={value === 'Disabled'}
isLoading={value === 'Loading'}
- id={value}
+ id={`${value}-default`}
/>
<Toggle
{...toggleProps}
label="Toggle"
defaultChecked
disabled={value === 'Disabled'}
isLoading={value === 'Loading'}
- id={value}
+ id={`${value}-default-checked`}
/>
</Col>
<Col width="auto" className="display-flex align-items-center gap-3">
<Toggle
{...toggleProps}
label="Toggle"
size="large"
disabled={value === 'Disabled'}
isLoading={value === 'Loading'}
- id={value}
+ id={`${value}-large`}
/>
<Toggle
{...toggleProps}
label="Toggle"
defaultChecked
size="large"
disabled={value === 'Disabled'}
isLoading={value === 'Loading'}
- id={value}
+ id={`${value}-large-checked`}
/>
</Col>
<Col width="auto" className="display-flex align-items-center gap-3">
<Toggle
{...toggleProps}
label="Toggle"
size="large"
disabled={value === 'Disabled'}
isLoading={value === 'Loading'}
- id={value}
+ id={`${value}-large-icon`}
icon
/>
<Toggle
{...toggleProps}
label="Toggle"
defaultChecked
size="large"
disabled={value === 'Disabled'}
isLoading={value === 'Loading'}
- id={value}
+ id={`${value}-large-icon-checked`}
icon
/>
</Col>📝 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.
Suggested change
| <Toggle | |
| {...toggleProps} | |
| label="Toggle" | |
| disabled={value === 'Disabled'} | |
| isLoading={value === 'Loading'} | |
| id={value} | |
| /> | |
| <Toggle | |
| {...toggleProps} | |
| label="Toggle" | |
| defaultChecked | |
| disabled={value === 'Disabled'} | |
| isLoading={value === 'Loading'} | |
| id={value} | |
| /> | |
| </Col> | |
| <Col width="auto" className="display-flex align-items-center gap-3"> | |
| <Toggle | |
| {...toggleProps} | |
| label="Toggle" | |
| size="large" | |
| disabled={value === 'Disabled'} | |
| isLoading={value === 'Loading'} | |
| id={value} | |
| /> | |
| <Toggle | |
| {...toggleProps} | |
| label="Toggle" | |
| defaultChecked | |
| size="large" | |
| disabled={value === 'Disabled'} | |
| isLoading={value === 'Loading'} | |
| id={value} | |
| /> | |
| </Col> | |
| <Col width="auto" className="display-flex align-items-center gap-3"> | |
| <Toggle | |
| {...toggleProps} | |
| label="Toggle" | |
| size="large" | |
| disabled={value === 'Disabled'} | |
| isLoading={value === 'Loading'} | |
| id={value} | |
| icon | |
| /> | |
| <Toggle | |
| {...toggleProps} | |
| label="Toggle" | |
| defaultChecked | |
| size="large" | |
| disabled={value === 'Disabled'} | |
| isLoading={value === 'Loading'} | |
| id={value} | |
| icon | |
| /> | |
| </Col> | |
| <Toggle | |
| {...toggleProps} | |
| label="Toggle" | |
| disabled={value === 'Disabled'} | |
| isLoading={value === 'Loading'} | |
| id={`${value}-default`} | |
| /> | |
| <Toggle | |
| {...toggleProps} | |
| label="Toggle" | |
| defaultChecked | |
| disabled={value === 'Disabled'} | |
| isLoading={value === 'Loading'} | |
| id={`${value}-default-checked`} | |
| /> | |
| </Col> | |
| <Col width="auto" className="display-flex align-items-center gap-3"> | |
| <Toggle | |
| {...toggleProps} | |
| label="Toggle" | |
| size="large" | |
| disabled={value === 'Disabled'} | |
| isLoading={value === 'Loading'} | |
| id={`${value}-large`} | |
| /> | |
| <Toggle | |
| {...toggleProps} | |
| label="Toggle" | |
| defaultChecked | |
| size="large" | |
| disabled={value === 'Disabled'} | |
| isLoading={value === 'Loading'} | |
| id={`${value}-large-checked`} | |
| /> | |
| </Col> | |
| <Col width="auto" className="display-flex align-items-center gap-3"> | |
| <Toggle | |
| {...toggleProps} | |
| label="Toggle" | |
| size="large" | |
| disabled={value === 'Disabled'} | |
| isLoading={value === 'Loading'} | |
| id={`${value}-large-icon`} | |
| icon | |
| /> | |
| <Toggle | |
| {...toggleProps} | |
| label="Toggle" | |
| defaultChecked | |
| size="large" | |
| disabled={value === 'Disabled'} | |
| isLoading={value === 'Loading'} | |
| id={`${value}-large-icon-checked`} | |
| icon | |
| /> | |
| </Col> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/tedi/components/form/toggle/toggle.stories.tsx` around lines 69 - 124,
TemplateStates renders multiple Toggle components sharing the same id={value},
causing duplicate DOM IDs; update each Toggle to receive a unique id (e.g.,
derive from value plus a suffix or index) so IDs are unique per instance. Locate
the Toggle usages in TemplateStates (the blocks passing {...toggleProps}, label,
size, defaultChecked, disabled/isLoading and id={value}) and replace id={value}
with a generated unique id string (for example `${value}-small-1`,
`${value}-large-2`, or `${value}-${index}`) or use a helper that composes value
with a position/variant identifier to guarantee uniqueness across the row.
Ensure the same unique id is used where needed for label association.
Contributor
Author
There was a problem hiding this comment.
@mart-sessman kas see on suur probleem storybooki accessibility jaoks v mitte? praegu paljud muud storyd sh ka angularis samamoodi kordavad seda id per row
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
ly-tempel-bitweb
approved these changes
Apr 14, 2026
github-actions Bot
pushed a commit
that referenced
this pull request
Apr 29, 2026
# [17.0.0](react-16.1.0...react-17.0.0) (2026-04-29) ### Bug Fixes * **checkbox:** invalid indicator hover border fix [#605](#605) ([#609](#609)) ([f1d62c6](f1d62c6)) * **select:** select placeholder no longer blocks context menu interactions [#584](#584) ([#585](#585)) ([e8d86ab](e8d86ab)) * **variables:** update core version, update variable names [#592](#592) ([#598](#598)) ([1f15b36](1f15b36)) ### Features * **button-group:** add mobile variant [#448](#448) ([#606](#606)) ([54dee90](54dee90)), closes [#94](#94) [#94](#94) [#94](#94) [#94](#94) [#94](#94) [#94](#94) [#94](#94) [#94](#94) [#94](#94) [#94](#94) [#94](#94) [#94](#94) [#94](#94) [#94](#94) * **card:** add more control to borderRadius usage, add examples [#444](#444) ([#597](#597)) ([deac9db](deac9db)) * **print:** introduce PrintingProvider + context-based usePrint [#99](#99) ([#497](#497)) ([a58cb70](a58cb70)) * **spinner:** add new sizes [#586](#586) ([#589](#589)) ([fbea0c3](fbea0c3)) * **tabs:** new tedi-ready component [#555](#555) ([#557](#557)) ([9c06c51](9c06c51)) * **textarea:** add autoGrow, height and maxHeight props [#588](#588) ([#593](#593)) ([2c86740](2c86740)) * **toggle:** new TEDI-Ready component [#305](#305) ([#594](#594)) ([6f28045](6f28045)) ### BREAKING CHANGES * **print:** usePrint hook removed. Replace with usePrint from the new PrintingProvider context.
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
Summary by CodeRabbit
New Features
Style
Tests
Documentation
Chores