Skip to content

Commit f8ac84e

Browse files
jvictorfsilvariddhybansalmaradwan26heloiselui
authored
fix(dropdown): Disallow invalid & warn when readonly or disabled (#20803)
* fix(dropdown): prevent validation states in disabled/readonly dropdowns * refactor(dropdown): use normalizedProps for validation state management * refactor(dropdown): update validation state logic * fix(dropdown): hide warningIcon when readonly or disabled * fix(dropdown): revert removed expect from unit test --------- Co-authored-by: Riddhi Bansal <41935566+riddhybansal@users.noreply.github.com> Co-authored-by: Mahmoud <132728978+maradwan26@users.noreply.github.com> Co-authored-by: Heloise Lui <71858203+heloiselui@users.noreply.github.com>
1 parent d7b276e commit f8ac84e

File tree

5 files changed

+341
-47
lines changed

5 files changed

+341
-47
lines changed

‎packages/react/src/components/Dropdown/Dropdown.tsx‎

Lines changed: 40 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ import { deprecate } from '../../prop-types/deprecate';
4848
import { usePrefix } from '../../internal/usePrefix';
4949
import { FormContext } from '../FluidForm';
5050
import type { TranslateWithId } from '../../types/common';
51-
import { useId } from '../../internal/useId';
51+
import { useNormalizedInputProps } from '../../internal/useNormalizedInputProps';
5252
import {
5353
useFloating,
5454
flip,
@@ -430,7 +430,6 @@ const Dropdown = React.forwardRef(
430430
downshiftProps,
431431
]
432432
);
433-
const dropdownInstanceId = useId();
434433

435434
// only set selectedItem if the prop is defined. Setting if it is undefined
436435
// will overwrite default selected items from useSelect
@@ -448,17 +447,26 @@ const Dropdown = React.forwardRef(
448447
highlightedIndex,
449448
} = useSelect(selectProps);
450449
const inline = type === 'inline';
451-
const showWarning = !invalid && warn;
450+
451+
const normalizedProps = useNormalizedInputProps({
452+
id,
453+
readOnly,
454+
disabled: disabled ?? false,
455+
invalid: invalid ?? false,
456+
invalidText,
457+
warn: warn ?? false,
458+
warnText,
459+
});
452460

453461
const [isFocused, setIsFocused] = useState(false);
454462

455463
const className = cx(`${prefix}--dropdown`, {
456-
[`${prefix}--dropdown--invalid`]: invalid,
457-
[`${prefix}--dropdown--warning`]: showWarning,
464+
[`${prefix}--dropdown--invalid`]: normalizedProps.invalid,
465+
[`${prefix}--dropdown--warning`]: normalizedProps.warn,
458466
[`${prefix}--dropdown--open`]: isOpen,
459467
[`${prefix}--dropdown--focus`]: isFocused,
460468
[`${prefix}--dropdown--inline`]: inline,
461-
[`${prefix}--dropdown--disabled`]: disabled,
469+
[`${prefix}--dropdown--disabled`]: normalizedProps.disabled,
462470
[`${prefix}--dropdown--light`]: light,
463471
[`${prefix}--dropdown--readonly`]: readOnly,
464472
[`${prefix}--dropdown--${size}`]: size,
@@ -467,12 +475,12 @@ const Dropdown = React.forwardRef(
467475
});
468476

469477
const titleClasses = cx(`${prefix}--label`, {
470-
[`${prefix}--label--disabled`]: disabled,
478+
[`${prefix}--label--disabled`]: normalizedProps.disabled,
471479
[`${prefix}--visually-hidden`]: hideLabel,
472480
});
473481

474482
const helperClasses = cx(`${prefix}--form__helper-text`, {
475-
[`${prefix}--form__helper-text--disabled`]: disabled,
483+
[`${prefix}--form__helper-text--disabled`]: normalizedProps.disabled,
476484
});
477485

478486
const wrapperClasses = cx(
@@ -482,18 +490,17 @@ const Dropdown = React.forwardRef(
482490
{
483491
[`${prefix}--dropdown__wrapper--inline`]: inline,
484492
[`${prefix}--list-box__wrapper--inline`]: inline,
485-
[`${prefix}--dropdown__wrapper--inline--invalid`]: inline && invalid,
486-
[`${prefix}--list-box__wrapper--inline--invalid`]: inline && invalid,
487-
[`${prefix}--list-box__wrapper--fluid--invalid`]: isFluid && invalid,
493+
[`${prefix}--dropdown__wrapper--inline--invalid`]:
494+
inline && normalizedProps.invalid,
495+
[`${prefix}--list-box__wrapper--inline--invalid`]:
496+
inline && normalizedProps.invalid,
497+
[`${prefix}--list-box__wrapper--fluid--invalid`]:
498+
isFluid && normalizedProps.invalid,
488499
[`${prefix}--list-box__wrapper--slug`]: slug,
489500
[`${prefix}--list-box__wrapper--decorator`]: decorator,
490501
}
491502
);
492503

493-
const helperId = !helperText
494-
? undefined
495-
: `dropdown-helper-text-${dropdownInstanceId}`;
496-
497504
// needs to be Capitalized for react to render it correctly
498505
const ItemToElement = itemToElement;
499506
const toggleButtonProps = getToggleButtonProps({
@@ -502,7 +509,7 @@ const Dropdown = React.forwardRef(
502509

503510
const helper =
504511
helperText && !isFluid ? (
505-
<div id={helperId} className={helperClasses}>
512+
<div id={normalizedProps.helperId} className={helperClasses}>
506513
{helperText}
507514
</div>
508515
) : null;
@@ -618,18 +625,16 @@ const Dropdown = React.forwardRef(
618625
onBlur={handleFocus}
619626
size={size}
620627
className={className}
621-
invalid={invalid}
622-
invalidText={invalidText}
623-
warn={warn}
624-
warnText={warnText}
628+
invalid={normalizedProps.invalid}
629+
warn={normalizedProps.warn}
625630
light={light}
626631
isOpen={isOpen}
627632
ref={enableFloatingStyles || autoAlign ? refs.setReference : null}
628633
id={id}>
629-
{invalid && (
634+
{normalizedProps.invalid && (
630635
<WarningFilled className={`${prefix}--list-box__invalid-icon`} />
631636
)}
632-
{showWarning && (
637+
{normalizedProps.warn && (
633638
<WarningAltFilled
634639
className={`${prefix}--list-box__invalid-icon ${prefix}--list-box__invalid-icon--warning`}
635640
/>
@@ -638,10 +643,19 @@ const Dropdown = React.forwardRef(
638643
type="button"
639644
// aria-expanded is already being passed through {...toggleButtonProps}
640645
className={`${prefix}--list-box__field`}
641-
disabled={disabled}
646+
disabled={normalizedProps.disabled}
642647
aria-disabled={readOnly ? true : undefined} // aria-disabled to remain focusable
643648
aria-describedby={
644-
!inline && !invalid && !warn && helper ? helperId : undefined
649+
!inline &&
650+
!normalizedProps.invalid &&
651+
!normalizedProps.warn &&
652+
helper
653+
? normalizedProps.helperId
654+
: normalizedProps.invalid
655+
? normalizedProps.invalidId
656+
: normalizedProps.warn
657+
? normalizedProps.warnId
658+
: undefined
645659
}
646660
title={
647661
selectedItem && itemToString !== undefined
@@ -710,7 +724,8 @@ const Dropdown = React.forwardRef(
710724
})}
711725
</ListBox.Menu>
712726
</ListBox>
713-
{!inline && !invalid && !warn && helper}
727+
{!inline && !isFluid && !normalizedProps.validation && helper}
728+
{!inline && !isFluid && normalizedProps.validation}
714729
</div>
715730
);
716731
}

‎packages/react/src/components/Dropdown/__tests__/Dropdown-test.js‎

Lines changed: 139 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -411,6 +411,142 @@ describe('Dropdown', () => {
411411
});
412412
});
413413

414+
describe('Validation states with disabled/readonly', () => {
415+
let mockProps;
416+
417+
beforeEach(() => {
418+
mockProps = {
419+
id: 'test-dropdown',
420+
items: generateItems(5, generateGenericItem),
421+
onChange: jest.fn(),
422+
label: 'input',
423+
titleText: 'Dropdown label',
424+
};
425+
});
426+
427+
it('should not show invalid state when disabled', async () => {
428+
const { container } = render(
429+
<Dropdown {...mockProps} disabled invalid invalidText="Error message" />
430+
);
431+
await waitForPosition();
432+
433+
expect(
434+
container.querySelector(`.${prefix}--dropdown--invalid`)
435+
).not.toBeInTheDocument();
436+
expect(screen.queryByText('Error message')).not.toBeInTheDocument();
437+
expect(
438+
container.querySelector(`.${prefix}--list-box__invalid-icon`)
439+
).not.toBeInTheDocument();
440+
});
441+
442+
it('should not show invalid state when readonly', async () => {
443+
const { container } = render(
444+
<Dropdown {...mockProps} readOnly invalid invalidText="Error message" />
445+
);
446+
await waitForPosition();
447+
448+
expect(
449+
container.querySelector(`.${prefix}--dropdown--invalid`)
450+
).not.toBeInTheDocument();
451+
expect(screen.queryByText('Error message')).not.toBeInTheDocument();
452+
expect(
453+
container.querySelector(`.${prefix}--list-box__invalid-icon`)
454+
).not.toBeInTheDocument();
455+
});
456+
457+
it('should not show warning state when disabled', async () => {
458+
const { container } = render(
459+
<Dropdown {...mockProps} disabled warn warnText="Warning message" />
460+
);
461+
await waitForPosition();
462+
463+
expect(
464+
container.querySelector(`.${prefix}--dropdown--warning`)
465+
).not.toBeInTheDocument();
466+
expect(screen.queryByText('Warning message')).not.toBeInTheDocument();
467+
expect(
468+
container.querySelector(`.${prefix}--list-box__invalid-icon--warning`)
469+
).not.toBeInTheDocument();
470+
});
471+
472+
it('should not show warning state when readonly', async () => {
473+
const { container } = render(
474+
<Dropdown {...mockProps} readOnly warn warnText="Warning message" />
475+
);
476+
await waitForPosition();
477+
478+
expect(
479+
container.querySelector(`.${prefix}--dropdown--warning`)
480+
).not.toBeInTheDocument();
481+
expect(screen.queryByText('Warning message')).not.toBeInTheDocument();
482+
expect(
483+
container.querySelector(`.${prefix}--list-box__invalid-icon--warning`)
484+
).not.toBeInTheDocument();
485+
});
486+
487+
it('should show invalid state when interactive', async () => {
488+
const { container } = render(
489+
<Dropdown {...mockProps} invalid invalidText="Error message" />
490+
);
491+
await waitForPosition();
492+
493+
expect(
494+
container.querySelector(`.${prefix}--dropdown--invalid`)
495+
).toBeInTheDocument();
496+
expect(screen.getByText('Error message')).toBeInTheDocument();
497+
expect(
498+
container.querySelector(`.${prefix}--list-box__invalid-icon`)
499+
).toBeInTheDocument();
500+
});
501+
502+
it('should show warning state when interactive', async () => {
503+
const { container } = render(
504+
<Dropdown {...mockProps} warn warnText="Warning message" />
505+
);
506+
await waitForPosition();
507+
508+
expect(
509+
container.querySelector(`.${prefix}--dropdown--warning`)
510+
).toBeInTheDocument();
511+
expect(screen.getByText('Warning message')).toBeInTheDocument();
512+
expect(
513+
container.querySelector(`.${prefix}--list-box__invalid-icon--warning`)
514+
).toBeInTheDocument();
515+
});
516+
517+
it('should show helper text instead of validation when disabled', async () => {
518+
render(
519+
<Dropdown
520+
{...mockProps}
521+
disabled
522+
invalid
523+
invalidText="Error"
524+
helperText="Helper text"
525+
/>
526+
);
527+
await waitForPosition();
528+
529+
expect(screen.getByText('Helper text')).toBeInTheDocument();
530+
expect(screen.queryByText('Error')).not.toBeInTheDocument();
531+
});
532+
533+
it('should show helper text instead of validation when readonly', async () => {
534+
render(
535+
<Dropdown
536+
{...mockProps}
537+
readOnly
538+
warn
539+
warnText="Warning"
540+
helperText="Helper text"
541+
/>
542+
);
543+
await waitForPosition();
544+
545+
expect(screen.getByText('Helper text')).toBeInTheDocument();
546+
expect(screen.queryByText('Warning')).not.toBeInTheDocument();
547+
});
548+
});
549+
414550
describe('DropdownSkeleton', () => {
415551
describe('Renders as expected', () => {
416552
it('Has the expected classes', () => {
@@ -470,8 +606,8 @@ describe('Test useEffect ', () => {
470606

471607
expect(attributes).toEqual({
472608
class: 'cds--label',
473-
for: 'downshift-_r_2d_-toggle-button',
474-
id: 'downshift-_r_2d_-label',
609+
for: 'downshift-_r_25_-toggle-button',
610+
id: 'downshift-_r_25_-label',
475611
});
476612
});
477613

@@ -486,7 +622,7 @@ describe('Test useEffect ', () => {
486622

487623
expect(attributes).toEqual({
488624
class: 'cds--label',
489-
id: 'downshift-_r_2g_-label',
625+
id: 'downshift-_r_27_-label',
490626
});
491627
});
492628
});

0 commit comments

Comments
 (0)