Skip to content

Commit

Permalink
fix(dropdown): broken forwardRef binding (#7270)
Browse files Browse the repository at this point in the history
* fix(dropdown): broken forwardRef binding

* fix(multiselect): merge dropdown refs

* fix(dropdown): merge additional dropdown refs

* fix(multiselect): update yarn.lock

* fix(multiselect): fix broken yarn.lock

* fix(react): update Dropdown and MultiSelect to merge refs

* chore(project): update offline mirror

Co-authored-by: Conrad Schmidt <conrad.schmidt@de.ibm.com>
Co-authored-by: Josh Black <josh@josh.black>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
  • Loading branch information
4 people committed Dec 8, 2020
1 parent 5fcbf46 commit 6e25954
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 6 deletions.
11 changes: 11 additions & 0 deletions packages/react/src/components/Dropdown/Dropdown-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* LICENSE file in the root directory of this source tree.
*/

import { cleanup, render } from '@testing-library/react';
import React from 'react';
import { mount, shallow } from 'enzyme';
import {
Expand Down Expand Up @@ -177,6 +178,16 @@ describe('Dropdown', () => {
);
});
});

describe('Component API', () => {
afterEach(cleanup);

it('should accept a `ref` for the underlying button element', () => {
const ref = React.createRef();
render(<Dropdown {...mockProps} ref={ref} />);
expect(ref.current.getAttribute('aria-haspopup')).toBe('listbox');
});
});
});

describe('DropdownSkeleton', () => {
Expand Down
7 changes: 4 additions & 3 deletions packages/react/src/components/Dropdown/Dropdown.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
} from '@carbon/icons-react';
import ListBox, { PropTypes as ListBoxPropTypes } from '../ListBox';
import { mapDownshiftProps } from '../../tools/createPropAdapter';
import mergeRefs from '../../tools/mergeRefs';
import deprecate from '../../prop-types/deprecate';

const { prefix } = settings;
Expand Down Expand Up @@ -116,7 +117,7 @@ const Dropdown = React.forwardRef(function Dropdown(

// needs to be Capitalized for react to render it correctly
const ItemToElement = itemToElement;

const toggleButtonProps = getToggleButtonProps();
const helper = helperText ? (
<div className={helperClasses}>{helperText}</div>
) : null;
Expand Down Expand Up @@ -155,11 +156,11 @@ const Dropdown = React.forwardRef(function Dropdown(
)}
<button
type="button"
ref={ref}
className={`${prefix}--list-box__field`}
disabled={disabled}
aria-disabled={disabled}
{...getToggleButtonProps()}>
{...toggleButtonProps}
ref={mergeRefs(toggleButtonProps.ref, ref)}>
<span className={`${prefix}--list-box__label`}>
{selectedItem ? itemToString(selectedItem) : label}
</span>
Expand Down
7 changes: 5 additions & 2 deletions packages/react/src/components/MultiSelect/MultiSelect.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { defaultSortItems, defaultCompareItems } from './tools/sorting';
import { useSelection } from '../../internal/Selection';
import setupGetInstanceId from '../../tools/setupGetInstanceId';
import { mapDownshiftProps } from '../../tools/createPropAdapter';
import mergeRefs from '../../tools/mergeRefs';

const { prefix } = settings;
const noop = () => {};
Expand Down Expand Up @@ -188,6 +189,8 @@ const MultiSelect = React.forwardRef(function MultiSelect(
}
}

const toggleButtonProps = getToggleButtonProps();

return (
<div className={wrapperClasses}>
{titleText && (
Expand Down Expand Up @@ -217,11 +220,11 @@ const MultiSelect = React.forwardRef(function MultiSelect(
)}
<button
type="button"
ref={ref}
className={`${prefix}--list-box__field`}
disabled={disabled}
aria-disabled={disabled}
{...getToggleButtonProps()}>
{...toggleButtonProps}
ref={mergeRefs(toggleButtonProps.ref, ref)}>
{selectedItems.length > 0 && (
<ListBox.Selection
clearSelection={!disabled ? clearSelection : noop}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import { getByText, isElementVisible } from '@carbon/test-utils/dom';
import { pressEnter, pressSpace, pressTab } from '@carbon/test-utils/keyboard';
import { render, cleanup } from '@carbon/test-utils/react';
import { cleanup, render } from '@testing-library/react';
import React from 'react';
import { act, Simulate } from 'react-dom/test-utils';
import MultiSelect from '../';
Expand Down Expand Up @@ -400,5 +400,13 @@ describe('MultiSelect', () => {
// the first option in the list to the the former third option in the list
expect(optionsArray[0].title).toBe('Item 2');
});

it('should accept a `ref` for the underlying button element', () => {
const ref = React.createRef();
const items = generateItems(4, generateGenericItem);
const label = 'test-label';
render(<MultiSelect id="test" label={label} items={items} ref={ref} />);
expect(ref.current.getAttribute('aria-haspopup')).toBe('listbox');
});
});
});

0 comments on commit 6e25954

Please sign in to comment.