Skip to content

fix(dropdown): Disallow invalid & warn when readonly or disabled#20803

Merged
riddhybansal merged 20 commits into
carbon-design-system:mainfrom
jvictorfsilva:dropdown-warn-fix
Nov 13, 2025
Merged

fix(dropdown): Disallow invalid & warn when readonly or disabled#20803
riddhybansal merged 20 commits into
carbon-design-system:mainfrom
jvictorfsilva:dropdown-warn-fix

Conversation

@jvictorfsilva

Copy link
Copy Markdown
Contributor

Closes #20723

Fix Dropdown showing validation states when disabled or readonly

Description

Prevents dropdown components (React and Web Components) from displaying invalid/warn states when in disabled or readonly mode, as users cannot interact with non-interactive components to fix validation issues.

Changelog

Changed

React Component (Dropdown.tsx):

  • Added isInteractive flag (!readOnly && !disabled) to conditionally show validation states
  • Introduced derived values showInvalid and showWarning based on interactivity
  • Applied consistently across: CSS classes, ListBox props, validation icons, aria-describedby, and helper text rendering

Web Components (dropdown.ts):

  • Changed invalid and warn properties from reflect: true to reflect: false
  • Added manual attribute reflection in updated() method to control when attributes appear on host element
  • Implemented same isInteractive logic in _classes getter and render() method
  • Fixed shouldUpdate() to properly enable/disable child items when disabled property toggles
  • Applied validation state logic to: CSS classes, helper text classes, validation icons, and helper message display

Testing / Reviewing

React Component:

  1. Open Dropdown component in Storybook
  2. Set invalid={true} and invalidText="Error message"
  3. Verify invalid state is visible (red border, error icon, error text)
  4. Set disabled={true}
  5. Verify invalid state is hidden (no red border, no error icon, helper text shown instead)
  6. Set disabled={false}
  7. Verify invalid state returns
  8. Repeat steps 4-7 with readOnly={true}
  9. Test same flow with warn prop

Web Components:

  1. Create dropdown with invalid and invalid-text="Error message"
  2. Verify invalid state is visible
  3. Inspect element - confirm invalid attribute exists on <cds-dropdown>
  4. Set disabled attribute
  5. Verify invalid state is hidden and invalid attribute is removed from host
  6. Repeat with read-only attribute

Expected behavior:

  • When disabled=true or readOnly=true: No validation classes, no validation icons, no validation text, no validation attributes on host element
  • When interactive (default): Validation states display normally
  • Helper text displays when non-interactive, validation text displays when interactive and invalid/warn

Previous behavior:

  • Validation states (invalid/warn) were shown even when user couldn't interact with the component
  • Created confusing UX where disabled/readonly fields showed errors users couldn't fix

Accessibility Impact

Improved:

  • aria-describedby correctly associates helper text instead of validation text when non-interactive
  • Validation attributes removed from host element when non-interactive, preventing screen readers from announcing errors on disabled/readonly fields
  • Helper text remains available to assistive technologies for disabled/readonly states

PR Checklist

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

  • Reviewed every line of the diff
  • 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

@jvictorfsilva jvictorfsilva requested a review from a team as a code owner October 24, 2025 19:04
@netlify

netlify Bot commented Oct 24, 2025

Copy link
Copy Markdown

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

Name Link
🔨 Latest commit 6ee41c2
🔍 Latest deploy log https://app.netlify.com/projects/v11-carbon-web-components/deploys/69159ff03884cb000785614b
😎 Deploy Preview https://deploy-preview-20803--v11-carbon-web-components.netlify.app
📱 Preview on mobile
Toggle QR Code...

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

netlify Bot commented Oct 24, 2025

Copy link
Copy Markdown

Deploy Preview for v11-carbon-react ready!

Name Link
🔨 Latest commit 6ee41c2
🔍 Latest deploy log https://app.netlify.com/projects/v11-carbon-react/deploys/69159ff0da43560008d1d51d
😎 Deploy Preview https://deploy-preview-20803--v11-carbon-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

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

netlify Bot commented Oct 24, 2025

Copy link
Copy Markdown

Deploy Preview for carbon-elements ready!

Name Link
🔨 Latest commit 6ee41c2
🔍 Latest deploy log https://app.netlify.com/projects/carbon-elements/deploys/69159ff0328f4e0008e2402e
😎 Deploy Preview https://deploy-preview-20803--carbon-elements.netlify.app
📱 Preview on mobile
Toggle QR Code...

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

codecov Bot commented Oct 27, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.39%. Comparing base (d7b276e) to head (6ee41c2).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #20803      +/-   ##
==========================================
+ Coverage   92.37%   92.39%   +0.01%     
==========================================
  Files         515      515              
  Lines       37605    37629      +24     
  Branches     5782     5740      -42     
==========================================
+ Hits        34739    34766      +27     
+ Misses       2716     2714       -2     
+ Partials      150      149       -1     
Flag Coverage Δ
main-packages 85.57% <100.00%> (+0.02%) ⬆️
web-components 96.66% <100.00%> (+<0.01%) ⬆️

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.

@riddhybansal riddhybansal left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey this looks good

Small Suggestions for Improvement
Code Consistency

Consider using useNormalizedInputProps in all components rather than creating separate validation logic:

normalizedProps = useNormalizedInputProps({
id, disabled, readOnly, invalid, invalidText, warn, warnText,
});

Then reference normalizedProps.invalid and normalizedProps.warn for validation states
Our input controls already use this pattern, which makes future fixes simpler (just update the hook once).

Here is the reference PR

@jvictorfsilva

Copy link
Copy Markdown
Contributor Author

The changes have been applied. Could you please add the Hacktoberfest label? If possible, also include it on pull request #20791

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

@riddhybansal riddhybansal left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi there, the React implementation works as expected, but I've identified an issue with the web-components version. When a component is in an invalid state and I enable the disabled attribute, the invalid styles correctly disappear. However, when I subsequently remove the disabled attribute while the component remains invalid, the invalid styles fail to reappear as they should.

Screen.Recording.2025-10-28.at.11.57.19.AM.mov

@jvictorfsilva

Copy link
Copy Markdown
Contributor Author

I've completed the requested changes. Do you have any estimate on when my other pull request might be reviewed?

@maradwan26 maradwan26 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for contributing! Just one small comment, the rest LGTM

@maradwan26 maradwan26 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx! LGTM

@heloiselui

Copy link
Copy Markdown
Contributor

@riddhybansal could you approve this PR so we can merge it? I think you requested some changes. Thanks!

@riddhybansal riddhybansal added this pull request to the merge queue Nov 13, 2025
Merged via the queue into carbon-design-system:main with commit f8ac84e Nov 13, 2025
41 of 43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

hacktoberfest See https://hacktoberfest.com/ status: ready to merge 🎉

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Dropdown]: Disallow invalid & warn when readonly or disabled

5 participants