From b4e89d35b1d4d58fc403530e17e9d242cc03bb28 Mon Sep 17 00:00:00 2001 From: Diego Fortes Date: Tue, 27 Oct 2020 10:47:57 -0300 Subject: [PATCH] [Cleanup] - Part 2 (#461) * make button onCloick requried proptypes * carousel callout * dialogModal and rest param types * convert offclick wrapper * feedback changes --- src/shared-components/accordion/index.tsx | 4 +- src/shared-components/alert/index.tsx | 4 +- src/shared-components/button/index.tsx | 2 +- src/shared-components/callout/index.tsx | 4 +- .../carousel/arrow/index.tsx | 98 +++---- src/shared-components/carousel/index.tsx | 268 +++++++----------- src/shared-components/checkbox/index.tsx | 2 +- src/shared-components/chip/index.tsx | 4 - src/shared-components/dialogModal/index.tsx | 174 +++++------- .../offClickWrapper/index.tsx | 80 ++---- src/shared-components/progressBar/index.tsx | 2 +- src/shared-components/radioButton/index.tsx | 2 +- .../selectorButton/index.tsx | 2 +- src/utils/helperTransition/index.tsx | 2 +- 14 files changed, 264 insertions(+), 384 deletions(-) diff --git a/src/shared-components/accordion/index.tsx b/src/shared-components/accordion/index.tsx index 9635f8573..77a3f57ed 100644 --- a/src/shared-components/accordion/index.tsx +++ b/src/shared-components/accordion/index.tsx @@ -1,5 +1,5 @@ import PropTypes from 'prop-types'; -import React, { useEffect, useState } from 'react'; +import React, { useEffect, useState, useRef } from 'react'; import ChevronIcon from 'src/svgs/icons/chevron-icon.svg'; import { COLORS } from 'src/constants'; @@ -52,7 +52,7 @@ export const Accordion = ({ }: AccordionProps) => { const [contentHeight, setContentHeight] = useState('0px'); - const contentRef = React.createRef(); + const contentRef = useRef(null); useEffect(() => { const nextHeight = diff --git a/src/shared-components/alert/index.tsx b/src/shared-components/alert/index.tsx index f9f118109..b5247a5ad 100644 --- a/src/shared-components/alert/index.tsx +++ b/src/shared-components/alert/index.tsx @@ -1,4 +1,4 @@ -import React, { useState, useEffect } from 'react'; +import React, { useState, useEffect, useRef } from 'react'; import PropTypes from 'prop-types'; import { Avatar } from '../avatar'; @@ -71,7 +71,7 @@ export const Alert = (alertProps: AlertProps) => { const [exiting, setExiting] = useState(false); const [exited, setExited] = useState(false); - const contentText = React.createRef(); + const contentText = useRef(null); let timer: number | undefined; diff --git a/src/shared-components/button/index.tsx b/src/shared-components/button/index.tsx index ff01fee50..d410accf5 100644 --- a/src/shared-components/button/index.tsx +++ b/src/shared-components/button/index.tsx @@ -130,7 +130,7 @@ Button.propTypes = { isFullWidth: PropTypes.bool, isLoading: PropTypes.bool, loading: isLoadingPropFunction, - onClick: PropTypes.func, + onClick: PropTypes.func.isRequired, textColor: PropTypes.string, }; diff --git a/src/shared-components/callout/index.tsx b/src/shared-components/callout/index.tsx index b9ad83bee..fb498e92c 100644 --- a/src/shared-components/callout/index.tsx +++ b/src/shared-components/callout/index.tsx @@ -30,8 +30,8 @@ type CalloutProps = { */ export const Callout = ({ children, - icon = null, color = COLORS.primary, + icon = null, }: CalloutProps) => ( {children} @@ -41,8 +41,8 @@ export const Callout = ({ Callout.propTypes = { children: PropTypes.node.isRequired, - icon: PropTypes.node, color: PropTypes.string, + icon: PropTypes.node, }; Callout.Container = ParentContainer; diff --git a/src/shared-components/carousel/arrow/index.tsx b/src/shared-components/carousel/arrow/index.tsx index 1ff5aec47..bb8a167fe 100644 --- a/src/shared-components/carousel/arrow/index.tsx +++ b/src/shared-components/carousel/arrow/index.tsx @@ -8,65 +8,51 @@ import { ArrowContainer, BottomRightAlignedArrowContainer } from './style'; type ArrowProps = { bottomRightAlignedArrows?: boolean; - prev?: boolean; - next?: boolean; disabled?: boolean; - onClick: () => void; + next?: boolean; + onClick?: () => void; + prev?: boolean; }; -class Arrow extends React.Component { - static propTypes = { - bottomRightAlignedArrows: PropTypes.bool, - prev: PropTypes.bool, - next: PropTypes.bool, - disabled: PropTypes.bool, - onClick: PropTypes.func.isRequired, - }; - - static defaultProps = { - bottomRightAlignedArrows: false, - prev: false, - next: false, - disabled: false, - }; - - arrowClickHandler = () => { - const { onClick } = this.props; - onClick(); - }; - - render() { - const { - bottomRightAlignedArrows = false, - prev = false, - next = false, - disabled = false, - } = this.props; - const ArrowContainerComponent = bottomRightAlignedArrows - ? BottomRightAlignedArrowContainer - : ArrowContainer; +const Arrow = ({ + bottomRightAlignedArrows = false, + disabled = false, + next = false, + onClick = () => undefined, + prev = false, +}: ArrowProps) => { + const ArrowContainerComponent = bottomRightAlignedArrows + ? BottomRightAlignedArrowContainer + : ArrowContainer; + + return ( + + {prev && ( + } + onClick={onClick} + /> + )} + {next && ( + } + onClick={onClick} + /> + )} + + ); +}; - return ( - - {prev && ( - } - onClick={this.arrowClickHandler} - /> - )} - {next && ( - } - onClick={this.arrowClickHandler} - /> - )} - - ); - } -} +Arrow.propTypes = { + bottomRightAlignedArrows: PropTypes.bool, + disabled: PropTypes.bool, + next: PropTypes.bool, + onClick: PropTypes.func, + prev: PropTypes.bool, +}; export default Arrow; diff --git a/src/shared-components/carousel/index.tsx b/src/shared-components/carousel/index.tsx index 8d70b5712..8090fc933 100644 --- a/src/shared-components/carousel/index.tsx +++ b/src/shared-components/carousel/index.tsx @@ -1,10 +1,11 @@ -import React from 'react'; +import React, { useState, useEffect, useRef } from 'react'; import PropTypes from 'prop-types'; import Slider from 'react-slick'; import Arrow from './arrow'; import { OuterContainer, InnerContainer, Card } from './style'; +const FIRST_INDEX = 0; const BASE_SLIDER_CONFIG = { focusOnSelect: true, pauseOnHover: true, @@ -45,11 +46,6 @@ type CarouselProps = { numCardsVisible: 1 | 2 | 3; }; -type CarouselState = { - currentIndex: number; - lastIndex: number; -}; - /** * Carousels should be used to provide valuable information or additional context on a page. One of the best examples of a Carousel is for product recommendations. * @@ -57,183 +53,131 @@ type CarouselState = { * * An array of `Carousel.Card` must be used for the carousel content. It includes the base styles for the Card which may be extended as shown above. */ -export class Carousel extends React.Component { - static Card = Card; - - static propTypes = { - autoplay: PropTypes.bool, - autoplaySpeed: PropTypes.number, - carouselType: PropTypes.oneOf(['primary', 'secondary']), - centerMode: PropTypes.bool, - /** - * Array of `Carousel.Card` - */ - children: PropTypes.arrayOf(PropTypes.node).isRequired, - hideArrows: PropTypes.bool, - hideDots: PropTypes.bool, - bottomRightAlignedArrows: PropTypes.bool, - infinite: PropTypes.bool, - numCardsVisible: PropTypes.number.isRequired, - }; - - static defaultProps = { - autoplay: false, - autoplaySpeed: 5000, - carouselType: 'primary', - centerMode: true, - hideArrows: false, - hideDots: false, - bottomRightAlignedArrows: false, - infinite: false, +export const Carousel = ({ + autoplay = false, + autoplaySpeed = 5000, + bottomRightAlignedArrows = false, + carouselType = 'primary', + centerMode = true, + children, + hideArrows = false, + hideDots = false, + infinite = false, + numCardsVisible, +}: CarouselProps) => { + const getLastIndex = () => { + const numberSlides = children.length; + return numberSlides - numCardsVisible; }; - slider: React.RefObject = React.createRef(); - - timeoutId: number | null = null; - - userHasInteracted = false; - - constructor(props: CarouselProps) { - super(props); - - this.state = { - currentIndex: 0, - lastIndex: this.getLastIndex(), - }; - } - - componentDidUpdate() { - if (this.shouldReplay) { - this.replay(); - } - - this.updateLastIndex(); - } + const [currentIndex, setCurrentIndex] = useState(FIRST_INDEX); + const [lastIndex, setLastIndex] = useState(getLastIndex); - get shouldReplay() { - const { autoplay, infinite } = this.props; - const { currentIndex, lastIndex } = this.state; + const hasUserInteractedRef = useRef(false); + const timeoutIdRef = useRef(null); + const slider = useRef(null); + const shouldReplay = () => { const onLastCard = currentIndex === lastIndex; - return autoplay && !infinite && onLastCard && !this.userHasInteracted; - } + return autoplay && !infinite && onLastCard && !hasUserInteractedRef.current; + }; - getLastIndex() { - const { children, numCardsVisible } = this.props; - const numberSlides = children.length; + const replay = () => { + timeoutIdRef.current = window.setTimeout(() => { + if (slider.current) { + slider.current.slickGoTo(FIRST_INDEX); + } + }, autoplaySpeed); + }; - return numberSlides - numCardsVisible; - } - - getCarouselSettings() { - const { currentIndex, lastIndex } = this.state; - const firstIndex = 0; - - const { - autoplay, - autoplaySpeed, - centerMode, - hideArrows, - hideDots, - bottomRightAlignedArrows, - infinite, - numCardsVisible, - } = this.props; - - const allowCenterMode = numCardsVisible > 1 || infinite; - - const settings = { - ...BASE_SLIDER_CONFIG, - arrows: !hideArrows, - autoplay, - autoplaySpeed, - centerMode: allowCenterMode ? false : centerMode, - dots: !hideDots && !bottomRightAlignedArrows, - infinite, - slidesToShow: numCardsVisible, - beforeChange: this.onCardChange, - onSwipe: this.onUserInteraction, - prevArrow: ( - - ), - nextArrow: ( - - ), - }; - - return settings; - } - - updateLastIndex = () => { - const { lastIndex, currentIndex } = this.state; - const updatedLastIndex = this.getLastIndex(); + const updateLastIndex = () => { + const updatedLastIndex = getLastIndex(); if (lastIndex !== updatedLastIndex) { - this.setState({ lastIndex: updatedLastIndex }); + setLastIndex(updatedLastIndex); } - if (this.slider.current && currentIndex > updatedLastIndex) { - this.slider.current.slickGoTo(updatedLastIndex); + if (slider.current && currentIndex > updatedLastIndex) { + slider.current.slickGoTo(updatedLastIndex); } }; - onCardChange = (_oldIndex: number, nextIndex: number) => { - this.timeoutId = null; + useEffect(() => { + if (shouldReplay()) { + replay(); + } + updateLastIndex(); + }); - this.setState({ - currentIndex: nextIndex, - }); + const onCardChange = (_oldIndex: number, nextIndex: number) => { + timeoutIdRef.current = null; + setCurrentIndex(nextIndex); }; - onUserInteraction = () => { - clearTimeout(this.timeoutId as number); - this.userHasInteracted = true; + const onUserInteraction = () => { + if (timeoutIdRef.current) { + clearTimeout(timeoutIdRef.current); + } + hasUserInteractedRef.current = true; - if (this.slider.current) { - this.slider.current.slickPause(); + if (slider.current) { + slider.current.slickPause(); } }; - replay() { - const { autoplaySpeed } = this.props; - const firstIndex = 0; + const carouselSettings = { + ...BASE_SLIDER_CONFIG, + arrows: !hideArrows, + autoplay, + autoplaySpeed, + beforeChange: onCardChange, + centerMode: numCardsVisible > 1 || infinite ? false : centerMode, + dots: !hideDots && !bottomRightAlignedArrows, + infinite, + nextArrow: ( + + ), + onSwipe: onUserInteraction, + prevArrow: ( + + ), + slidesToShow: numCardsVisible, + }; - const timeoutId = setTimeout(() => { - if (this.slider.current) { - this.slider.current.slickGoTo(firstIndex); - } - }, autoplaySpeed); + return ( + + + {/* eslint-disable-next-line react/jsx-props-no-spreading */} + + {children} + + + + ); +}; - this.timeoutId = timeoutId; - } - - render() { - const { children, carouselType = 'primary', numCardsVisible } = this.props; - const settings = this.getCarouselSettings(); - - return ( - - - {/* eslint-disable-next-line react/jsx-props-no-spreading */} - - {children} - - - - ); - } -} +Carousel.Card = Card; + +Carousel.propTypes = { + autoplay: PropTypes.bool, + autoplaySpeed: PropTypes.number, + bottomRightAlignedArrows: PropTypes.bool, + carouselType: PropTypes.oneOf(['primary', 'secondary']), + centerMode: PropTypes.bool, + children: PropTypes.arrayOf(PropTypes.node).isRequired, + hideArrows: PropTypes.bool, + hideDots: PropTypes.bool, + infinite: PropTypes.bool, + numCardsVisible: PropTypes.number.isRequired, +}; diff --git a/src/shared-components/checkbox/index.tsx b/src/shared-components/checkbox/index.tsx index dcd4339d5..f9f82255e 100644 --- a/src/shared-components/checkbox/index.tsx +++ b/src/shared-components/checkbox/index.tsx @@ -19,7 +19,7 @@ type CheckboxProps = { ) => void; size?: SizeType; type?: StyleType; - [key: string]: any; + [key: string]: unknown; }; /** diff --git a/src/shared-components/chip/index.tsx b/src/shared-components/chip/index.tsx index 33834ec0f..7e4c1505b 100644 --- a/src/shared-components/chip/index.tsx +++ b/src/shared-components/chip/index.tsx @@ -25,7 +25,3 @@ Chip.propTypes = { status: PropTypes.oneOf(['default', 'success', 'error', 'secondary']), text: PropTypes.string.isRequired, }; - -Chip.defaultProps = { - status: 'default', -}; diff --git a/src/shared-components/dialogModal/index.tsx b/src/shared-components/dialogModal/index.tsx index 1bededa81..f178d23c6 100644 --- a/src/shared-components/dialogModal/index.tsx +++ b/src/shared-components/dialogModal/index.tsx @@ -1,5 +1,5 @@ import PropTypes from 'prop-types'; -import React from 'react'; +import React, { useRef, useState, useEffect } from 'react'; import ReactDOM from 'react-dom'; import { Transition } from 'react-transition-group'; import { FocusScope } from '@react-aria/focus'; @@ -22,16 +22,16 @@ type DialogModalProps = { * If provided, DialogModal displays a Close Icon positioned top-right. * This function must contain the logic for closing the modal. */ - onCloseIconClick?: () => void | null; + onCloseIconClick?: () => void; title?: string; - [key: string]: any; + [key: string]: unknown; }; -type DialogModalState = { - isClosing: boolean; -}; - -const reactPortalSectionId = '#reactPortalSection'; +const REACT_PORTAL_SECTION_ID = 'reactPortalSection'; +const getHtmlNode = () => document.querySelector('html') || document.body; +const getDomNode = () => + (document.getElementById(REACT_PORTAL_SECTION_ID) as HTMLElement) || + document.body; /** * Dialog modals shouldn't contain large content and should not scroll unless screen size dictates it. To display large amounts of content, use `Immersive modal` instead. @@ -40,105 +40,79 @@ const reactPortalSectionId = '#reactPortalSection'; * * Dialog Modals should always contain at least 1 button and the logic should close the modal at some point. */ -export class DialogModal extends React.Component< - DialogModalProps, - DialogModalState -> { - static propTypes = { - children: PropTypes.node.isRequired, - onCloseIconClick: PropTypes.func, - title: PropTypes.string, - }; - - static defaultProps = { - onCloseIconClick: null, - title: '', - }; - - state = { - isClosing: false, - }; - - htmlNode: HTMLElement; - - domNode: HTMLElement; - - constructor(props: DialogModalProps) { - super(props); - - this.htmlNode = document.querySelector('html') || document.body; - - this.domNode = - document.querySelector(reactPortalSectionId) || document.body; - } - - componentDidMount(): void { - this.htmlNode.classList.add('no-scroll'); - } - - componentWillUnmount(): void { - this.htmlNode.classList.remove('no-scroll'); - } - - handleCloseIntent = (): void => { - const { onCloseIconClick } = this.props as Required; - +export const DialogModal = ({ + children, + onCloseIconClick, + title = '', + ...rest +}: DialogModalProps) => { + const [isClosing, setIsClosing] = useState(false); + + const domNode = useRef(getDomNode()); + const htmlNode = useRef(getHtmlNode()); + + useEffect(() => { + domNode.current = getDomNode(); + htmlNode.current = getHtmlNode(); + htmlNode.current.classList.add('no-scroll'); + return () => htmlNode.current.classList.remove('no-scroll'); + }, []); + + const handleCloseIntent = () => { if (onCloseIconClick) { - this.setState({ isClosing: true }); + setIsClosing(true); setTimeout(onCloseIconClick, 350); } }; - handleKeyDown = (event: React.KeyboardEvent): void => { + const handleKeyDown = (event: React.KeyboardEvent): void => { if (event.key === 'Escape') { - this.handleCloseIntent(); + handleCloseIntent(); } }; - render(): JSX.Element { - const { - children, title, onCloseIconClick, ...rest - } = this - .props as Required; - const { isClosing } = this.state; + return ReactDOM.createPortal( + + {(transitionState): JSX.Element => ( + // eslint-disable-next-line react/jsx-props-no-spreading + + + + {onCloseIconClick && ( + + + + )} + {!!title && {title}} + {children} + + + + )} + , + domNode.current, + ); +}; - return ReactDOM.createPortal( - - {(transitionState): JSX.Element => ( - // eslint-disable-next-line react/jsx-props-no-spreading - - - - {onCloseIconClick && ( - - - - )} - {!!title && {title}} - {children} - - - - )} - , - this.domNode, - ); - } -} +DialogModal.propTypes = { + children: PropTypes.node.isRequired, + onCloseIconClick: PropTypes.func, + title: PropTypes.string, +}; diff --git a/src/shared-components/offClickWrapper/index.tsx b/src/shared-components/offClickWrapper/index.tsx index 24a2a3f6c..302e32dee 100644 --- a/src/shared-components/offClickWrapper/index.tsx +++ b/src/shared-components/offClickWrapper/index.tsx @@ -1,4 +1,4 @@ -import React from 'react'; +import React, { useEffect, useRef } from 'react'; import PropTypes from 'prop-types'; type OffClickWrapperProps = { @@ -13,52 +13,21 @@ type OffClickWrapperProps = { onOffClick: (event: KeyboardEvent | MouseEvent) => void; }; -export class OffClickWrapper extends React.Component { - static propTypes = { - children: PropTypes.node.isRequired, - className: PropTypes.string, - onOffClick: PropTypes.func.isRequired, - }; - - static defaultProps = { - className: undefined, - }; - - containerRef: React.RefObject; - - constructor(props: OffClickWrapperProps) { - super(props); - this.containerRef = React.createRef(); - } - - componentDidMount() { - this.addOffClickListener(); - } - - componentWillUnmount() { - this.removeOffClickListener(); - } +export const OffClickWrapper = ({ + children, + className, + onOffClick, +}: OffClickWrapperProps) => { + const containerRef = useRef(null); - addOffClickListener = () => { - document.addEventListener('click', this.handleOffClick, false); - document.addEventListener('keydown', this.handleKeyPress, false); - }; - - removeOffClickListener = () => { - document.removeEventListener('click', this.handleOffClick, false); - document.removeEventListener('keydown', this.handleKeyPress, false); - }; - - handleKeyPress = (event: KeyboardEvent) => { + const handleKeyPress = (event: KeyboardEvent) => { if (event.key === 'Escape') { - const { onOffClick } = this.props; onOffClick(event); } }; - handleOffClick = (event: MouseEvent) => { - const node = this.containerRef.current; - const { onOffClick } = this.props; + const handleOffClick = (event: MouseEvent) => { + const node = containerRef.current; if (!node) { return; @@ -71,13 +40,24 @@ export class OffClickWrapper extends React.Component { onOffClick(event); }; - render() { - const { children, className } = this.props; + useEffect(() => { + document.addEventListener('click', handleOffClick, false); + document.addEventListener('keydown', handleKeyPress, false); + return () => { + document.removeEventListener('click', handleOffClick, false); + document.removeEventListener('keydown', handleKeyPress, false); + }; + }, []); + + return ( +
+ {children} +
+ ); +}; - return ( -
- {children} -
- ); - } -} +OffClickWrapper.propTypes = { + children: PropTypes.node.isRequired, + className: PropTypes.string, + onOffClick: PropTypes.func.isRequired, +}; diff --git a/src/shared-components/progressBar/index.tsx b/src/shared-components/progressBar/index.tsx index c2e8118cb..41d6b3dc7 100644 --- a/src/shared-components/progressBar/index.tsx +++ b/src/shared-components/progressBar/index.tsx @@ -19,7 +19,7 @@ type ProgressBarProps = { loadingTime?: string; status: ProgressBarStatusType; // ...rest type - [key: string]: any; + [key: string]: unknown; }; /** diff --git a/src/shared-components/radioButton/index.tsx b/src/shared-components/radioButton/index.tsx index 1d5dc54d4..86a6baea7 100644 --- a/src/shared-components/radioButton/index.tsx +++ b/src/shared-components/radioButton/index.tsx @@ -13,7 +13,7 @@ type RadioButtonProps = { ) => void; size?: SizeType; type?: StyleType; - [key: string]: any; + [key: string]: unknown; }; /** diff --git a/src/shared-components/selectorButton/index.tsx b/src/shared-components/selectorButton/index.tsx index 037bf57f5..01df8f829 100644 --- a/src/shared-components/selectorButton/index.tsx +++ b/src/shared-components/selectorButton/index.tsx @@ -28,7 +28,7 @@ type SelectorButtonProps = { selector?: SelectorType; size?: SizeType; type?: StyleType; - [key: string]: any; + [key: string]: unknown; }; export const SelectorButton = ({ diff --git a/src/utils/helperTransition/index.tsx b/src/utils/helperTransition/index.tsx index 253822161..fa9764364 100644 --- a/src/utils/helperTransition/index.tsx +++ b/src/utils/helperTransition/index.tsx @@ -38,7 +38,7 @@ const getStyleForTransitionState = (transitionState: string) => { type HelperTransitionParamsType = { children: JSX.Element; - [key: string]: any; + [key: string]: unknown; }; const HelperTransition = ({