From 185d8fed040efa43cdba8f568c691b9d8d03afe8 Mon Sep 17 00:00:00 2001 From: Kyle Alwyn Date: Mon, 18 Mar 2019 18:00:52 -0700 Subject: [PATCH 1/6] fix(Icon): Wrap in forwardRef to be used as Dropdown trigger --- src/Dropdown/Dropdown.js | 6 +++++- src/Dropdown/Dropdown.mdx | 21 ++++++++++++++++++++- src/Icon/Icon.js | 24 ++++++++---------------- 3 files changed, 33 insertions(+), 18 deletions(-) diff --git a/src/Dropdown/Dropdown.js b/src/Dropdown/Dropdown.js index 36678fb..99ac760 100644 --- a/src/Dropdown/Dropdown.js +++ b/src/Dropdown/Dropdown.js @@ -100,7 +100,7 @@ export default function Dropdown({ - {({ ref: triggerRef }) => + {({ ref: triggerRef, style = {} }) => React.cloneElement(trigger, { ref: node => { triggerRef(node); @@ -113,6 +113,10 @@ export default function Dropdown({ 'aria-haspopup': true, 'aria-expanded': isOpen, onClick: handleTrigger, + style: { + cursor: 'pointer', + ...style, + }, }) } diff --git a/src/Dropdown/Dropdown.mdx b/src/Dropdown/Dropdown.mdx index 4e00cbc..eed5b76 100644 --- a/src/Dropdown/Dropdown.mdx +++ b/src/Dropdown/Dropdown.mdx @@ -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'; @@ -26,7 +27,7 @@ Easily display contextual overlays using custom trigger elements. Dropdown's pos <> Placement} value={placement} choices={Object.keys(PLACEMENT_TRANSITION_ORIGINS).map(placement => ({ value: placement, @@ -73,6 +74,24 @@ Easily display contextual overlays using custom trigger elements. Dropdown's pos Footer + +
+ } + > + + + + Section One + Item One + Item Two + + + Footer + +
); diff --git a/src/Icon/Icon.js b/src/Icon/Icon.js index 6de222e..ced6749 100644 --- a/src/Icon/Icon.js +++ b/src/Icon/Icon.js @@ -3,7 +3,7 @@ import Proptypes from 'prop-types'; import { css } from 'styled-components'; import { createComponent } from '../utils'; -const StyledIcon = createComponent({ +const Icon = createComponent({ name: 'Icon', tag: 'i', style: ({ theme, size, color, disabled }) => { @@ -12,7 +12,7 @@ const StyledIcon = createComponent({ return css` color: ${resolvedColor || 'inherit'}; - font-size: ${`${size}px` || 'inherit'}; + font-size: ${size ? `${size}px` : 'inherit'}; ${disabled && css` @@ -23,23 +23,15 @@ 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 ; - } -} - Icon.propTypes = { name: Proptypes.string.isRequired, size: Proptypes.number, color: Proptypes.string, }; -export default Icon; +Icon.iconPrefix = 'mdi'; +Icon.getIconClassName = name => `${Icon.iconPrefix} ${Icon.iconPrefix}-${name}`; + +export default React.forwardRef(({ name, className, ...props }, ref) => ( + +)); From c2459c75b00207cf9f0087eee242ba3258de13be Mon Sep 17 00:00:00 2001 From: Kyle Alwyn Date: Mon, 18 Mar 2019 18:02:41 -0700 Subject: [PATCH 2/6] Update snapshot --- src/Dropdown/Dropdown.js | 4 ++-- src/Dropdown/__snapshots__/Dropdown.spec.js.snap | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Dropdown/Dropdown.js b/src/Dropdown/Dropdown.js index 99ac760..7c0e70b 100644 --- a/src/Dropdown/Dropdown.js +++ b/src/Dropdown/Dropdown.js @@ -100,7 +100,7 @@ export default function Dropdown({ - {({ ref: triggerRef, style = {} }) => + {({ ref: triggerRef }) => React.cloneElement(trigger, { ref: node => { triggerRef(node); @@ -115,7 +115,7 @@ export default function Dropdown({ onClick: handleTrigger, style: { cursor: 'pointer', - ...style, + ...(trigger.style || {}), }, }) } diff --git a/src/Dropdown/__snapshots__/Dropdown.spec.js.snap b/src/Dropdown/__snapshots__/Dropdown.spec.js.snap index 03c480a..da93e80 100644 --- a/src/Dropdown/__snapshots__/Dropdown.spec.js.snap +++ b/src/Dropdown/__snapshots__/Dropdown.spec.js.snap @@ -42,6 +42,7 @@ exports[` only renders trigger on mount 1`] = ` aria-expanded="false" aria-haspopup="true" class="re-button re-button-primary c0" + style="cursor: pointer;" > Trigger @@ -90,6 +91,7 @@ exports[` 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 From 7c0ba63439ba0c3656e3a19ffb0f8b1430cd2f35 Mon Sep 17 00:00:00 2001 From: Kyle Alwyn Date: Mon, 18 Mar 2019 18:09:56 -0700 Subject: [PATCH 3/6] More better --- src/Icon/Icon.js | 18 +++++++++--------- src/utils.js | 17 +++++++++++------ 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/src/Icon/Icon.js b/src/Icon/Icon.js index ced6749..03b00cc 100644 --- a/src/Icon/Icon.js +++ b/src/Icon/Icon.js @@ -1,11 +1,13 @@ -import React from 'react'; -import Proptypes from 'prop-types'; +import PropTypes from 'prop-types'; import { css } from 'styled-components'; import { createComponent } from '../utils'; 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; @@ -24,14 +26,12 @@ const Icon = createComponent({ }); 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.getIconClassName = name => `${Icon.iconPrefix} ${Icon.iconPrefix}-${name}`; +Icon.getClassName = name => `${Icon.iconPrefix} ${Icon.iconPrefix}-${name}`; -export default React.forwardRef(({ name, className, ...props }, ref) => ( - -)); +export default Icon; diff --git a/src/utils.js b/src/utils.js index 2e0b8a6..30b65f8 100644 --- a/src/utils.js +++ b/src/utils.js @@ -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] || {}} From dd20cce9b22be6e051ab41d77b0a48c5dcd424d8 Mon Sep 17 00:00:00 2001 From: Kyle Alwyn Date: Tue, 19 Mar 2019 10:45:43 -0700 Subject: [PATCH 4/6] Support Enter and Space for non-button elements --- src/Dropdown/Dropdown.js | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/Dropdown/Dropdown.js b/src/Dropdown/Dropdown.js index 7c0e70b..3e69954 100644 --- a/src/Dropdown/Dropdown.js +++ b/src/Dropdown/Dropdown.js @@ -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 @@ -110,9 +114,12 @@ 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 || {}), @@ -144,7 +151,7 @@ export default function Dropdown({ {({ ref, style }) => ( {state => ( - + { menuRef.current = menuInner; From 141f192538c334665e1b44b345ed752676ecea77 Mon Sep 17 00:00:00 2001 From: Kyle Alwyn Date: Tue, 19 Mar 2019 11:12:45 -0700 Subject: [PATCH 5/6] Wrap cloned trigger in element we control to get determinstic ref --- src/Dropdown/Dropdown.js | 47 +++++++++++++++++++++------------------- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/src/Dropdown/Dropdown.js b/src/Dropdown/Dropdown.js index 3e69954..e9fdb20 100644 --- a/src/Dropdown/Dropdown.js +++ b/src/Dropdown/Dropdown.js @@ -28,6 +28,15 @@ export const PLACEMENT_TRANSITION_ORIGINS = { const ARROW_KEYS = ['ArrowUp', 'ArrowDown']; +const TriggerWrapper = createComponent({ + name: 'DropdownTrigger', + style: css` + display: inline-block; + align-self: flex-start; + outline: none; + `, +}); + export default function Dropdown({ autoclose, placement, @@ -104,28 +113,22 @@ export default function Dropdown({ - {({ ref: triggerRef }) => - React.cloneElement(trigger, { - ref: node => { - triggerRef(node); - - const { ref } = trigger; - if (typeof ref === 'function') { - 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 || {}), - }, - }) - } + {({ ref: triggerRef }) => ( + + {React.cloneElement(trigger, { + role: trigger.role || 'button', + tabIndex: trigger.tabIndex || 0, + 'aria-haspopup': true, + 'aria-expanded': isOpen, + onClick: handleTrigger, + onKeyPress: handleTrigger, + style: { + cursor: 'pointer', + ...(trigger.style || {}), + }, + })} + + )} {isOpen && ( From f4a0cacd3aa655fcb55f7dca938afeb64576daaa Mon Sep 17 00:00:00 2001 From: Kyle Alwyn Date: Tue, 19 Mar 2019 11:13:26 -0700 Subject: [PATCH 6/6] Update snapshot --- .../__snapshots__/Dropdown.spec.js.snap | 66 ++++++++++++++----- 1 file changed, 48 insertions(+), 18 deletions(-) diff --git a/src/Dropdown/__snapshots__/Dropdown.spec.js.snap b/src/Dropdown/__snapshots__/Dropdown.spec.js.snap index da93e80..7076de2 100644 --- a/src/Dropdown/__snapshots__/Dropdown.spec.js.snap +++ b/src/Dropdown/__snapshots__/Dropdown.spec.js.snap @@ -3,6 +3,14 @@ exports[` only renders trigger on mount 1`] = ` .c0 { + display: inline-block; + -webkit-align-self: flex-start; + -ms-flex-item-align: start; + align-self: flex-start; + outline: none; +} + +.c1 { font-family: inherit; display: inline-block; text-align: center; @@ -28,30 +36,45 @@ exports[` only renders trigger on mount 1`] = ` transition: 175ms; } -.c0:hover { +.c1:hover { background: #45b4f3; border-color: #45b4f3; } -.c0:active { +.c1:active { background: #51b9f4; border-color: #51b9f4; } - + + `; exports[` opens menu with focus when trigger is clicked 1`] = ` .c0 { + display: inline-block; + -webkit-align-self: flex-start; + -ms-flex-item-align: start; + align-self: flex-start; + outline: none; +} + +.c1 { font-family: inherit; display: inline-block; text-align: center; @@ -77,24 +100,31 @@ exports[` opens menu with focus when trigger is clicked 1`] = ` transition: 175ms; } -.c0:hover { +.c1:hover { background: #45b4f3; border-color: #45b4f3; } -.c0:active { +.c1:active { background: #51b9f4; border-color: #51b9f4; } - + +