From 61ef94ce3264d6252642ffe2cc0d075efbfaac2b Mon Sep 17 00:00:00 2001 From: Nicko Winner-Arroyo Date: Fri, 8 May 2020 14:55:21 -0400 Subject: [PATCH 1/6] Add Modal, and HxDiv elements & Alert event fixes - create useEventListener hook for more terse hook creation - Favoring shorter smaller wrappers with more ...rest passing, relying on prop types for prop documentation, Webstorm seems to handle this less explicit approach well. - Add actions to Alert, and update update the event names we are listening for to be correct - Move constants to a single file. --- src/Alert/index.js | 31 +++++++--------------- src/Alert/stories.js | 3 +++ src/HxDiv/index.js | 24 ++++++++++++++++++ src/Modal/index.js | 32 +++++++++++++++++++++++ src/Modal/stories.js | 48 +++++++++++++++++++++++++++++++++++ src/Tooltip/index.js | 5 ++-- src/Tooltip/positions.js | 16 ------------ src/constants.js | 22 ++++++++++++++++ src/hooks/useEventListener.js | 31 ++++++++++++++++++++++ 9 files changed, 171 insertions(+), 41 deletions(-) create mode 100644 src/HxDiv/index.js create mode 100644 src/Modal/index.js create mode 100644 src/Modal/stories.js delete mode 100644 src/Tooltip/positions.js create mode 100644 src/constants.js create mode 100644 src/hooks/useEventListener.js diff --git a/src/Alert/index.js b/src/Alert/index.js index cdaa0d3..263dd36 100644 --- a/src/Alert/index.js +++ b/src/Alert/index.js @@ -1,18 +1,9 @@ -import React, { useRef, useEffect} from 'react'; +import React from 'react'; import PropTypes from 'prop-types'; +import useEventListener from '../hooks/useEventListener'; -const Alert = (props) => { - const hxRef = useRef(null); - - useEffect(() => { - hxRef.current.addEventListener('open', props.onOpen); - hxRef.current.addEventListener('close', props.onClose); - return () => { - hxRef.current.removeEventListener('open', props.onOpen); - hxRef.current.removeEventListener('close', props.onClose); - }; - }, []); - +const Alert = ({onOpen, onClose, className, children, onDismiss, onSubmit, ...rest }) => { + const hxRef = useEventListener({ onDismiss, onSubmit }); return ( <> {/* @@ -20,15 +11,11 @@ const Alert = (props) => { about where highest level parent element went, and will throw an error. */} - {props.children} + {children} ); @@ -41,8 +28,8 @@ Alert.propTypes = { status: PropTypes.string, cta: PropTypes.string, persist: PropTypes.bool, - onOpen: PropTypes.func, - onClose: PropTypes.func + onDismiss: PropTypes.func, + onSubmit: PropTypes.func }; export default Alert; diff --git a/src/Alert/stories.js b/src/Alert/stories.js index 8d4c625..abfd5b9 100644 --- a/src/Alert/stories.js +++ b/src/Alert/stories.js @@ -2,6 +2,7 @@ import React from 'react'; import { storiesOf } from '@storybook/react'; import { boolean, select, text } from '@storybook/addon-knobs/react'; import Alert from './index'; +import { action } from '@storybook/addon-actions'; const TYPES = { 'info': 'info', @@ -24,6 +25,8 @@ storiesOf('Alert', module) { ...( status && { status }) } { ...( persist && { persist }) } { ...( type && { type }) } + onDismiss={action('onDismiss')} + onSubmit={action('onSubmit')} >{content} ); }); diff --git a/src/HxDiv/index.js b/src/HxDiv/index.js new file mode 100644 index 0000000..e785e40 --- /dev/null +++ b/src/HxDiv/index.js @@ -0,0 +1,24 @@ +import React from 'react'; +import PropTypes from 'prop-types'; + +const HxDiv = ({ className, ...rest }) => { + return ( + + ); +}; + +HxDiv.propTypes = { + className: PropTypes.string, + children: PropTypes.node.isRequired, + scroll: PropTypes.oneOf([ + 'vertical', + 'horizontal', + 'both', + 'none' + ]), +}; + +export default HxDiv; diff --git a/src/Modal/index.js b/src/Modal/index.js new file mode 100644 index 0000000..1c38675 --- /dev/null +++ b/src/Modal/index.js @@ -0,0 +1,32 @@ +import React from 'react'; +import PropTypes from 'prop-types'; +import classNames from 'classnames'; + +import useEventListener from '../hooks/useEventListener'; +import { SIZES } from '../constants'; + +const Modal = ({ onOpen, onClose, className, open, size, children, ...rest }) => { + const hxRef = useEventListener({ onOpen, onClose }); + return ( + + {children} + + ); +}; + +Modal.propTypes = { + id: PropTypes.string, + size: PropTypes.string, + children: PropTypes.node.isRequired, + className: PropTypes.string, + open: PropTypes.bool, + onClose: PropTypes.func, + onOpen: PropTypes.func, +}; + +export default Modal; diff --git a/src/Modal/stories.js b/src/Modal/stories.js new file mode 100644 index 0000000..7af4ffe --- /dev/null +++ b/src/Modal/stories.js @@ -0,0 +1,48 @@ +import React from 'react'; +import { storiesOf } from '@storybook/react'; +import { action } from '@storybook/addon-actions'; +import { boolean, select, text } from '@storybook/addon-knobs/react'; +import Modal from './index'; +import HxDiv from '../HxDiv'; +import Button from '../Button'; + +const SIZES = { + 'small': 'small', + 'medium': 'medium', + 'large': 'large', +}; + +storiesOf('Modal', module) + .add('All Knobs', () => { + let header = text('header in H3', 'Modal Header'); + let footer = text('footer', ''); + let open = boolean('open', true); + let size = select('size', SIZES, 'medium'); + let scroll = boolean('scroll', false); + + const smallText = 'This is the body of a demo modal. Interaction with content behind this modal cannot take place until this modal is closed.\n'; + const lorumIpsum =

Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Gravida rutrum quisque non tellus. Sagittis vitae et leo duis ut diam quam nulla. Diam vel quam elementum pulvinar etiam non. Pulvinar sapien et ligula ullamcorper malesuada proin libero nunc. Ultricies integer quis auctor elit sed vulputate mi sit amet. Egestas dui id ornare arcu odio ut. In iaculis nunc sed augue. Pellentesque adipiscing commodo elit at imperdiet dui accumsan sit amet. Erat velit scelerisque in dictum non. Auctor augue mauris augue neque gravida in fermentum et. Posuere sollicitudin aliquam ultrices sagittis orci a scelerisque purus. Ullamcorper dignissim cras tincidunt lobortis feugiat vivamus at augue. Tincidunt vitae semper quis lectus nulla. Purus ut faucibus pulvinar elementum integer enim neque volutpat. Etiam sit amet nisl purus in mollis nunc. Diam sit amet nisl suscipit. Nulla pharetra diam sit amet nisl. Arcu odio ut sem nulla.

; + const longText = [1,2,3,4,5].map(() => lorumIpsum); + const defaultFooter = ( + <> + + + + ); + + return ( + + {header &&

{header}

} + + {smallText}{scroll ? longText : null} + + {
{footer || defaultFooter}
} + + ); + }); diff --git a/src/Tooltip/index.js b/src/Tooltip/index.js index b8181b5..85c4b70 100644 --- a/src/Tooltip/index.js +++ b/src/Tooltip/index.js @@ -1,7 +1,6 @@ import PropTypes from 'prop-types'; import React from 'react'; - -import positions from './positions.js'; +import { POSITIONS } from '../constants'; const Tooltip = ({ children, id, position }) => { return ( @@ -14,7 +13,7 @@ const Tooltip = ({ children, id, position }) => { Tooltip.propTypes = { children: PropTypes.node.isRequired, id: PropTypes.string.isRequired, - position: PropTypes.oneOf(positions), + position: PropTypes.oneOf(POSITIONS), }; Tooltip.defaultProps = { diff --git a/src/Tooltip/positions.js b/src/Tooltip/positions.js deleted file mode 100644 index 3ff927e..0000000 --- a/src/Tooltip/positions.js +++ /dev/null @@ -1,16 +0,0 @@ -const positions = [ - 'top-left', - 'top-center', - 'top-right', - 'right-top', - 'right-middle', - 'right-bottom', - 'bottom-right', - 'bottom-center', - 'bottom-left', - 'left-bottom', - 'left-middle', - 'left-top', -]; - -export default positions; diff --git a/src/constants.js b/src/constants.js new file mode 100644 index 0000000..6233d0a --- /dev/null +++ b/src/constants.js @@ -0,0 +1,22 @@ +export const SIZES = { + small: 'hxSm', + medium: 'hxMd', + large: 'hxLg' +}; + +export const POSITIONS = [ + 'top-left', + 'top-center', + 'top-right', + 'right-top', + 'right-middle', + 'right-bottom', + 'bottom-right', + 'bottom-center', + 'bottom-left', + 'left-bottom', + 'left-middle', + 'left-top', +]; + + diff --git a/src/hooks/useEventListener.js b/src/hooks/useEventListener.js new file mode 100644 index 0000000..3b0d55f --- /dev/null +++ b/src/hooks/useEventListener.js @@ -0,0 +1,31 @@ +import { useEffect, useRef } from 'react'; + +const handlerNameToEvent = (handlerName) => handlerName.replace(/^on/,'').toLowerCase(); + +/** + * maps event handlers like onOpen or onClose to event listeners: 'open' or 'close' + * @param {object} eventHandlers such as onClose, onOpen, ...etc + * @param {React.MutableRefObject} ref + * @return {React.MutableRefObject} + */ +function useEventListener(eventHandlers = {}, ref) { + const theRef = ref || useRef(null); + useEffect(() => { + Object.entries(eventHandlers).forEach(([handlerName, eventHandler] , key) => { + theRef.current.addEventListener( + handlerNameToEvent(handlerName), + eventHandler + ); + }); + return () => { + Object.entries(eventHandlers).forEach(([handlerName, eventHandler] , key) => { + theRef.current.addEventListener( + handlerNameToEvent(handlerName), + eventHandler + ); + }); + }; + }, []); + return theRef; +} +export default useEventListener; From 630e352da9ac2d8ab6bb6491b223eae6be638df8 Mon Sep 17 00:00:00 2001 From: Nicko Winner-Arroyo Date: Fri, 8 May 2020 15:26:18 -0400 Subject: [PATCH 2/6] spacing changes --- src/Alert/index.js | 40 +++++++--------- src/Alert/stories.js | 45 +++++++++--------- src/HxDiv/index.js | 18 ++----- src/Modal/index.js | 36 +++++++------- src/Modal/stories.js | 89 +++++++++++++++++++++-------------- src/Select/stories.js | 6 +-- src/constants.js | 32 ++++++------- src/hooks/useEventListener.js | 32 +++++-------- 8 files changed, 146 insertions(+), 152 deletions(-) diff --git a/src/Alert/index.js b/src/Alert/index.js index 263dd36..8e8a6f9 100644 --- a/src/Alert/index.js +++ b/src/Alert/index.js @@ -2,34 +2,30 @@ import React from 'react'; import PropTypes from 'prop-types'; import useEventListener from '../hooks/useEventListener'; -const Alert = ({onOpen, onClose, className, children, onDismiss, onSubmit, ...rest }) => { - const hxRef = useEventListener({ onDismiss, onSubmit }); - return ( - <> - {/* +const Alert = ({ onOpen, onClose, className, children, onDismiss, onSubmit, ...rest }) => { + const hxRef = useEventListener({ onDismiss, onSubmit }); + return ( + <> + {/* Wrappping element needed: Otherwise when alert removes itself from DOM on close it confusing React about where highest level parent element went, and will throw an error. */} - - {children} - - - ); + + {children} + + + ); }; Alert.propTypes = { - className: PropTypes.string, - children: PropTypes.node.isRequired, - type: PropTypes.string, - status: PropTypes.string, - cta: PropTypes.string, - persist: PropTypes.bool, - onDismiss: PropTypes.func, - onSubmit: PropTypes.func + className: PropTypes.string, + children: PropTypes.node.isRequired, + type: PropTypes.string, + status: PropTypes.string, + cta: PropTypes.string, + persist: PropTypes.bool, + onDismiss: PropTypes.func, + onSubmit: PropTypes.func, }; export default Alert; diff --git a/src/Alert/stories.js b/src/Alert/stories.js index abfd5b9..e852182 100644 --- a/src/Alert/stories.js +++ b/src/Alert/stories.js @@ -5,28 +5,29 @@ import Alert from './index'; import { action } from '@storybook/addon-actions'; const TYPES = { - 'info': 'info', - 'error': 'error', - 'success': 'success', - 'warning': 'warning' + info: 'info', + error: 'error', + success: 'success', + warning: 'warning', }; -storiesOf('Alert', module) - .add('All Knobs', () => { - let content = text('content', 'Nope! Nope! Nope! Nope! Nope!'); - let cta = text('cta', 'burn it'); - let status = text('status', 'spider'); - let persist = boolean('persist', false); - let type = select('type', TYPES, ''); +storiesOf('Alert', module).add('All Knobs', () => { + let content = text('content', 'Nope! Nope! Nope! Nope! Nope!'); + let cta = text('cta', 'burn it'); + let status = text('status', 'spider'); + let persist = boolean('persist', false); + let type = select('type', TYPES, ''); - return ( - {content} - ); - }); + return ( + + {content} + + ); +}); diff --git a/src/HxDiv/index.js b/src/HxDiv/index.js index e785e40..28b584f 100644 --- a/src/HxDiv/index.js +++ b/src/HxDiv/index.js @@ -2,23 +2,13 @@ import React from 'react'; import PropTypes from 'prop-types'; const HxDiv = ({ className, ...rest }) => { - return ( - - ); + return ; }; HxDiv.propTypes = { - className: PropTypes.string, - children: PropTypes.node.isRequired, - scroll: PropTypes.oneOf([ - 'vertical', - 'horizontal', - 'both', - 'none' - ]), + className: PropTypes.string, + children: PropTypes.node.isRequired, + scroll: PropTypes.oneOf(['vertical', 'horizontal', 'both', 'none']), }; export default HxDiv; diff --git a/src/Modal/index.js b/src/Modal/index.js index 1c38675..78a2b93 100644 --- a/src/Modal/index.js +++ b/src/Modal/index.js @@ -6,27 +6,27 @@ import useEventListener from '../hooks/useEventListener'; import { SIZES } from '../constants'; const Modal = ({ onOpen, onClose, className, open, size, children, ...rest }) => { - const hxRef = useEventListener({ onOpen, onClose }); - return ( - - {children} - - ); + const hxRef = useEventListener({ onOpen, onClose }); + return ( + + {children} + + ); }; Modal.propTypes = { - id: PropTypes.string, - size: PropTypes.string, - children: PropTypes.node.isRequired, - className: PropTypes.string, - open: PropTypes.bool, - onClose: PropTypes.func, - onOpen: PropTypes.func, + id: PropTypes.string, + size: PropTypes.oneOf(SIZES), + children: PropTypes.node.isRequired, + className: PropTypes.string, + open: PropTypes.bool, + onClose: PropTypes.func, + onOpen: PropTypes.func, }; export default Modal; diff --git a/src/Modal/stories.js b/src/Modal/stories.js index 7af4ffe..8e57910 100644 --- a/src/Modal/stories.js +++ b/src/Modal/stories.js @@ -7,42 +7,61 @@ import HxDiv from '../HxDiv'; import Button from '../Button'; const SIZES = { - 'small': 'small', - 'medium': 'medium', - 'large': 'large', + small: 'small', + medium: 'medium', + large: 'large', }; -storiesOf('Modal', module) - .add('All Knobs', () => { - let header = text('header in H3', 'Modal Header'); - let footer = text('footer', ''); - let open = boolean('open', true); - let size = select('size', SIZES, 'medium'); - let scroll = boolean('scroll', false); +storiesOf('Modal', module).add('All Knobs', () => { + let header = text('header in H3', 'Modal Header'); + let footer = text('footer', ''); + let open = boolean('open', true); + let size = select('size', SIZES, 'medium'); + let scroll = boolean('scroll', false); - const smallText = 'This is the body of a demo modal. Interaction with content behind this modal cannot take place until this modal is closed.\n'; - const lorumIpsum =

Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Gravida rutrum quisque non tellus. Sagittis vitae et leo duis ut diam quam nulla. Diam vel quam elementum pulvinar etiam non. Pulvinar sapien et ligula ullamcorper malesuada proin libero nunc. Ultricies integer quis auctor elit sed vulputate mi sit amet. Egestas dui id ornare arcu odio ut. In iaculis nunc sed augue. Pellentesque adipiscing commodo elit at imperdiet dui accumsan sit amet. Erat velit scelerisque in dictum non. Auctor augue mauris augue neque gravida in fermentum et. Posuere sollicitudin aliquam ultrices sagittis orci a scelerisque purus. Ullamcorper dignissim cras tincidunt lobortis feugiat vivamus at augue. Tincidunt vitae semper quis lectus nulla. Purus ut faucibus pulvinar elementum integer enim neque volutpat. Etiam sit amet nisl purus in mollis nunc. Diam sit amet nisl suscipit. Nulla pharetra diam sit amet nisl. Arcu odio ut sem nulla.

; - const longText = [1,2,3,4,5].map(() => lorumIpsum); - const defaultFooter = ( - <> - - - - ); + const smallText = + 'This is the body of a demo modal. Interaction with content behind this modal cannot take place until this modal is closed.\n'; + const lorumIpsum = ( +

+ Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut + labore et dolore magna aliqua. Gravida rutrum quisque non tellus. Sagittis vitae et leo duis + ut diam quam nulla. Diam vel quam elementum pulvinar etiam non. Pulvinar sapien et ligula + ullamcorper malesuada proin libero nunc. Ultricies integer quis auctor elit sed vulputate mi + sit amet. Egestas dui id ornare arcu odio ut. In iaculis nunc sed augue. Pellentesque + adipiscing commodo elit at imperdiet dui accumsan sit amet. Erat velit scelerisque in dictum + non. Auctor augue mauris augue neque gravida in fermentum et. Posuere sollicitudin aliquam + ultrices sagittis orci a scelerisque purus. Ullamcorper dignissim cras tincidunt lobortis + feugiat vivamus at augue. Tincidunt vitae semper quis lectus nulla. Purus ut faucibus pulvinar + elementum integer enim neque volutpat. Etiam sit amet nisl purus in mollis nunc. Diam sit amet + nisl suscipit. Nulla pharetra diam sit amet nisl. Arcu odio ut sem nulla. +

+ ); + const longText = [1, 2, 3, 4, 5].map(() => lorumIpsum); + const defaultFooter = ( + <> + + + + ); - return ( - - {header &&

{header}

} - - {smallText}{scroll ? longText : null} - - {
{footer || defaultFooter}
} - - ); - }); + return ( + + {header && ( +
+

{header}

+
+ )} + + {smallText} + {scroll ? longText : null} + + {
{footer || defaultFooter}
} + + ); +}); diff --git a/src/Select/stories.js b/src/Select/stories.js index 10caca2..890f58d 100644 --- a/src/Select/stories.js +++ b/src/Select/stories.js @@ -27,11 +27,7 @@ storiesOf('Select', module) const Demo = (props) => { return (
-
diff --git a/src/constants.js b/src/constants.js index 6233d0a..7aa835d 100644 --- a/src/constants.js +++ b/src/constants.js @@ -1,22 +1,20 @@ export const SIZES = { - small: 'hxSm', - medium: 'hxMd', - large: 'hxLg' + small: 'hxSm', + medium: 'hxMd', + large: 'hxLg', }; export const POSITIONS = [ - 'top-left', - 'top-center', - 'top-right', - 'right-top', - 'right-middle', - 'right-bottom', - 'bottom-right', - 'bottom-center', - 'bottom-left', - 'left-bottom', - 'left-middle', - 'left-top', + 'top-left', + 'top-center', + 'top-right', + 'right-top', + 'right-middle', + 'right-bottom', + 'bottom-right', + 'bottom-center', + 'bottom-left', + 'left-bottom', + 'left-middle', + 'left-top', ]; - - diff --git a/src/hooks/useEventListener.js b/src/hooks/useEventListener.js index 3b0d55f..e20a1da 100644 --- a/src/hooks/useEventListener.js +++ b/src/hooks/useEventListener.js @@ -1,6 +1,6 @@ import { useEffect, useRef } from 'react'; -const handlerNameToEvent = (handlerName) => handlerName.replace(/^on/,'').toLowerCase(); +const handlerNameToEvent = (handlerName) => handlerName.replace(/^on/, '').toLowerCase(); /** * maps event handlers like onOpen or onClose to event listeners: 'open' or 'close' @@ -9,23 +9,17 @@ const handlerNameToEvent = (handlerName) => handlerName.replace(/^on/,'').toLowe * @return {React.MutableRefObject} */ function useEventListener(eventHandlers = {}, ref) { - const theRef = ref || useRef(null); - useEffect(() => { - Object.entries(eventHandlers).forEach(([handlerName, eventHandler] , key) => { - theRef.current.addEventListener( - handlerNameToEvent(handlerName), - eventHandler - ); - }); - return () => { - Object.entries(eventHandlers).forEach(([handlerName, eventHandler] , key) => { - theRef.current.addEventListener( - handlerNameToEvent(handlerName), - eventHandler - ); - }); - }; - }, []); - return theRef; + const theRef = ref || useRef(null); + useEffect(() => { + Object.entries(eventHandlers).forEach(([handlerName, eventHandler], key) => { + theRef.current.addEventListener(handlerNameToEvent(handlerName), eventHandler); + }); + return () => { + Object.entries(eventHandlers).forEach(([handlerName, eventHandler], key) => { + theRef.current.addEventListener(handlerNameToEvent(handlerName), eventHandler); + }); + }; + }, []); + return theRef; } export default useEventListener; From c42b4de6d607e66209e9258c273be7288e4939ac Mon Sep 17 00:00:00 2001 From: Nicko Winner-Arroyo Date: Fri, 8 May 2020 15:36:56 -0400 Subject: [PATCH 3/6] fix prettier --- src/Alert/index.js | 5 +---- src/Modal/index.js | 1 - 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/Alert/index.js b/src/Alert/index.js index 8e8a6f9..98c709d 100644 --- a/src/Alert/index.js +++ b/src/Alert/index.js @@ -6,10 +6,7 @@ const Alert = ({ onOpen, onClose, className, children, onDismiss, onSubmit, ...r const hxRef = useEventListener({ onDismiss, onSubmit }); return ( <> - {/* - Wrappping element needed: Otherwise when alert removes itself from DOM on close it confusing React - about where highest level parent element went, and will throw an error. - */} + {/* Wrappping element needed: Otherwise when alert removes itself from DOM on close, it will cause error */} {children} diff --git a/src/Modal/index.js b/src/Modal/index.js index 78a2b93..5e3c514 100644 --- a/src/Modal/index.js +++ b/src/Modal/index.js @@ -1,7 +1,6 @@ import React from 'react'; import PropTypes from 'prop-types'; import classNames from 'classnames'; - import useEventListener from '../hooks/useEventListener'; import { SIZES } from '../constants'; From 622bc13b2c715efcf540fe4a7710207ea446ddcc Mon Sep 17 00:00:00 2001 From: Nicko Winner-Arroyo Date: Fri, 8 May 2020 17:48:10 -0400 Subject: [PATCH 4/6] Convert icon to a functional component --- src/Icon/index.js | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/Icon/index.js b/src/Icon/index.js index 14370e0..9d166a3 100644 --- a/src/Icon/index.js +++ b/src/Icon/index.js @@ -4,15 +4,10 @@ import PropTypes from 'prop-types'; export const Icons = Object.keys(HelixUI.Utils.ICONS); -class Icon extends React.Component { - render() { - const { type, ...props } = this.props; - - return ; - } -} +const Icon = ({ type, ...rest }) => { + return ; +}; -// additional configs here Icon.propTypes = { type: PropTypes.string, }; From b64acfe8000524f86f4c8d10d039af8eabb5216a Mon Sep 17 00:00:00 2001 From: Nicko Winner-Arroyo Date: Fri, 8 May 2020 17:51:54 -0400 Subject: [PATCH 5/6] Add Everything to default exports --- src/Icon/index.js | 2 +- src/index.mjs | 12 +++++++++--- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/Icon/index.js b/src/Icon/index.js index 9d166a3..1e469a2 100644 --- a/src/Icon/index.js +++ b/src/Icon/index.js @@ -5,7 +5,7 @@ import PropTypes from 'prop-types'; export const Icons = Object.keys(HelixUI.Utils.ICONS); const Icon = ({ type, ...rest }) => { - return ; + return ; }; Icon.propTypes = { diff --git a/src/index.mjs b/src/index.mjs index ea17a6c..9043e83 100644 --- a/src/index.mjs +++ b/src/index.mjs @@ -2,9 +2,15 @@ import Button from './Button'; import Alert from './Alert'; import Icon from './Icon'; +import Tooltip from './Tooltip'; +import Modal from './Modal'; +import Select from './Select'; export default { - Button, - Icon, - Alert + Button, + Alert, + Icon, + Modal, + Select, + Tooltip }; From 0f69a974b4e7cfb915891d720e50970ff989be0b Mon Sep 17 00:00:00 2001 From: Nicko Winner-Arroyo Date: Fri, 15 May 2020 13:57:50 -0400 Subject: [PATCH 6/6] Code review --- src/Icon/index.js | 5 +++-- src/Modal/index.js | 3 ++- src/hooks/useEventListener.js | 4 ++-- src/utils.js | 7 +++++++ 4 files changed, 14 insertions(+), 5 deletions(-) create mode 100644 src/utils.js diff --git a/src/Icon/index.js b/src/Icon/index.js index 1e469a2..9e2baac 100644 --- a/src/Icon/index.js +++ b/src/Icon/index.js @@ -4,12 +4,13 @@ import PropTypes from 'prop-types'; export const Icons = Object.keys(HelixUI.Utils.ICONS); -const Icon = ({ type, ...rest }) => { - return ; +const Icon = ({ type, className, ...rest }) => { + return ; }; Icon.propTypes = { type: PropTypes.string, + className: PropTypes.string, }; export default Icon; diff --git a/src/Modal/index.js b/src/Modal/index.js index 5e3c514..d8c6b32 100644 --- a/src/Modal/index.js +++ b/src/Modal/index.js @@ -3,6 +3,7 @@ import PropTypes from 'prop-types'; import classNames from 'classnames'; import useEventListener from '../hooks/useEventListener'; import { SIZES } from '../constants'; +import { wcBool } from '../utils'; const Modal = ({ onOpen, onClose, className, open, size, children, ...rest }) => { const hxRef = useEventListener({ onOpen, onClose }); @@ -10,7 +11,7 @@ const Modal = ({ onOpen, onClose, className, open, size, children, ...rest }) => {children} diff --git a/src/hooks/useEventListener.js b/src/hooks/useEventListener.js index e20a1da..8a387f0 100644 --- a/src/hooks/useEventListener.js +++ b/src/hooks/useEventListener.js @@ -3,7 +3,7 @@ import { useEffect, useRef } from 'react'; const handlerNameToEvent = (handlerName) => handlerName.replace(/^on/, '').toLowerCase(); /** - * maps event handlers like onOpen or onClose to event listeners: 'open' or 'close' + * Adds event handlers like onOpen or onClose to event listeners: 'open' or 'close' * @param {object} eventHandlers such as onClose, onOpen, ...etc * @param {React.MutableRefObject} ref * @return {React.MutableRefObject} @@ -16,7 +16,7 @@ function useEventListener(eventHandlers = {}, ref) { }); return () => { Object.entries(eventHandlers).forEach(([handlerName, eventHandler], key) => { - theRef.current.addEventListener(handlerNameToEvent(handlerName), eventHandler); + theRef.current.removeEventListener(handlerNameToEvent(handlerName), eventHandler); }); }; }, []); diff --git a/src/utils.js b/src/utils.js new file mode 100644 index 0000000..d04106a --- /dev/null +++ b/src/utils.js @@ -0,0 +1,7 @@ +/** + * Because Web components attributes convert booleans to strings + * this stops values like false from being converted to "false" and then evaluating as true. + * @param {boolean} bool + * @return {*} + */ +export const wcBool = (bool) => bool ? true : null;