Skip to content

Commit

Permalink
[Popover] add targetProps (#3213)
Browse files Browse the repository at this point in the history
* add safeInvokeMember util

* add Popover targetProps, using new util for overridden events

* tests

* only one spread

* add note about ref

* add notes about types
  • Loading branch information
giladgray authored Dec 5, 2018
1 parent 12827db commit 19d1b78
Show file tree
Hide file tree
Showing 5 changed files with 138 additions and 5 deletions.
1 change: 1 addition & 0 deletions packages/core/src/common/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import * as React from "react";
import { CLAMP_MIN_MAX } from "./errors";

export * from "./utils/compareUtils";
export * from "./utils/safeInvokeMember";

// only accessible within this file, so use `Utils.isNodeEnv(env)` from the outside.
declare var process: { env: any };
Expand Down
85 changes: 85 additions & 0 deletions packages/core/src/common/utils/safeInvokeMember.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
/*
* Copyright 2018 Palantir Technologies, Inc. All rights reserved.
*
* Licensed under the terms of the LICENSE file distributed with this project.
*/

import { isFunction } from "../utils";

/*
Understanding the types here:
`<Object, Key, ...Args, Return>`
`{ [k in K]?: (a: A) => R }`: This is a MAPPED TYPE that enforces that keys `K`
are all optional member functions with the given signature. The type `k` is only
used in the mapping definition.
`K extends keyof T`: A subset of the keys of the object `T`. The mapped type
above then imposes a restriction on the _values_ of these keys in `T`. Note that
`safeInvokeMember` only supports a single key, so the subset here has exactly
one item.
*/

/**
* Safely invoke the member function with no arguments, if the object
* exists and the given key is indeed a function, and return its value.
* Otherwise, return `undefined`.
*/
export function safeInvokeMember<T extends { [k in K]?: () => R }, K extends keyof T, R = void>(
obj: T | undefined,
key: K,
): R | undefined;
/**
* Safely invoke the member function with one argument, if the object
* exists and the given key is indeed a function, and return its value.
* Otherwise, return `undefined`.
*
* ```js
* // example usage
* safeInvokeMember(this.props.inputProps, "onChange", evt);
* ```
*/
export function safeInvokeMember<T extends { [k in K]?: (a: A) => R }, K extends keyof T, A, R = void>(
obj: T | undefined,
key: K,
arg1: A,
): R | undefined;
/**
* Safely invoke the member function with two arguments, if the object
* exists and the given key is indeed a function, and return its value.
* Otherwise, return `undefined`.
*/
export function safeInvokeMember<T extends { [k in K]?: (a: A, b: B) => R }, K extends keyof T, A, B, R = void>(
obj: T | undefined,
key: K,
arg1: A,
arg2: B,
): R | undefined;
/**
* Safely invoke the member function with three arguments, if the object
* exists and the given key is indeed a function, and return its value.
* Otherwise, return undefined.
*/
export function safeInvokeMember<
T extends { [k in K]?: (a: A, b: B, c: C) => R },
K extends keyof T,
A,
B,
C,
R = void
>(obj: T | undefined, key: K, arg1: A, arg2: B, arg3: C): R | undefined;
// tslint:disable-next-line:ban-types
export function safeInvokeMember<T extends { [P in K]?: Function }, K extends keyof T>(
obj: T | null | undefined,
key: K,
...args: any[]
) {
if (obj != null) {
const member = obj[key];
if (isFunction(member)) {
return member(...args);
}
}
return undefined;
}
22 changes: 17 additions & 5 deletions packages/core/src/components/popover/popover.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -296,11 +296,11 @@ export class Popover extends AbstractPureComponent<IPopoverProps, IPopoverState>
};

private renderTarget = (referenceProps: ReferenceChildrenProps) => {
const { openOnTargetFocus, targetClassName, targetTagName: TagName } = this.props;
const { openOnTargetFocus, targetClassName, targetProps = {}, targetTagName: TagName } = this.props;
const { isOpen } = this.state;
const isHoverInteractionKind = this.isHoverInteractionKind();

const targetProps: React.HTMLProps<HTMLElement> = isHoverInteractionKind
const finalTargetProps: React.HTMLProps<HTMLElement> = isHoverInteractionKind
? {
// HOVER handlers
onBlur: this.handleTargetBlur,
Expand All @@ -312,8 +312,13 @@ export class Popover extends AbstractPureComponent<IPopoverProps, IPopoverState>
// CLICK needs only one handler
onClick: this.handleTargetClick,
};
targetProps.className = classNames(Classes.POPOVER_TARGET, { [Classes.POPOVER_OPEN]: isOpen }, targetClassName);
targetProps.ref = referenceProps.ref;
finalTargetProps.className = classNames(
Classes.POPOVER_TARGET,
{ [Classes.POPOVER_OPEN]: isOpen },
targetProps.className,
targetClassName,
);
finalTargetProps.ref = referenceProps.ref;

const rawTarget = Utils.ensureElement(this.understandChildren().target);
const rawTabIndex = rawTarget.props.tabIndex;
Expand All @@ -329,7 +334,9 @@ export class Popover extends AbstractPureComponent<IPopoverProps, IPopoverState>
});
return (
<ResizeSensor onResize={this.handlePopoverResize}>
<TagName {...targetProps}>{clonedTarget}</TagName>
<TagName {...targetProps} {...finalTargetProps}>
{clonedTarget}
</TagName>
</ResizeSensor>
);
};
Expand Down Expand Up @@ -386,6 +393,7 @@ export class Popover extends AbstractPureComponent<IPopoverProps, IPopoverState>
}
this.handleMouseEnter(e);
}
Utils.safeInvokeMember(this.props.targetProps, "onFocus", e);
};

private handleTargetBlur = (e: React.FocusEvent<HTMLElement>) => {
Expand All @@ -397,6 +405,7 @@ export class Popover extends AbstractPureComponent<IPopoverProps, IPopoverState>
}
}
this.lostFocusOnSamePage = e.relatedTarget != null;
Utils.safeInvokeMember(this.props.targetProps, "onBlur", e);
};

private handleMouseEnter = (e: React.SyntheticEvent<HTMLElement>) => {
Expand All @@ -415,6 +424,7 @@ export class Popover extends AbstractPureComponent<IPopoverProps, IPopoverState>
// only begin opening popover when it is enabled
this.setOpenState(true, e, this.props.hoverOpenDelay);
}
Utils.safeInvokeMember(this.props.targetProps, "onMouseEnter", e);
};

private handleMouseLeave = (e: React.SyntheticEvent<HTMLElement>) => {
Expand All @@ -430,6 +440,7 @@ export class Popover extends AbstractPureComponent<IPopoverProps, IPopoverState>
// user-configurable closing delay is helpful when moving mouse from target to popover
this.setOpenState(false, e, this.props.hoverCloseDelay);
});
Utils.safeInvokeMember(this.props.targetProps, "onMouseLeave", e);
};

private handlePopoverClick = (e: React.MouseEvent<HTMLElement>) => {
Expand Down Expand Up @@ -465,6 +476,7 @@ export class Popover extends AbstractPureComponent<IPopoverProps, IPopoverState>
this.setOpenState(!this.props.isOpen, e);
}
}
Utils.safeInvokeMember(this.props.targetProps, "onClick", e);
};

// a wrapper around setState({isOpen}) that will call props.onInteraction instead when in controlled mode.
Expand Down
6 changes: 6 additions & 0 deletions packages/core/src/components/popover/popoverSharedProps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,12 @@ export interface IPopoverSharedProps extends IOverlayableProps, IProps {
*/
targetClassName?: string;

/**
* HTML props to spread to target element. Use `targetTagName` to change
* the type of element rendered. Note that `ref` is not supported.
*/
targetProps?: React.HTMLAttributes<HTMLElement>;

/**
* HTML tag name for the target element. This must be an HTML element to
* ensure that it supports the necessary DOM event handlers.
Expand Down
29 changes: 29 additions & 0 deletions packages/core/test/popover/popoverTests.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,35 @@ describe("<Popover>", () => {
assert.isTrue(onOpening.calledOnce);
});

describe("targetProps", () => {
const spy = sinon.spy();
const targetProps: React.HTMLAttributes<HTMLElement> = {
className: "test-test",
// hover & click events & onKeyDown for fun
onClick: spy,
onKeyDown: spy,
onMouseEnter: spy,
onMouseLeave: spy,
tabIndex: 400,
};
function targetPropsTest(interactionKind: PopoverInteractionKind) {
spy.resetHistory();
wrapper = renderPopover({ interactionKind, targetTagName: "address", targetProps })
.simulateTarget("click")
.simulateTarget("keydown")
.simulateTarget("mouseenter")
.simulateTarget("mouseleave");
const target = wrapper.find("address");
assert.isTrue(target.prop("className").indexOf(Classes.POPOVER_TARGET) >= 0);
assert.isTrue(target.prop("className").indexOf(targetProps.className) >= 0);
assert.equal(target.prop("tabIndex"), targetProps.tabIndex);
assert.equal(spy.callCount, 4);
}

it("passed to target element (click)", () => targetPropsTest("click"));
it("passed to target element (hover)", () => targetPropsTest("hover"));
});

describe("openOnTargetFocus", () => {
describe("if true (default)", () => {
it('adds tabindex="0" to target\'s child node when interactionKind is HOVER', () => {
Expand Down

1 comment on commit 19d1b78

@blueprint-bot
Copy link

Choose a reason for hiding this comment

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

[Popover] add targetProps (#3213)

Previews: documentation | landing | table

Please sign in to comment.