Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Overlay] better lifecycle props for all overlay components #2581

Merged
merged 11 commits into from
Jun 12, 2018
24 changes: 17 additions & 7 deletions packages/core/src/components/alert/alert.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ import { safeInvoke } from "../../common/utils";
import { Button } from "../button/buttons";
import { Dialog } from "../dialog/dialog";
import { Icon, IconName } from "../icon/icon";
import { IOverlayLifecycleProps } from "../overlay/overlay";

export interface IAlertProps extends IProps {
export interface IAlertProps extends IOverlayLifecycleProps, IProps {
/**
* Whether pressing <kbd>escape</kbd> when focused on the Alert should cancel the alert.
* If this prop is enabled, then either `onCancel` or `onClose` must also be defined.
Expand Down Expand Up @@ -110,16 +111,25 @@ export class Alert extends AbstractPureComponent<IAlertProps, {}> {
public static displayName = "Blueprint2.Alert";

public render() {
const { children, className, icon, intent, cancelButtonText, confirmButtonText } = this.props;
const {
canEscapeKeyCancel,
canOutsideClickCancel,
children,
className,
icon,
intent,
cancelButtonText,
confirmButtonText,
onClose,
...overlayProps
} = this.props;
return (
<Dialog
{...overlayProps}
className={classNames(Classes.ALERT, className)}
canEscapeKeyClose={this.props.canEscapeKeyCancel}
canOutsideClickClose={this.props.canOutsideClickCancel}
isOpen={this.props.isOpen}
canEscapeKeyClose={canEscapeKeyCancel}
canOutsideClickClose={canOutsideClickCancel}
onClose={this.handleCancel}
style={this.props.style}
transitionDuration={this.props.transitionDuration}
>
<div className={Classes.ALERT_BODY}>
<Icon icon={icon} iconSize={40} intent={intent} />
Expand Down
12 changes: 4 additions & 8 deletions packages/core/src/components/context-menu/contextMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,14 @@ import { AbstractPureComponent } from "../../common/abstractPureComponent";
import * as Classes from "../../common/classes";
import { Position } from "../../common/position";
import { safeInvoke } from "../../common/utils";
import { IOverlayLifecycleProps } from "../overlay/overlay";
import { Popover, PopperModifiers } from "../popover/popover";

export interface IOffset {
left: number;
top: number;
}

interface IContextMenuProps {
/** Callback invoked after the menu has finished transitioning to a closed state. */
didClose: () => void;
}

interface IContextMenuState {
isOpen: boolean;
isDarkTheme: boolean;
Expand All @@ -38,7 +34,7 @@ const POPPER_MODIFIERS: PopperModifiers = {
const TRANSITION_DURATION = 100;

/* istanbul ignore next */
class ContextMenu extends AbstractPureComponent<IContextMenuProps, IContextMenuState> {
class ContextMenu extends AbstractPureComponent<IOverlayLifecycleProps, IContextMenuState> {
public state: IContextMenuState = {
isDarkTheme: false,
isOpen: false,
Expand All @@ -56,6 +52,7 @@ class ContextMenu extends AbstractPureComponent<IContextMenuProps, IContextMenuS
return (
<div className={Classes.CONTEXT_MENU_POPOVER_TARGET} style={this.state.offset}>
<Popover
{...this.props}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems like a pretty major change where users could possible pass props to the popover this way, now they can't.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this component is not even exported, so it think we're safe here. users use either a decorator @ContextMenuTarget or a static method ContextMenu.show() (which looks like this component but isn't).

backdropProps={{ onContextMenu: this.handleBackdropContextMenu }}
content={content}
enforceFocus={false}
Expand All @@ -66,7 +63,6 @@ class ContextMenu extends AbstractPureComponent<IContextMenuProps, IContextMenuS
onInteraction={this.handlePopoverInteraction}
position={Position.RIGHT_TOP}
popoverClassName={popoverClassName}
popoverDidClose={this.props.didClose}
target={<div />}
transitionDuration={TRANSITION_DURATION}
/>
Expand Down Expand Up @@ -122,7 +118,7 @@ export function show(menu: JSX.Element, offset: IOffset, onClose?: () => void, i
contextMenuElement = document.createElement("div");
contextMenuElement.classList.add(Classes.CONTEXT_MENU);
document.body.appendChild(contextMenuElement);
contextMenu = ReactDOM.render(<ContextMenu didClose={remove} />, contextMenuElement) as ContextMenu;
contextMenu = ReactDOM.render(<ContextMenu onClosed={remove} />, contextMenuElement) as ContextMenu;
}

contextMenu.show(menu, offset, onClose, isDarkTheme);
Expand Down
66 changes: 42 additions & 24 deletions packages/core/src/components/overlay/overlay.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { IProps } from "../../common/props";
import { safeInvoke } from "../../common/utils";
import { Portal } from "../portal/portal";

export interface IOverlayableProps {
export interface IOverlayableProps extends IOverlayLifecycleProps {
/**
* Whether the overlay should acquire application focus when it first opens.
* @default true
Expand Down Expand Up @@ -72,11 +72,40 @@ export interface IOverlayableProps {
/**
* A callback that is invoked when user interaction causes the overlay to close, such as
* clicking on the overlay or pressing the `esc` key (if enabled).
*
* Receives the event from the user's interaction, if there was an event (generally either a
* mouse or key event). Note that, since this component is controlled by the `isOpen` prop, it
* will not actually close itself until that prop becomes `false`.
*/
onClose?(event?: React.SyntheticEvent<HTMLElement>): void;
onClose?: (event?: React.SyntheticEvent<HTMLElement>) => void;
}

export interface IOverlayLifecycleProps {
/**
* Lifecycle method invoked just before the CSS _close_ transition begins on
* a child. Receives the DOM element of the child being closed.
*/
onClosing?: (node: HTMLElement) => void;

/**
* Lifecycle method invoked just after the CSS _close_ transition ends but
* before the child has been removed from the DOM. Receives the DOM element
* of the child being closed.
*/
onClosed?: (node: HTMLElement) => void;

/**
* Lifecycle method invoked just after mounting the child in the DOM but
* just before the CSS _open_ transition begins. Receives the DOM element of
* the child being opened.
*/
onOpening?: (node: HTMLElement) => void;

/**
* Lifecycle method invoked just after the CSS _open_ transition ends.
* Receives the DOM element of the child being opened.
*/
onOpened?: (node: HTMLElement) => void;
}

export interface IBackdropProps {
Expand All @@ -101,16 +130,6 @@ export interface IBackdropProps {
}

export interface IOverlayProps extends IOverlayableProps, IBackdropProps, IProps {
/** Lifecycle callback invoked after the overlay opens and is mounted in the DOM. */
didOpen?: () => any;

/**
* Lifecycle callback invoked after a child element finishes exiting the DOM.
* This will be invoked for each child of the `Overlay` except for the backdrop element.
* The argument is the underlying HTML element that left the DOM.
*/
didClose?: (node: HTMLElement) => any;

/**
* Toggles the visibility of the overlay and its children.
* This prop is required because the component is controlled.
Expand Down Expand Up @@ -195,7 +214,7 @@ export class Overlay extends React.PureComponent<IOverlayProps, IOverlayState> {
</TransitionGroup>
);
if (usePortal) {
return <Portal onChildrenMount={this.handleContentMount}>{transitionGroup}</Portal>;
return <Portal>{transitionGroup}</Portal>;
} else {
return transitionGroup;
}
Expand Down Expand Up @@ -266,9 +285,16 @@ export class Overlay extends React.PureComponent<IOverlayProps, IOverlayState> {
) : (
<span className={Classes.OVERLAY_CONTENT}>{child}</span>
);
const { transitionDuration, transitionName } = this.props;
const { onOpening, onOpened, onClosing, onClosed, transitionDuration, transitionName } = this.props;
return (
<CSSTransition classNames={transitionName} onExited={this.props.didClose} timeout={transitionDuration}>
<CSSTransition
classNames={transitionName}
onEntering={onOpening}
onEntered={onOpened}
onExiting={onClosing}
onExited={onClosed}
timeout={transitionDuration}
>
{decoratedChild}
</CSSTransition>
);
Expand Down Expand Up @@ -339,9 +365,7 @@ export class Overlay extends React.PureComponent<IOverlayProps, IOverlayState> {
document.addEventListener("mousedown", this.handleDocumentClick);
}

if (!this.props.usePortal) {
safeInvoke(this.props.didOpen);
} else if (this.props.hasBackdrop) {
if (this.props.hasBackdrop && this.props.usePortal) {
// add a class to the body to prevent scrolling of content below the overlay
document.body.classList.add(Classes.OVERLAY_OPEN);
}
Expand Down Expand Up @@ -378,12 +402,6 @@ export class Overlay extends React.PureComponent<IOverlayProps, IOverlayState> {
}
};

private handleContentMount = () => {
if (this.props.isOpen) {
safeInvoke(this.props.didOpen);
}
};

private handleDocumentFocus = (e: FocusEvent) => {
if (
this.props.enforceFocus &&
Expand Down
50 changes: 7 additions & 43 deletions packages/core/src/components/popover/popover.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -134,31 +134,11 @@ export interface IPopoverProps extends IOverlayableProps, IProps {
*/
popoverClassName?: string;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a major API deprecation. What is the upgrade path for existing code?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: this interface inherits the new lifecycle interface, so this will just be a rename.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/**
* Callback invoked after the popover closes and has been removed from the DOM.
*/
popoverDidClose?: () => void;

/**
* Callback invoked when the popover opens after it is added to the DOM.
*/
popoverDidOpen?: () => void;

/**
* Ref supplied to the `Classes.POPOVER` element.
*/
popoverRef?: (ref: HTMLDivElement | null) => void;

/**
* Callback invoked when a popover begins to close.
*/
popoverWillClose?: () => void;

/**
* Callback invoked before the popover opens.
*/
popoverWillOpen?: () => void;

/**
* Space-delimited string of class names applied to the
* portal that holds the popover if `usePortal={true}`.
Expand Down Expand Up @@ -248,8 +228,6 @@ export class Popover extends AbstractPureComponent<IPopoverProps, IPopoverState>
/** DOM element that contains the target. */
public targetElement: HTMLElement;

// a flag that is set to true while we are waiting for the underlying Portal to complete rendering
private isContentMounting = false;
private cancelOpenTimeout: () => void;

// a flag that lets us detect mouse movement between the target and popover,
Expand Down Expand Up @@ -277,7 +255,7 @@ export class Popover extends AbstractPureComponent<IPopoverProps, IPopoverState>
}

public render() {
const { className, disabled, hasBackdrop, targetClassName, targetElementTag } = this.props;
const { className, disabled, targetClassName, targetElementTag } = this.props;
const { isOpen } = this.state;
const isHoverInteractionKind = this.isHoverInteractionKind();

Expand Down Expand Up @@ -333,15 +311,17 @@ export class Popover extends AbstractPureComponent<IPopoverProps, IPopoverState>
canEscapeKeyClose={this.props.canEscapeKeyClose}
canOutsideClickClose={this.props.interactionKind === PopoverInteractionKind.CLICK}
className={this.props.portalClassName}
didClose={this.props.popoverDidClose}
didOpen={this.handleContentMount}
enforceFocus={this.props.enforceFocus}
hasBackdrop={hasBackdrop}
usePortal={this.props.usePortal}
hasBackdrop={this.props.hasBackdrop}
isOpen={isOpen && !isContentEmpty}
onClose={this.handleOverlayClose}
onClosed={this.props.onClosed}
onClosing={this.props.onClosing}
onOpened={this.props.onOpened}
onOpening={this.props.onOpening}
transitionDuration={this.props.transitionDuration}
transitionName={Classes.POPOVER}
usePortal={this.props.usePortal}
>
{this.renderPopper(children.content)}
</Overlay>
Expand Down Expand Up @@ -369,15 +349,6 @@ export class Popover extends AbstractPureComponent<IPopoverProps, IPopoverState>
}
}

public componentWillUpdate(_: IPopoverProps, nextState: IPopoverState) {
if (!this.state.isOpen && nextState.isOpen) {
this.isContentMounting = true;
Utils.safeInvoke(this.props.popoverWillOpen);
} else if (this.state.isOpen && !nextState.isOpen) {
Utils.safeInvoke(this.props.popoverWillClose);
}
}

public componentDidUpdate() {
this.updateDarkParent();
}
Expand Down Expand Up @@ -504,13 +475,6 @@ export class Popover extends AbstractPureComponent<IPopoverProps, IPopoverState>
}
}

private handleContentMount = () => {
if (this.isContentMounting) {
Utils.safeInvoke(this.props.popoverDidOpen);
this.isContentMounting = false;
}
};

private handleTargetFocus = (e: React.FocusEvent<HTMLElement>) => {
if (this.props.openOnTargetFocus && this.isHoverInteractionKind()) {
if (e.relatedTarget == null && !this.lostFocusOnSamePage) {
Expand Down
3 changes: 2 additions & 1 deletion packages/core/src/components/tooltip/tooltip.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@ import * as React from "react";
import * as Classes from "../../common/classes";
import { Position } from "../../common/position";
import { IIntentProps, IProps } from "../../common/props";
import { IOverlayLifecycleProps } from "../overlay/overlay";
import { Popover, PopoverInteractionKind, PopperModifiers } from "../popover/popover";

export interface ITooltipProps extends IProps, IIntentProps {
export interface ITooltipProps extends IProps, IIntentProps, IOverlayLifecycleProps {
/**
* The content that will be displayed inside of the tooltip.
*/
Expand Down
13 changes: 13 additions & 0 deletions packages/core/test/alert/alertTests.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,19 @@ describe("<Alert>", () => {
assert.lengthOf(wrapper.find(Icon), 1);
});

it("supports overlay lifecycle props", () => {
const onOpening = spy();
const wrapper = mount(
<Alert isOpen={true} onOpening={onOpening}>
Alert
<p>Are you sure you want to delete this file?</p>
<p>There is no going back.</p>
</Alert>,
);
assert.isTrue(onOpening.calledOnce);
wrapper.unmount();
});

describe("confirm button", () => {
const onConfirm = spy();
const onClose = spy();
Expand Down
10 changes: 10 additions & 0 deletions packages/core/test/dialog/dialogTests.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,16 @@ describe("<Dialog>", () => {
assert.isTrue(onClose.notCalled);
});

it("supports overlay lifecycle props", () => {
const onOpening = spy();
mount(
<Dialog isOpen={true} onOpening={onOpening}>
body
</Dialog>,
);
assert.isTrue(onOpening.calledOnce);
});

describe("header", () => {
it(`renders .${Classes.DIALOG_HEADER} if title prop is given`, () => {
const dialog = mount(
Expand Down
Loading