Skip to content

Commit

Permalink
fix(modal): prop naming improvements (#16)
Browse files Browse the repository at this point in the history
  • Loading branch information
Meemaw committed Apr 8, 2019
1 parent 8643efc commit 68ff0b1
Show file tree
Hide file tree
Showing 7 changed files with 80 additions and 75 deletions.
10 changes: 5 additions & 5 deletions docs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ import MicroModal from 'react-micro-modal';
/*
Boolean describing if the modal should be open on first render. (if used as a uncontrolled component, else just use open)
*/
initiallyOpen={false}
openInitially={false}
/*
Function that recieves a handleClose function and should render the modal content.
*/
Expand All @@ -46,15 +46,15 @@ import MicroModal from 'react-micro-modal';
/*
Boolean indicating whether to close modal on escape keypress
*/
closeOnEscapeClick={true}
closeOnEscapePress={true}
/*
Boolean indicating whether to close modal on document click (outside of modal content)
*/
closeOnOverlayClick={false}
/*
Boolean indicating whether focus should be given to first element in modal after it got open
*/
disableFocus={false}
disableFirstElementFocus={false}
/*
Boolean indicating whether an animation should be used when closing the modal. Animation has to be applied as the modal is waiting for the "animationend" DOM event. Basic animation is provided and can be imported from "react-micro-modal/dist/index.css".
*/
Expand All @@ -70,10 +70,10 @@ import MicroModal from 'react-micro-modal';
/*
CSSProperties to be applied to the modal overlay element
*/
modalOverlayStyle={{ background: 'red' }}
modalOverlayStyles={{ background: 'red' }}
/*
CSSProperties to be applied to the modal container element
*/
containerStyle={{ background: 'red' }}
containerStyles={{ background: 'red' }}
/>;
```
7 changes: 4 additions & 3 deletions src/Portal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,23 @@ import ReactDOM from 'react-dom';

export interface PortalBaseProps {
parentSelector?: () => HTMLElement;
id?: string;
}

interface Props extends PortalBaseProps {
children: React.ReactNode;
}

function portalNode(): HTMLDivElement {
function portalNode(id?: string): HTMLDivElement {
const el = document.createElement('div');
el.className = `modal-portal-${portalIndex++}`;
el.className = id ? `${id}-portal` : `micro-modal-portal-${portalIndex++}`;
return el;
}

export let portalIndex = 1;

class ModalPortal extends React.Component<Props> {
node = portalNode();
node = portalNode(this.props.id);

componentDidMount() {
this.getParent().appendChild(this.node);
Expand Down
49 changes: 28 additions & 21 deletions src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,28 +12,30 @@ type State = {
function getInitialState(props: Props): State {
return {
isClosing: false,
open: props.initiallyOpen || false
open: props.openInitially || false
};
}

type OptionalProps = {
closeOnEscapeClick?: boolean;
closeOnEscapePress?: boolean;
closeOnOverlayClick?: boolean;
modalOverlayStyle?: React.CSSProperties;
containerStyle?: React.CSSProperties;
disableFocus?: boolean;
modalOverlayStyles?: React.CSSProperties;
containerStyles?: React.CSSProperties;
disableFirstElementFocus?: boolean;
closeOnAnimationEnd?: boolean;
modalClassName?: string;
modalOverlayClassName?: string;
initiallyOpen?: boolean;
openInitially?: boolean;
};

interface Props extends PortalBaseProps, OptionalProps {
children: (handleClose: () => void) => React.ReactNode;
export interface BaseProps extends PortalBaseProps, OptionalProps {
trigger?: (handleOpen: () => void) => React.ReactNode;
open?: boolean;
handleClose?: () => void;
id?: string;
}

interface Props extends BaseProps {
children: (handleClose: () => void) => React.ReactNode;
}

const ESCAPE_KEY: 'Escape' = 'Escape';
Expand All @@ -49,19 +51,19 @@ class MicroModal extends React.PureComponent<Props, State> {
readonly state: State = getInitialState(this.props);

static defaultProps: OptionalProps = {
disableFocus: false,
modalOverlayStyle: {},
containerStyle: {},
closeOnEscapeClick: true,
disableFirstElementFocus: false,
modalOverlayStyles: {},
containerStyles: {},
closeOnEscapePress: true,
closeOnOverlayClick: true,
closeOnAnimationEnd: false,
modalOverlayClassName: '',
modalClassName: '',
initiallyOpen: false
openInitially: false
};

componentDidMount() {
if (this.props.initiallyOpen && !this.props.disableFocus) {
if (this.props.openInitially && !this.props.disableFirstElementFocus) {
this.focusFirstNode();
}
}
Expand Down Expand Up @@ -157,7 +159,7 @@ class MicroModal extends React.PureComponent<Props, State> {
};

private focusFirstNode(): void {
if (this.props.disableFocus) {
if (this.props.disableFirstElementFocus) {
return;
}

Expand All @@ -184,7 +186,7 @@ class MicroModal extends React.PureComponent<Props, State> {

private onKeydown = (event: KeyboardEvent): void => {
if (this.containerRef === getLastOpenContainer()) {
if (event.key === ESCAPE_KEY && this.props.closeOnEscapeClick) {
if (event.key === ESCAPE_KEY && this.props.closeOnEscapePress) {
this.handleClose();
}
if (event.key === TAB_KEY) {
Expand Down Expand Up @@ -224,7 +226,7 @@ class MicroModal extends React.PureComponent<Props, State> {
const { zIndex, ...restOverlayStyle } = overlayStyle;

return (
<ModalPortal parentSelector={parentSelector}>
<ModalPortal parentSelector={parentSelector} id={this.props.id}>
<div
className={modalClassName}
aria-hidden={ariaHidden}
Expand All @@ -243,7 +245,7 @@ class MicroModal extends React.PureComponent<Props, State> {
>
<div
className="modal-container"
style={{ ...CONTAINER_BASE_STYLE, ...this.props.containerStyle }}
style={{ ...CONTAINER_BASE_STYLE, ...this.props.containerStyles }}
role="dialog"
aria-modal="true"
ref={this.containerRef}
Expand All @@ -258,7 +260,12 @@ class MicroModal extends React.PureComponent<Props, State> {
};

render() {
const { trigger, children, modalOverlayStyle, parentSelector } = this.props;
const {
trigger,
children,
modalOverlayStyles,
parentSelector
} = this.props;
const { open, isClosing } = this.state;

return (
Expand All @@ -267,7 +274,7 @@ class MicroModal extends React.PureComponent<Props, State> {
open,
isClosing,
children,
modalOverlayStyle!,
modalOverlayStyles!,
parentSelector
)}
{trigger !== undefined && trigger(this.handleOpen)}
Expand Down
10 changes: 5 additions & 5 deletions stories/Uncontrolled.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,18 @@ import { StoryUncontrolledModal } from './components';

storiesOf('react-micro-modal', module)
.add('Default', () => <StoryUncontrolledModal />)
.add('Initially open', () => <StoryUncontrolledModal initiallyOpen={true} />)
.add('Modal closing animation', () => (
.add('Initially open', () => <StoryUncontrolledModal openInitially={true} />)
.add('Animate modal closing', () => (
<StoryUncontrolledModal closeOnAnimationEnd={true} />
))
.add("Doesn't close on escape click", () => (
<StoryUncontrolledModal closeOnEscapeClick={false} />
<StoryUncontrolledModal closeOnEscapePress={false} />
))
.add("Doesn't close on document click", () => (
<StoryUncontrolledModal closeOnOverlayClick={false} />
))
.add('Disable focusing first element on afterOpen', () => (
<StoryUncontrolledModal disableFocus={true} />
.add('Disable first element focus on modal open', () => (
<StoryUncontrolledModal disableFirstElementFocus={true} />
))

.add('Custom className', () => (
Expand Down
16 changes: 2 additions & 14 deletions stories/components.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React from 'react';

import MicroModal from '../src';
import MicroModal, { BaseProps } from '../src';

type ContentProps = {
handleClose: () => void;
Expand Down Expand Up @@ -58,19 +58,7 @@ const StoryModalContent = ({ handleClose }: ContentProps) => {
);
};

type StoryModalProps = {
closeOnEscapeClick?: boolean;
closeOnOverlayClick?: boolean;
modalOverlayStyle?: React.CSSProperties;
containerStyle?: React.CSSProperties;
disableFocus?: boolean;
closeOnAnimationEnd?: boolean;
modalClassName?: string;
modalOverlayClassName?: string;
initiallyOpen?: boolean;
};

const StoryUncontrolledModal = (props: StoryModalProps) => (
const StoryUncontrolledModal = (props: BaseProps) => (
<MicroModal
trigger={handleOpen => (
<button role="button" onClick={handleOpen}>
Expand Down
41 changes: 18 additions & 23 deletions test/__helpers__/components.tsx
Original file line number Diff line number Diff line change
@@ -1,39 +1,32 @@
import React, { useState } from 'react';

import { BaseProps } from '../../src';
import Modal from '../../src/react-micro-modal';

export const closeModalElementText = 'Close';
export const openModalTriggerText = 'Trigger';
export const firstFocusableElementText = 'Anchor element';

type TestProps = {
closeOnEscapeClick?: boolean;
closeOnOverlayClick?: boolean;
modalClassName?: string;
modalOverlayClassName?: string;
closeOnAnimationEnd?: boolean;
initiallyOpen?: boolean;
open?: boolean;
};

const UncontrolledTestModal = ({
closeOnEscapeClick = true,
closeOnEscapePress = true,
closeOnOverlayClick = true,
modalClassName = '',
modalOverlayClassName = '',
closeOnAnimationEnd = false,
initiallyOpen = false,
open
}: TestProps) => {
openInitially = false,
open,
disableFirstElementFocus = false
}: BaseProps) => {
return (
<Modal
modalOverlayClassName={modalOverlayClassName}
closeOnAnimationEnd={closeOnAnimationEnd}
modalClassName={modalClassName}
closeOnEscapeClick={closeOnEscapeClick}
closeOnEscapePress={closeOnEscapePress}
closeOnOverlayClick={closeOnOverlayClick}
initiallyOpen={initiallyOpen}
openInitially={openInitially}
open={open}
disableFirstElementFocus={disableFirstElementFocus}
trigger={handleOpen => (
<button onClick={handleOpen}>{openModalTriggerText}</button>
)}
Expand All @@ -44,24 +37,26 @@ const UncontrolledTestModal = ({
};

const ControlledTestModal = ({
initiallyOpen = false,
closeOnEscapeClick = true,
openInitially = false,
closeOnEscapePress = true,
closeOnOverlayClick = true,
modalClassName = '',
modalOverlayClassName = '',
closeOnAnimationEnd = false
}: TestProps) => {
const [open, setOpen] = useState(initiallyOpen);
closeOnAnimationEnd = false,
disableFirstElementFocus = false
}: BaseProps) => {
const [open, setOpen] = useState(openInitially);
return (
<div>
<button onClick={() => setOpen(true)}>{openModalTriggerText}</button>
<Modal
closeOnAnimationEnd={closeOnAnimationEnd}
modalClassName={modalClassName}
modalOverlayClassName={modalOverlayClassName}
closeOnEscapeClick={closeOnEscapeClick}
closeOnEscapePress={closeOnEscapePress}
disableFirstElementFocus={disableFirstElementFocus}
closeOnOverlayClick={closeOnOverlayClick}
initiallyOpen={initiallyOpen}
openInitially={openInitially}
open={open}
handleClose={() => setOpen(false)}
>
Expand Down
22 changes: 18 additions & 4 deletions test/react-micro-modal.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ describe('Micro modal', () => {

it('Open modal should not close on escape key press', () => {
openModalShouldNotCloseOnEscapeKeyPress(
render(<ModalComponent closeOnEscapeClick={false} />)
render(<ModalComponent closeOnEscapePress={false} />)
);
});

Expand Down Expand Up @@ -128,8 +128,14 @@ describe('Micro modal', () => {
});

it('Should be initially open with first element focues', () => {
shouldBeInitiallyOpenWithFocuesElement(
render(<ModalComponent initiallyOpen={true} />)
shouldBeInitiallyOpenWithFocusedElement(
render(<ModalComponent openInitially={true} />)
);
});

it('Should not focus first element when focus disabled', () => {
shouldNotFocusFirstElementOnDisableFocus(
render(<ModalComponent disableFirstElementFocus />)
);
});

Expand All @@ -147,7 +153,15 @@ describe('Micro modal', () => {
});
});

function shouldBeInitiallyOpenWithFocuesElement(renderResult: RenderResult) {
function shouldNotFocusFirstElementOnDisableFocus(renderResult: RenderResult) {
const { getByTestId, getByText } = renderResult;
let modalWrapper = getByTestId('micro-modal');
openModal(getByText);
expectModalIsOpen(modalWrapper);
expect(getByText(firstFocusableElementText)).not.toBe(document.activeElement);
}

function shouldBeInitiallyOpenWithFocusedElement(renderResult: RenderResult) {
const { getByTestId, getByText } = renderResult;
let modalWrapper = getByTestId('micro-modal');
expectModalIsOpen(modalWrapper);
Expand Down

0 comments on commit 68ff0b1

Please sign in to comment.