Skip to content
This repository has been archived by the owner on Oct 6, 2020. It is now read-only.

fix(Icon): Wrap in forwardRef to be used as Dropdown trigger #31

Merged
merged 6 commits into from
Mar 19, 2019
Merged
Show file tree
Hide file tree
Changes from 4 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
17 changes: 14 additions & 3 deletions src/Dropdown/Dropdown.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,12 @@ export default function Dropdown({
const toggle = () => (isOpen ? close() : open());

const handleTrigger = e => {
e.stopPropagation();
toggle();
// Allow all clicks and, for non-button elements, Enter and Space to toggle Dropdown
// https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/button_role#Required_JavaScript_Features
if (e.type === 'click' || (e.type === 'keypress' && (e.which === 13 || e.which === 32))) {
e.stopPropagation();
toggle();
}
};

// Wait for next tick after open to prevent page jump when focusing
Expand Down Expand Up @@ -110,9 +114,16 @@ export default function Dropdown({
ref(node);
}
},
role: trigger.role || 'button',
tabIndex: trigger.tabIndex || 0,
'aria-haspopup': true,
'aria-expanded': isOpen,
onClick: handleTrigger,
onKeyPress: handleTrigger,
style: {
cursor: 'pointer',
...(trigger.style || {}),
},
})
}
</Reference>
Expand Down Expand Up @@ -140,7 +151,7 @@ export default function Dropdown({
{({ ref, style }) => (
<Transition in={isOpen} timeout={0} appear>
{state => (
<FocusTrap autoFocus={false}>
<FocusTrap returnFocus autoFocus={false}>
<DropdownMenu
ref={menuInner => {
menuRef.current = menuInner;
Expand Down
21 changes: 20 additions & 1 deletion src/Dropdown/Dropdown.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ name: Dropdown
import { useState } from 'react';
import { Playground, PropsTable } from 'docz';
import Dropdown, { PLACEMENT_TRANSITION_ORIGINS } from './Dropdown';
import Icon from '../Icon';
import RadioGroup from '../Form/RadioGroup';
import Button from '../Button';

Expand All @@ -26,7 +27,7 @@ Easily display contextual overlays using custom trigger elements. Dropdown's pos
<>
<Button.Group>
<RadioGroup
label="Placement"
label={<strong>Placement</strong>}
value={placement}
choices={Object.keys(PLACEMENT_TRANSITION_ORIGINS).map(placement => ({
value: placement,
Expand Down Expand Up @@ -73,6 +74,24 @@ Easily display contextual overlays using custom trigger elements. Dropdown's pos

<Dropdown.Footer>Footer</Dropdown.Footer>
</Dropdown>

<div>
<Dropdown
width={250}
placement="top"
trigger={<Icon name="information-outline" />}
Copy link
Contributor

Choose a reason for hiding this comment

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

In order to make icon buttons accessible, we can wrap the icon in a div with role=Button and tabIndex=0. https://louiseclark.tech/tech-blog/accessibility-journey-making-buttons-accessible

>
<Dropdown.Header title="Dropdown" />

<Dropdown.Body>
<Dropdown.SectionTitle>Section One</Dropdown.SectionTitle>
<Dropdown.Item>Item One</Dropdown.Item>
<Dropdown.Item>Item Two</Dropdown.Item>
</Dropdown.Body>

<Dropdown.Footer>Footer</Dropdown.Footer>
</Dropdown>
</div>
</Button.Group>
</>
);
Expand Down
2 changes: 2 additions & 0 deletions src/Dropdown/__snapshots__/Dropdown.spec.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ exports[`<Dropdown /> only renders trigger on mount 1`] = `
aria-expanded="false"
aria-haspopup="true"
class="re-button re-button-primary c0"
style="cursor: pointer;"
>
Trigger
</button>
Expand Down Expand Up @@ -90,6 +91,7 @@ exports[`<Dropdown /> opens menu with focus when trigger is clicked 1`] = `
aria-expanded="true"
aria-haspopup="true"
class="re-button re-button-primary c0"
style="cursor: pointer;"
>
Trigger
</button>
Expand Down
32 changes: 12 additions & 20 deletions src/Icon/Icon.js
Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@
import React from 'react';
import Proptypes from 'prop-types';
import PropTypes from 'prop-types';
import { css } from 'styled-components';
import { createComponent } from '../utils';

const StyledIcon = createComponent({
const Icon = createComponent({
name: 'Icon',
tag: 'i',
props: ({ name, className }) => ({
className: `${Icon.getClassName(name)} ${className || ''}`,
}),
style: ({ theme, size, color, disabled }) => {
const colorFromTheme = theme.colors[color];
const resolvedColor = colorFromTheme || color;

return css`
color: ${resolvedColor || 'inherit'};
font-size: ${`${size}px` || 'inherit'};
font-size: ${size ? `${size}px` : 'inherit'};

${disabled &&
css`
Expand All @@ -23,23 +25,13 @@ const StyledIcon = createComponent({
},
});

class Icon extends React.Component {
static iconPrefix = 'mdi';
static getIconClassName(name) {
return `${this.iconPrefix} ${this.iconPrefix}-${name}`;
}

render() {
const { name, className = '', ...props } = this.props;

return <StyledIcon {...props} className={`${this.constructor.getIconClassName(name)} ${className}`} />;
}
}

Icon.propTypes = {
name: Proptypes.string.isRequired,
size: Proptypes.number,
color: Proptypes.string,
name: PropTypes.string.isRequired,
size: PropTypes.number,
color: PropTypes.string,
};

Icon.iconPrefix = 'mdi';
Icon.getClassName = name => `${Icon.iconPrefix} ${Icon.iconPrefix}-${name}`;

export default Icon;
17 changes: 11 additions & 6 deletions src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,19 @@ export const getComponentStyle = componentName => themeGet(`components.${compone
export const getComponentClassName = ({ className, theme: { classPrefix }, variant }, name) =>
`${className || ''} ${classPrefix}-${name} ${variant ? `${classPrefix}-${name}-${variant}` : ''}`.trim();

export const createComponent = ({ name, tag = 'div', as, style, props: defaultProps = () => ({}) }) => {
export const createComponent = ({ name, tag = 'div', as, style, props: baseProps = () => ({}) }) => {
const component = as ? styled(as) : styled[tag];

return component.attrs(props => ({
...defaultProps(props),
...props,
className: getComponentClassName(props, kebabCase(name)),
}))`
return component.attrs(props => {
const resolvedProps = {
...baseProps(props),
...props,
};
return {
...resolvedProps,
className: getComponentClassName(resolvedProps, kebabCase(name)),
};
})`
${style}
${getComponentStyle(name)}
${({ styles = {} }) => styles[name] || {}}
Expand Down