fix(slider): Disallow invalid & warn when readonly or disabled by BiaBiruka · Pull Request #20784 · carbon-design-system/carbon

BiaBiruka · 2025-10-22T23:04:30Z

Closes #20731

Fix the behavior for Slider and Two Handle Slider. When component is disabled or readOnly, it should not show waning nor invalid message.

Changelog

Changed

Instead of using readOnly state to check if should display warnings or not, Slider now also checks if component is disabled. This way, warnings will not be shown if the component is disabled/readOnly.

Testing / Reviewing

Access the Slider playground on storybook (/?path=/story/components-slider--default) and change the state to "Invalid = true" and "disabled = true". The invalid message should not be shown.
Repeat above test with "readOnly = true" instead of disabled. The invalid message should not be shown.
Repeat the two tests above using "warn = true" instead of "invalid = true". The warning message on both scenarios should not be shown.

PR Checklist

As the author of this PR, before marking ready for review, confirm you:

Reviewed every line of the diff
~~Updated documentation and storybook examples~~
Wrote passing tests that cover this change
Addressed any impact on accessibility (a11y)
Tested for cross-browser consistency
Validated that this code is ready for review and status checks should pass

More details can be found in the pull request guide

…20731) fix slider to not show warning message when component is disabled or readOnly.

add test cases to not show warnings when disabled or readOnly.

github-actions · 2025-10-22T23:04:49Z

All contributors have signed the DCO.
_{Posted by the DCO Assistant Lite bot.}

BiaBiruka · 2025-10-22T23:05:48Z

I have read the DCO document and I hereby sign the DCO.

netlify · 2025-10-22T23:12:56Z

✅ Deploy Preview for carbon-elements ready!

Name	Link
🔨 Latest commit	`4fe30fd`
🔍 Latest deploy log	https://app.netlify.com/projects/carbon-elements/deploys/6900b2e10919260008f61738
😎 Deploy Preview	https://deploy-preview-20784--carbon-elements.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

netlify · 2025-10-22T23:13:09Z

✅ Deploy Preview for v11-carbon-react ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`4fe30fd`
🔍 Latest deploy log	https://app.netlify.com/projects/v11-carbon-react/deploys/6900b2e1d72eab0009870f44
😎 Deploy Preview	https://deploy-preview-20784--v11-carbon-react.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

riddhybansal

Hey, thanks for doing this.
This looks great just few changes I feel that needs to be address.

Variable Naming:

Consider renaming isCurrentlyEditable to isInteractive (preferred) or isEditableState.

Additional Test Cases:

Please add tests to verify:

Should not set aria-invalid when disabled with invalid/warn prop
Should not set aria-invalid when readOnly with invalid/warn prop

Consider doing same fix for web-component as well.

change variable name from "isCurrentlyEditable" to "isInteractive".

BiaBiruka · 2025-10-23T13:21:46Z

Thanks for the feedback!
The issues have been fixed and are ready for review again. Please let me know if there is anything else I can improve!

riddhybansal · 2025-10-23T13:38:34Z

Can you please do the same fix for the web-component slider as well.

netlify · 2025-10-23T13:39:17Z

✅ Deploy Preview for v11-carbon-web-components ready!

Name	Link
🔨 Latest commit	`4fe30fd`
🔍 Latest deploy log	https://app.netlify.com/projects/v11-carbon-web-components/deploys/6900b2e1778f610008e4ad85
😎 Deploy Preview	https://deploy-preview-20784--v11-carbon-web-components.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

codecov · 2025-10-23T13:46:40Z

Codecov Report

❌ Patch coverage is 97.18310% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.33%. Comparing base (766e563) to head (4fe30fd).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
packages/react/src/components/Slider/Slider.tsx	97.56%	1 Missing ⚠️
...b-components/src/components/slider/slider-input.ts	93.75%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #20784      +/-   ##
==========================================
- Coverage   92.46%   92.33%   -0.14%     
==========================================
  Files         508      511       +3     
  Lines       35753    37244    +1491     
  Branches     5673     5668       -5     
==========================================
+ Hits        33058    34388    +1330     
- Misses       2550     2709     +159     
- Partials      145      147       +2

Flag	Coverage Δ
main-packages	`85.50% <97.67%> (+0.01%)`	⬆️
web-components	`96.67% <96.42%> (-0.53%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

fix slider to not show warning message when component is disabled or readOnly.

…arbon into 20731-slider-warning

BiaBiruka · 2025-10-23T15:05:45Z

Done!
Just wanted to clarify one thing... Some components within the web-components package have unit tests, while others don't (the slider doesn't). Checking the Jest configuration, I noticed that the tests for this package are being ignored. Should I create them anyway?

riddhybansal · 2025-10-24T03:53:39Z

Done! Just wanted to clarify one thing... Some components within the web-components package have unit tests, while others don't (the slider doesn't). Checking the Jest configuration, I noticed that the tests for this package are being ignored. Should I create them anyway?

Yeah we are working on creating test files for all components for web-components. We have a whole of an epic for that you can go ahead and add test file for slider and add these test cases in it.

add test cases to not show warnings when disabled or readOnly (web-components).

BiaBiruka · 2025-10-24T17:32:37Z

Ok!
Test cases for invalid and warn states created.

riddhybansal

Thanks for your contribution.

riddhybansal

Can you please add these test cases

Please add tests to verify:

Should not set aria-invalid when disabled with invalid/warn prop
Should not set aria-invalid when readOnly with invalid/warn prop

improve tests for two handle slider to check for data-invalid prop.

refactor slider to use the useNormalizedInputProps hook.

…arbon into 20731-slider-warning

BiaBiruka · 2025-10-27T14:10:52Z

Can you please add these test cases

Please add tests to verify:

Should not set aria-invalid when disabled with invalid/warn prop Should not set aria-invalid when readOnly with invalid/warn prop

Done! 
I also refactored the fix to use the normalizedProps hook in order to follow the same pattern requested on my other PR (#20806).

Quick question: for the IBM hacktoberfest event we were requested to add a "hacktoberfest" label on the pull requests for easier tracking of what we made. Would that be possible?

riddhybansal · 2025-10-27T14:51:41Z

Can you please add these test cases
Please add tests to verify:
Should not set aria-invalid when disabled with invalid/warn prop Should not set aria-invalid when readOnly with invalid/warn prop

Done!  I also refactored the fix to use the normalizedProps hook in order to follow the same pattern requested on my other PR (#20806).

Quick question: for the IBM hacktoberfest event we were requested to add a "hacktoberfest" label on the pull requests for easier tracking of what we made. Would that be possible?

Thank you !! That is just perfect !!

BiaBiruka added 3 commits October 22, 2025 19:55

docs: add contributor

e4a21a9

fix(slider): fix invalid warning when disabled (carbon-design-system#…

3db43ab

…20731) fix slider to not show warning message when component is disabled or readOnly.

test(slider): add tests for slider warning fixes

58007ae

add test cases to not show warnings when disabled or readOnly.

BiaBiruka requested a review from a team as a code owner October 22, 2025 23:04

BiaBiruka requested review from Gururajj77 and heloiselui October 22, 2025 23:04

riddhybansal requested changes Oct 23, 2025

View reviewed changes

riddhybansal and others added 3 commits October 23, 2025 12:01

Merge branch 'main' into 20731-slider-warning

ea7de65

fix(slider): change variable name

ba56494

change variable name from "isCurrentlyEditable" to "isInteractive".

test(slider): add missing validations

4b9be82

Merge branch 'main' into 20731-slider-warning

fbb57be

riddhybansal and others added 3 commits October 23, 2025 19:40

Merge branch 'main' into 20731-slider-warning

a6db581

fix(slider): fix invalid warning when disabled (web-components)

e1d4e7d

fix slider to not show warning message when component is disabled or readOnly.

Merge branch '20731-slider-warning' of https://github.com/BiaBiruka/c…

9497dda

…arbon into 20731-slider-warning

test(slider): add tests for slider warning fixes

9433d87

add test cases to not show warnings when disabled or readOnly (web-components).

BiaBiruka added 2 commits October 24, 2025 14:32

Merge branch 'main' into 20731-slider-warning

6294979

Merge branch 'main' into 20731-slider-warning

2a897b5

riddhybansal approved these changes Oct 27, 2025

View reviewed changes

riddhybansal reviewed Oct 27, 2025

View reviewed changes

test(slider): improve tests for slider warning fixes

bd4aa9a

improve tests for two handle slider to check for data-invalid prop.

Gururajj77 approved these changes Oct 27, 2025

View reviewed changes

BiaBiruka added 3 commits October 27, 2025 11:04

refactor(slider): use normalized props hook

e56ede4

refactor slider to use the useNormalizedInputProps hook.

Merge branch '20731-slider-warning' of https://github.com/BiaBiruka/c…

d101dc5

…arbon into 20731-slider-warning

test(slider): add test cases for aria-invalid

4f67f94

riddhybansal added the hacktoberfest See https://hacktoberfest.com/ label Oct 27, 2025

Merge branch 'main' into 20731-slider-warning

c118518

riddhybansal approved these changes Oct 27, 2025

View reviewed changes

riddhybansal enabled auto-merge October 27, 2025 14:52

BiaBiruka and others added 2 commits October 27, 2025 14:54

Merge branch 'main' into 20731-slider-warning

3c253c7

Merge branch 'main' into 20731-slider-warning

74161a8

riddhybansal added the status: ready to merge 🎉 label Oct 28, 2025

riddhybansal deployed to @carbon/react storybook preview October 28, 2025 04:00 — with GitHub Actions View deployment

riddhybansal added this pull request to the merge queue Oct 28, 2025

github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Oct 28, 2025

Merge branch 'main' into 20731-slider-warning

4fe30fd

riddhybansal enabled auto-merge October 28, 2025 12:49

BiaBiruka deployed to @carbon/react storybook preview October 28, 2025 13:10 — with GitHub Actions View deployment

riddhybansal added this pull request to the merge queue Oct 28, 2025

Merged via the queue into carbon-design-system:main with commit bc87e48 Oct 28, 2025
41 of 43 checks passed

maradwan26 mentioned this pull request Nov 3, 2025

feat: Added Checkbox behavior to not show invalid / warn when readOnly / disabled #20804

Merged

4 tasks

Conversation

BiaBiruka commented Oct 22, 2025

Changelog

Testing / Reviewing

PR Checklist

Uh oh!

github-actions Bot commented Oct 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BiaBiruka commented Oct 22, 2025

Uh oh!

netlify Bot commented Oct 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for carbon-elements ready!

Uh oh!

netlify Bot commented Oct 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for v11-carbon-react ready!

Uh oh!

riddhybansal left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Variable Naming:

Additional Test Cases:

Uh oh!

BiaBiruka commented Oct 23, 2025

Uh oh!

riddhybansal commented Oct 23, 2025

Uh oh!

netlify Bot commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for v11-carbon-web-components ready!

Uh oh!

codecov Bot commented Oct 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

BiaBiruka commented Oct 23, 2025

Uh oh!

riddhybansal commented Oct 24, 2025

Uh oh!

BiaBiruka commented Oct 24, 2025

Uh oh!

riddhybansal left a comment

Choose a reason for hiding this comment

Uh oh!

riddhybansal left a comment

Choose a reason for hiding this comment

Uh oh!

BiaBiruka commented Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

riddhybansal commented Oct 27, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Oct 22, 2025 •

edited

Loading

netlify Bot commented Oct 22, 2025 •

edited

Loading

netlify Bot commented Oct 22, 2025 •

edited

Loading

riddhybansal left a comment •

edited

Loading

netlify Bot commented Oct 23, 2025 •

edited

Loading

codecov Bot commented Oct 23, 2025 •

edited

Loading

BiaBiruka commented Oct 27, 2025 •

edited

Loading