From b5e1155a0a2e0ce23cbd440211ea4d39f56c4e8a Mon Sep 17 00:00:00 2001 From: Harry Yu Date: Fri, 20 Mar 2020 09:47:04 -0700 Subject: [PATCH 1/4] Made clicking on more parts of the backdrop dismiss it --- src/components/Carousel.js | 11 ++++++++++- src/components/Footer.js | 11 +++++++++-- src/components/Header.js | 11 +++++++++-- src/components/Modal/Modal.js | 25 ++++++++++++++++++++----- src/components/View.js | 11 +++++++++-- 5 files changed, 57 insertions(+), 12 deletions(-) diff --git a/src/components/Carousel.js b/src/components/Carousel.js index 8c012729..7481860c 100644 --- a/src/components/Carousel.js +++ b/src/components/Carousel.js @@ -87,6 +87,15 @@ const defaultProps = { }, }; +/** + * Used to get the className to select the track (area above and below the + * carousel) in other components. + * + * This className is we call `className()` in utils with to get the full + * className. + */ +export const trackBaseClassName = 'track'; + class Carousel extends Component { commonProps: any; // TODO components: CarouselComponents; @@ -381,7 +390,7 @@ class Carousel extends Component { {...this.getTrackProps(this.props)} style={{ display: 'flex', alignItems: 'center' }} currentView={currentIndex} - className={className('track')} + className={className(trackBaseClassName)} onViewChange={this.handleViewChange} ref={this.getTrack} > diff --git a/src/components/Footer.js b/src/components/Footer.js index eaa10c02..ebe88b15 100644 --- a/src/components/Footer.js +++ b/src/components/Footer.js @@ -42,6 +42,13 @@ export const footerCSS = ({ isModal, interactionIsIdle }: State) => ({ }, }); +/** + * Used to get the className to select the footer in other components. + * This className is we call `className()` in utils with to get the full + * className. + */ +export const footerBaseClassName = 'footer'; + const Footer = (props: Props) => { const { components, getStyles, innerProps, isFullscreen, isModal } = props; @@ -51,12 +58,12 @@ const Footer = (props: Props) => { const state = { isFullscreen, isModal }; const cn = { - container: className('footer', state), + container: className(footerBaseClassName, state), caption: className('footer__caption', state), count: className('footer__count', state), }; const css = { - container: getStyles('footer', props), + container: getStyles(footerBaseClassName, props), caption: getStyles('footerCaption', props), count: getStyles('footerCount', props), }; diff --git a/src/components/Header.js b/src/components/Header.js index 66f7fb58..f9a9059e 100644 --- a/src/components/Header.js +++ b/src/components/Header.js @@ -36,6 +36,13 @@ export const headerCSS = ({ interactionIsIdle }: State) => ({ zIndex: 1, }); +/** + * Used to get the className to select the header in other components. + * This className is we call `className()` in utils with to get the full + * className. + */ +export const headerBaseClassName = 'header'; + const Header = (props: Props) => { const { components, @@ -61,8 +68,8 @@ const Header = (props: Props) => { return (
void; @@ -55,6 +58,17 @@ const defaultProps = { preventScroll: true, styles: {}, }; + +/** Classes that when clicked on, close the backdrop */ +const backdropClassNames = new Set( + [ + viewBaseClassName, + headerBaseClassName, + footerBaseClassName, + trackBaseClassName, + ].map(className), +); + class Modal extends Component { commonProps: any; // TODO components: ModalComponents; @@ -106,10 +120,11 @@ class Modal extends Component { if (allowClose) this.handleClose(event); }; handleBackdropClick = (event: MouseEvent) => { - if ( - !event.target.classList.contains(className('view')) || - !this.props.closeOnBackdropClick - ) { + const hasBackdropClassName = event.target.classList.some(className => + backdropClassNames.has(className), + ); + + if (!hasBackdropClassName || !this.props.closeOnBackdropClick) { return; } diff --git a/src/components/View.js b/src/components/View.js index 57809aa8..a448cf42 100644 --- a/src/components/View.js +++ b/src/components/View.js @@ -20,6 +20,13 @@ export const viewCSS = () => ({ textAlign: 'center', }); +/** + * Used to get the className to select the view in other components. + * This className is we call `className()` in utils with to get the full + * className. + */ +export const viewBaseClassName = 'view'; + const View = (props: Props) => { const { data, formatters, getStyles, index, isFullscreen, isModal } = props; const innerProps = { @@ -29,8 +36,8 @@ const View = (props: Props) => { return (
Date: Fri, 20 Mar 2020 10:16:32 -0700 Subject: [PATCH 2/4] Fixed attempt to call .some() on DomTokenList, which isn't an array --- src/components/Modal/Modal.js | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/components/Modal/Modal.js b/src/components/Modal/Modal.js index bb95df3e..bd369b34 100644 --- a/src/components/Modal/Modal.js +++ b/src/components/Modal/Modal.js @@ -120,9 +120,12 @@ class Modal extends Component { if (allowClose) this.handleClose(event); }; handleBackdropClick = (event: MouseEvent) => { - const hasBackdropClassName = event.target.classList.some(className => - backdropClassNames.has(className), - ); + let hasBackdropClassName = false; + for (const targetClass of event.target.classList) { + if (backdropClassNames.has(targetClass)) { + hasBackdropClassName = true; + } + } if (!hasBackdropClassName || !this.props.closeOnBackdropClick) { return; From 208414719f46b2b448c2f29c628d5a77a67ce532 Mon Sep 17 00:00:00 2001 From: Harry Yu Date: Fri, 20 Mar 2020 10:49:47 -0700 Subject: [PATCH 3/4] Refactored all the classNames into common file to avoid import loops --- src/components/Carousel.js | 10 ++-------- src/components/Footer.js | 8 ++------ src/components/Header.js | 8 ++------ src/components/Modal/Modal.js | 14 ++++++-------- src/components/View.js | 8 ++------ src/components/componentBaseClassNames.js | 13 +++++++++++++ 6 files changed, 27 insertions(+), 34 deletions(-) create mode 100644 src/components/componentBaseClassNames.js diff --git a/src/components/Carousel.js b/src/components/Carousel.js index 7481860c..255112e4 100644 --- a/src/components/Carousel.js +++ b/src/components/Carousel.js @@ -18,6 +18,7 @@ import { type ModalProps } from './Modal/Modal'; import { className, isTouch } from '../utils'; import formatters from '../formatters'; import { type ViewsType } from '../types'; +import componentBaseClassNames from './componentBaseClassNames'; type SpringConfig = { [key: string]: number }; export type fn = any => void; @@ -87,14 +88,7 @@ const defaultProps = { }, }; -/** - * Used to get the className to select the track (area above and below the - * carousel) in other components. - * - * This className is we call `className()` in utils with to get the full - * className. - */ -export const trackBaseClassName = 'track'; +export const trackBaseClassName = componentBaseClassNames.Track; class Carousel extends Component { commonProps: any; // TODO diff --git a/src/components/Footer.js b/src/components/Footer.js index ebe88b15..e31f8a04 100644 --- a/src/components/Footer.js +++ b/src/components/Footer.js @@ -7,6 +7,7 @@ import { smallDevice } from './css-helpers'; import { Div, Span } from '../primitives'; import type { PropsWithStyles, ViewType } from '../types'; import { className } from '../utils'; +import componentBaseClassNames from './componentBaseClassNames'; type State = { isModal: boolean, interactionIsIdle: boolean }; type Props = State & @@ -42,12 +43,7 @@ export const footerCSS = ({ isModal, interactionIsIdle }: State) => ({ }, }); -/** - * Used to get the className to select the footer in other components. - * This className is we call `className()` in utils with to get the full - * className. - */ -export const footerBaseClassName = 'footer'; +const footerBaseClassName = componentBaseClassNames.Footer; const Footer = (props: Props) => { const { components, getStyles, innerProps, isFullscreen, isModal } = props; diff --git a/src/components/Header.js b/src/components/Header.js index f9a9059e..42064d79 100644 --- a/src/components/Header.js +++ b/src/components/Header.js @@ -7,6 +7,7 @@ import { Button, Div } from '../primitives'; import { className } from '../utils'; import type { PropsWithStyles } from '../types'; import { Close, FullscreenEnter, FullscreenExit } from './svg'; +import componentBaseClassNames from './componentBaseClassNames'; type State = { interactionIsIdle: boolean }; type Props = PropsWithStyles & @@ -36,12 +37,7 @@ export const headerCSS = ({ interactionIsIdle }: State) => ({ zIndex: 1, }); -/** - * Used to get the className to select the header in other components. - * This className is we call `className()` in utils with to get the full - * className. - */ -export const headerBaseClassName = 'header'; +export const headerBaseClassName = componentBaseClassNames.Header; const Header = (props: Props) => { const { diff --git a/src/components/Modal/Modal.js b/src/components/Modal/Modal.js index bd369b34..37d1e3e6 100644 --- a/src/components/Modal/Modal.js +++ b/src/components/Modal/Modal.js @@ -10,12 +10,10 @@ import { type ModalComponents, } from '../defaultComponents'; import { Fade, SlideUp } from './Animation'; -import { type CarouselType, trackBaseClassName } from '../Carousel'; +import { type CarouselType } from '../Carousel'; import { defaultModalStyles, type ModalStylesConfig } from '../../styles'; import { isTouch, className } from '../../utils'; -import { headerBaseClassName } from '../Header'; -import { footerBaseClassName } from '../Footer'; -import { viewBaseClassName } from '../View'; +import componentBaseClassNames from '../componentBaseClassNames'; type MouseOrKeyboardEvent = MouseEvent | KeyboardEvent; export type CloseType = (event: MouseOrKeyboardEvent) => void; @@ -62,10 +60,10 @@ const defaultProps = { /** Classes that when clicked on, close the backdrop */ const backdropClassNames = new Set( [ - viewBaseClassName, - headerBaseClassName, - footerBaseClassName, - trackBaseClassName, + componentBaseClassNames.View, + componentBaseClassNames.Header, + componentBaseClassNames.Footer, + componentBaseClassNames.Track, ].map(className), ); diff --git a/src/components/View.js b/src/components/View.js index a448cf42..71b40bec 100644 --- a/src/components/View.js +++ b/src/components/View.js @@ -7,6 +7,7 @@ import { Div, Img } from '../primitives'; import { type PropsWithStyles } from '../types'; import { className } from '../utils'; import { getSource } from './component-helpers'; +import componentBaseClassNames from './componentBaseClassNames'; type Props = PropsWithStyles & { data: Object, @@ -20,12 +21,7 @@ export const viewCSS = () => ({ textAlign: 'center', }); -/** - * Used to get the className to select the view in other components. - * This className is we call `className()` in utils with to get the full - * className. - */ -export const viewBaseClassName = 'view'; +export const viewBaseClassName = componentBaseClassNames.View; const View = (props: Props) => { const { data, formatters, getStyles, index, isFullscreen, isModal } = props; diff --git a/src/components/componentBaseClassNames.js b/src/components/componentBaseClassNames.js new file mode 100644 index 00000000..2a8cbc07 --- /dev/null +++ b/src/components/componentBaseClassNames.js @@ -0,0 +1,13 @@ +/** + * Used to get the HTML class to select specific components. + * We call `className()` in utils with each of these to get the full className, + * with prefixes. + */ +const componentBaseClassNames = { + Header: 'header', + Footer: 'footer', + View: 'view', + Track: 'track', +}; + +export default componentBaseClassNames; From 2261155bd8143ca908dd2f6068557174856d2f05 Mon Sep 17 00:00:00 2001 From: Harry Yu Date: Fri, 20 Mar 2020 10:55:16 -0700 Subject: [PATCH 4/4] Removed unused `export const` statements --- src/components/Carousel.js | 2 +- src/components/Header.js | 2 +- src/components/View.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/Carousel.js b/src/components/Carousel.js index 255112e4..bcb25f10 100644 --- a/src/components/Carousel.js +++ b/src/components/Carousel.js @@ -88,7 +88,7 @@ const defaultProps = { }, }; -export const trackBaseClassName = componentBaseClassNames.Track; +const trackBaseClassName = componentBaseClassNames.Track; class Carousel extends Component { commonProps: any; // TODO diff --git a/src/components/Header.js b/src/components/Header.js index 42064d79..39f9a554 100644 --- a/src/components/Header.js +++ b/src/components/Header.js @@ -37,7 +37,7 @@ export const headerCSS = ({ interactionIsIdle }: State) => ({ zIndex: 1, }); -export const headerBaseClassName = componentBaseClassNames.Header; +const headerBaseClassName = componentBaseClassNames.Header; const Header = (props: Props) => { const { diff --git a/src/components/View.js b/src/components/View.js index 71b40bec..d86045f1 100644 --- a/src/components/View.js +++ b/src/components/View.js @@ -21,7 +21,7 @@ export const viewCSS = () => ({ textAlign: 'center', }); -export const viewBaseClassName = componentBaseClassNames.View; +const viewBaseClassName = componentBaseClassNames.View; const View = (props: Props) => { const { data, formatters, getStyles, index, isFullscreen, isModal } = props;