Skip to content

Commit

Permalink
Solving issue with having an id on the SelectButtonElement that was b…
Browse files Browse the repository at this point in the history
…reaking Riverty & CtP unit tests
  • Loading branch information
sponglord committed Jul 15, 2024
1 parent 99c146d commit 880595b
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ function Installments(props: InstallmentsProps) {
onChange={onSelectInstallment}
name={'installments'}
readonly={installmentOptions?.values?.length === 1}
allowIdOnButton={true}
/>
</Field>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ function Select({
disableTextFilter,
clearOnSelect,
blurOnClose,
onListToggle
onListToggle,
allowIdOnButton = false
}: SelectProps) {
const filterInputRef = useRef(null);
const selectContainerRef = useRef(null);
Expand Down Expand Up @@ -289,6 +290,7 @@ function Select({
toggleList={toggleList}
disabled={disabled}
ariaDescribedBy={uniqueId ? `${uniqueId}${ARIA_ERROR_SUFFIX}` : null}
allowIdOnButton={allowIdOnButton}
/>
<SelectList
active={activeOption}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@ import styles from '../Select.module.scss';
import Img from '../../../Img';

function SelectButtonElement({ filterable, toggleButtonRef, ...props }) {
if (filterable) return <div {...props} ref={toggleButtonRef} />;
if (filterable) {
// Even if passed, we can't add an id to this div since it is not allowed to associate a div with a label element
const { id, ...strippedProps } = props;
return <div {...strippedProps} ref={toggleButtonRef} />;
}

return <button id={props.id} aria-describedby={props.ariaDescribedBy} type={'button'} {...props} ref={toggleButtonRef} />;
}
Expand Down Expand Up @@ -58,7 +62,10 @@ function SelectButton(props: Readonly<SelectButtonProps>) {
onClick={onClickHandler}
onKeyDown={!readonly ? props.onButtonKeyDown : null}
toggleButtonRef={props.toggleButtonRef}
{...(props.id && { id: props.id })}
// Only for some dropdowns e.g. the one found in installments when it is just in the form of a single dropdown, do we want to add an id that links to a label's for attr
// If we allow an id to be added to the buttons in CtPCardsList, for example, unit tests start failing because it seems a button with an id no longer has a name property that can be used
// as a qualifier in findByRole
{...(props.allowIdOnButton && props.id && { id: props.id })}
>
{!props.filterable ? (
<Fragment>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export interface SelectProps {
clearOnSelect?: boolean;
blurOnClose?: boolean;
onListToggle?: (isOpen: boolean) => void;
allowIdOnButton?: boolean;
}

export interface SelectButtonProps {
Expand All @@ -61,6 +62,7 @@ export interface SelectButtonProps {
id?: string;
ariaDescribedBy: string;
disabled: boolean;
allowIdOnButton?: boolean;
}

export interface SelectListProps {
Expand Down

0 comments on commit 880595b

Please sign in to comment.