Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ListBox): fix labels of trigger buttons #4820

Merged
merged 15 commits into from
Jan 9, 2020
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions packages/react/src/components/ComboBox/ComboBox.js
Original file line number Diff line number Diff line change
Expand Up @@ -278,16 +278,22 @@ export default class ComboBox extends React.Component {
const titleClasses = cx(`${prefix}--label`, {
[`${prefix}--label--disabled`]: disabled,
});
const comboBoxHelperId = !helperText
? undefined
: `combobox-helper-text-${this.comboBoxInstanceId}`;
const comboBoxLabelId = `combobox-label-${this.comboBoxInstanceId}`;
const title = titleText ? (
<label htmlFor={id} className={titleClasses}>
<label id={comboBoxLabelId} htmlFor={id} className={titleClasses}>
{titleText}
</label>
) : null;
const helperClasses = cx(`${prefix}--form__helper-text`, {
[`${prefix}--form__helper-text--disabled`]: disabled,
});
const helper = helperText ? (
<div className={helperClasses}>{helperText}</div>
<div id={comboBoxHelperId} className={helperClasses}>
{helperText}
</div>
) : null;
const wrapperClasses = cx(`${prefix}--list-box__wrapper`);
const comboBoxA11yId = `combobox-a11y-${this.comboBoxInstanceId}`;
Expand Down Expand Up @@ -332,7 +338,8 @@ export default class ComboBox extends React.Component {
<ListBox.Field
id={id}
disabled={disabled}
translateWithId={translateWithId}
aria-labelledby={comboBoxLabelId}
aria-describedby={comboBoxHelperId}
{...getButtonProps({
disabled,
onClick: this.onToggleClick(isOpen),
Expand Down
2 changes: 1 addition & 1 deletion packages/react/src/components/Dropdown/Dropdown-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ describe('Dropdown', () => {

beforeEach(() => {
wrapper = mount(<Dropdown titleText="Email Input" {...mockProps} />);
renderedLabel = wrapper.find('label');
renderedLabel = wrapper.find('span[className="bx--label"]');
});

it('renders a title', () => {
Expand Down
144 changes: 80 additions & 64 deletions packages/react/src/components/Dropdown/Dropdown.js
Original file line number Diff line number Diff line change
Expand Up @@ -190,16 +190,24 @@ export default class Dropdown extends React.Component {
[`${prefix}--label--disabled`]: disabled,
});

const helperId = !helperText
? undefined
: `dropdown-helper-text-${this.dropdownInstanceId}`;
const labelId = `dropdown-label-${this.dropdownInstanceId}`;
const fieldLabelId = `dropdown-field-label-${this.dropdownInstanceId}`;

const title = titleText ? (
<label htmlFor={id} className={titleClasses}>
<span className={titleClasses}>
{titleText}
</label>
</span>
) : null;
const helperClasses = cx(`${prefix}--form__helper-text`, {
[`${prefix}--form__helper-text--disabled`]: disabled,
});
const helper = helperText ? (
<div className={helperClasses}>{helperText}</div>
<div id={helperId} className={helperClasses}>
{helperText}
</div>
) : null;
const wrapperClasses = cx(
`${prefix}--dropdown__wrapper`,
Expand Down Expand Up @@ -234,69 +242,77 @@ export default class Dropdown extends React.Component {
getItemProps,
getLabelProps,
toggleMenu,
}) => (
<ListBox
type={type}
size={size}
id={id}
aria-label={ariaLabel}
className={className({ isOpen })}
disabled={disabled}
isOpen={isOpen}
invalid={invalid}
invalidText={invalidText}
light={light}
{...getRootProps({ refKey: 'innerRef' })}>
{invalid && (
<WarningFilled16
className={`${prefix}--list-box__invalid-icon`}
/>
)}
<ListBox.Field
}) => {
const buttonProps = {
...getButtonProps({
onKeyDown: event => {
if (match(event, keys.Enter)) {
toggleMenu();
}
},
disabled,
}),
'aria-label': undefined,
};
return (
<ListBox
type={type}
size={size}
id={id}
tabIndex="0"
aria-label={ariaLabel}
className={className({ isOpen })}
disabled={disabled}
aria-disabled={disabled}
translateWithId={translateWithId}
{...getButtonProps({
onKeyDown: event => {
if (match(event, keys.Enter)) {
toggleMenu();
}
},
disabled,
})}>
<span
className={`${prefix}--list-box__label`}
{...getLabelProps()}>
{selectedItem ? itemToString(selectedItem) : label}
</span>
<ListBox.MenuIcon
isOpen={isOpen}
translateWithId={translateWithId}
/>
</ListBox.Field>
{isOpen && (
<ListBox.Menu aria-labelledby={id} id={id}>
{items.map((item, index) => (
<ListBox.MenuItem
key={itemToString(item)}
isActive={selectedItem === item}
isHighlighted={
highlightedIndex === index || selectedItem === item
}
{...getItemProps({ item, index })}>
{itemToElement ? (
<ItemToElement key={itemToString(item)} {...item} />
) : (
itemToString(item)
)}
</ListBox.MenuItem>
))}
</ListBox.Menu>
)}
</ListBox>
)}
isOpen={isOpen}
invalid={invalid}
invalidText={invalidText}
light={light}
{...getRootProps({ refKey: 'innerRef' })}>
{invalid && (
<WarningFilled16
className={`${prefix}--list-box__invalid-icon`}
/>
)}
<ListBox.Field
id={id}
tabIndex="0"
disabled={disabled}
aria-disabled={disabled}
aria-labelledby={`${labelId} ${fieldLabelId}`}
aria-describedby={helperId}
{...buttonProps}>
<span
id={fieldLabelId}
className={`${prefix}--list-box__label`}
{...getLabelProps()}>
{selectedItem ? itemToString(selectedItem) : label}
</span>
<ListBox.MenuIcon
isOpen={isOpen}
translateWithId={translateWithId}
/>
</ListBox.Field>
{isOpen && (
<ListBox.Menu aria-labelledby={id} id={id}>
{items.map((item, index) => (
<ListBox.MenuItem
key={itemToString(item)}
isActive={selectedItem === item}
isHighlighted={
highlightedIndex === index || selectedItem === item
}
{...getItemProps({ item, index })}>
{itemToElement ? (
<ItemToElement key={itemToString(item)} {...item} />
) : (
itemToString(item)
)}
</ListBox.MenuItem>
))}
</ListBox.Menu>
)}
</ListBox>
);
}}
</Downshift>
</div>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ exports[`Dropdown should render 1`] = `
aria-disabled={false}
aria-expanded={false}
aria-haspopup={true}
aria-label="open menu"
aria-labelledby="dropdown-label-1 dropdown-field-label-1"
data-toggle={true}
disabled={false}
id="test-dropdown"
Expand All @@ -94,15 +94,14 @@ exports[`Dropdown should render 1`] = `
onKeyDown={[Function]}
role="button"
tabIndex="0"
translateWithId={[Function]}
type="button"
>
<div
aria-controls={null}
aria-disabled={false}
aria-expanded={false}
aria-haspopup={true}
aria-label="Open menu"
aria-labelledby="dropdown-label-1 dropdown-field-label-1"
aria-owns={null}
className="bx--list-box__field"
data-toggle={true}
Expand All @@ -116,6 +115,7 @@ exports[`Dropdown should render 1`] = `
<span
className="bx--list-box__label"
htmlFor="downshift-0-input"
id="dropdown-field-label-1"
>
input
</span>
Expand Down Expand Up @@ -260,7 +260,7 @@ exports[`Dropdown should render custom item components 1`] = `
aria-disabled={false}
aria-expanded={true}
aria-haspopup={true}
aria-label="close menu"
aria-labelledby="dropdown-label-5 dropdown-field-label-5"
data-toggle={true}
disabled={false}
id="test-dropdown"
Expand All @@ -269,15 +269,14 @@ exports[`Dropdown should render custom item components 1`] = `
onKeyDown={[Function]}
role="button"
tabIndex="0"
translateWithId={[Function]}
type="button"
>
<div
aria-controls="test-dropdown__menu"
aria-disabled={false}
aria-expanded={true}
aria-haspopup={true}
aria-label="Close menu"
aria-labelledby="dropdown-label-5 dropdown-field-label-5"
aria-owns="test-dropdown__menu"
className="bx--list-box__field"
data-toggle={true}
Expand All @@ -291,6 +290,7 @@ exports[`Dropdown should render custom item components 1`] = `
<span
className="bx--list-box__label"
htmlFor="downshift-4-input"
id="dropdown-field-label-5"
>
input
</span>
Expand Down Expand Up @@ -594,7 +594,7 @@ exports[`Dropdown should render with strings as items 1`] = `
aria-disabled={false}
aria-expanded={true}
aria-haspopup={true}
aria-label="close menu"
aria-labelledby="dropdown-label-4 dropdown-field-label-4"
data-toggle={true}
disabled={false}
id="test-dropdown"
Expand All @@ -603,15 +603,14 @@ exports[`Dropdown should render with strings as items 1`] = `
onKeyDown={[Function]}
role="button"
tabIndex="0"
translateWithId={[Function]}
type="button"
>
<div
aria-controls="test-dropdown__menu"
aria-disabled={false}
aria-expanded={true}
aria-haspopup={true}
aria-label="Close menu"
aria-labelledby="dropdown-label-4 dropdown-field-label-4"
aria-owns="test-dropdown__menu"
className="bx--list-box__field"
data-toggle={true}
Expand All @@ -625,6 +624,7 @@ exports[`Dropdown should render with strings as items 1`] = `
<span
className="bx--list-box__label"
htmlFor="downshift-3-input"
id="dropdown-field-label-4"
>
input
</span>
Expand Down
34 changes: 4 additions & 30 deletions packages/react/src/components/ListBox/ListBoxField.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,37 +11,22 @@ import PropTypes from 'prop-types';

const { prefix } = settings;

export const translationIds = {
'close.menu': 'close.menu',
'open.menu': 'open.menu',
};

const defaultTranslations = {
[translationIds['close.menu']]: 'Close menu',
[translationIds['open.menu']]: 'Open menu',
};
export const translationIds = {}; // No longer used, left export for backward-compatibility

/**
* `ListBoxField` is responsible for creating the containing node for valid
* elements inside of a field. It also provides a11y-related attributes like
* `role` to make sure a user can focus the given field.
*/
const ListBoxField = ({
children,
id,
disabled,
tabIndex,
translateWithId: t,
...rest
}) => (
const ListBoxField = ({ children, id, disabled, tabIndex, ...rest }) => (
<div
role={rest['aria-expanded'] ? 'combobox' : rest.role || 'combobox'}
aria-owns={(rest['aria-expanded'] && `${id}__menu`) || null}
aria-controls={(rest['aria-expanded'] && `${id}__menu`) || null}
aria-haspopup="listbox"
className={`${prefix}--list-box__field`}
tabIndex={(!disabled && tabIndex) || -1}
{...rest}
aria-label={rest['aria-expanded'] ? t('close.menu') : t('open.menu')}>
{...rest}>
{children}
</div>
);
Expand All @@ -66,17 +51,6 @@ ListBoxField.propTypes = {
* Optional prop to specify the tabIndex of the <ListBox> trigger button
*/
tabIndex: PropTypes.oneOfType([PropTypes.number, PropTypes.string]),

/**
* i18n hook used to provide the appropriate description for the given menu
* icon. This function takes in an id defined in `translationIds` and should
* return a string message for that given message id.
*/
translateWithId: PropTypes.func.isRequired,
};

ListBoxField.defaultProps = {
translateWithId: id => defaultTranslations[id],
};

export default ListBoxField;
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ exports[`ListBox should render 1`] = `
tabindex="-1"
>
<div
aria-label="Open menu"
aria-haspopup="listbox"
class="bx--list-box__field"
role="combobox"
tabindex="-1"
Expand Down Expand Up @@ -44,11 +44,10 @@ exports[`ListBox should render 1`] = `
>
<ListBoxField
id="test-listbox"
translateWithId={[Function]}
>
<div
aria-controls={null}
aria-label="Open menu"
aria-haspopup="listbox"
aria-owns={null}
className="bx--list-box__field"
role="combobox"
Expand Down
Loading