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

[OverflowList] add onOverflow callback prop #2975

Merged
merged 11 commits into from
Oct 4, 2018
52 changes: 49 additions & 3 deletions packages/core/src/components/overflow-list/overflowList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,16 @@ import { Boundary } from "../../common/boundary";
import * as Classes from "../../common/classes";
import { OVERFLOW_LIST_OBSERVE_PARENTS_CHANGED } from "../../common/errors";
import { DISPLAYNAME_PREFIX, IProps } from "../../common/props";
import { safeInvoke } from "../../common/utils";
import { IResizeEntry, ResizeSensor } from "../resize-sensor/resizeSensor";

/** @internal - do not expose this type */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

thoughts on doing this to reduce the exported surface area? it excludes it from the .d.ts files and required marking the state field similarly (line 84).

Copy link
Contributor

Choose a reason for hiding this comment

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

🆒

export enum OverflowDirection {
NONE,
GROW,
SHRINK,
}

export interface IOverflowListProps<T> extends IProps {
/**
* Which direction the items should collapse from: start or end of the
Expand Down Expand Up @@ -47,6 +55,13 @@ export interface IOverflowListProps<T> extends IProps {
*/
observeParents?: boolean;

/**
* Callback invoked when the overflowed items change. This is called once
* after the DOM has settled, rather that on every intermediate change. It
* is not invoked if resizing produces an unchanged overflow state.
*/
onOverflow?: (overflowItems: T[]) => void;

/**
* Callback invoked to render the overflowed items. Unlike
* `visibleItemRenderer`, this prop is invoked once with all items that do
Expand All @@ -68,6 +83,13 @@ export interface IOverflowListProps<T> extends IProps {
}

export interface IOverflowListState<T> {
/**
* Direction of current overflow operation. An overflow can take several frames to settle.
* @internal don't expose the type
*/
direction: OverflowDirection;
/** Length of last overflow to dedupe `onOverflow` calls during smooth resizing. */
lastOverflowCount: number;
overflow: T[];
visible: T[];
}
Expand All @@ -85,6 +107,8 @@ export class OverflowList<T> extends React.PureComponent<IOverflowListProps<T>,
}

public state: IOverflowListState<T> = {
direction: OverflowDirection.NONE,
Copy link
Member

Choose a reason for hiding this comment

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

We initially call repartition in shrinking mode, so maybe this should be SHRINK (if you want onOverflow to be called initially, even when the overflow is empty. If you don't, just keep NONE)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i changed to SHRINK but it was not called initially in tests, so just sticking with this.

lastOverflowCount: 0,
overflow: [],
visible: this.props.items,
};
Expand Down Expand Up @@ -118,14 +142,25 @@ export class OverflowList<T> extends React.PureComponent<IOverflowListProps<T>,
) {
// reset visible state if the above props change.
this.setState({
direction: OverflowDirection.GROW,
giladgray marked this conversation as resolved.
Show resolved Hide resolved
lastOverflowCount: 0,
overflow: [],
visible: nextProps.items,
});
}
}

public componentDidUpdate() {
public componentDidUpdate(_prevProps: IOverflowListProps<T>, prevState: IOverflowListState<T>) {
this.repartition(false);
giladgray marked this conversation as resolved.
Show resolved Hide resolved
const { direction, overflow, lastOverflowCount } = this.state;
if (
// if a resize operation has just completed (transition to NONE)
direction === OverflowDirection.NONE &&
direction !== prevState.direction &&
overflow.length !== lastOverflowCount
) {
safeInvoke(this.props.onOverflow, overflow);
}
}

public render() {
Expand Down Expand Up @@ -166,10 +201,14 @@ export class OverflowList<T> extends React.PureComponent<IOverflowListProps<T>,
return;
}
if (growing) {
this.setState({
this.setState(state => ({
direction: OverflowDirection.GROW,
// store last overflow if this is the beginning of a resize (for check in componentDidUpdate).
lastOverflowCount:
state.direction === OverflowDirection.NONE ? state.overflow.length : state.lastOverflowCount,
overflow: [],
visible: this.props.items,
});
}));
} else if (this.spacer.getBoundingClientRect().width < 0.9) {
// spacer has flex-shrink and width 1px so if it's much smaller then we know to shrink
this.setState(state => {
Expand All @@ -184,10 +223,17 @@ export class OverflowList<T> extends React.PureComponent<IOverflowListProps<T>,
}
const overflow = collapseFromStart ? [...state.overflow, next] : [next, ...state.overflow];
return {
// set SHRINK mode unless a GROW is already in progress.
giladgray marked this conversation as resolved.
Show resolved Hide resolved
// GROW shows all items then shrinks until it settles, so we
// preserve the fact that the original trigger was a GROW.
direction: state.direction === OverflowDirection.NONE ? OverflowDirection.SHRINK : state.direction,
overflow,
visible,
};
});
} else {
// repartition complete!
this.setState({ direction: OverflowDirection.NONE });
}
}
}
67 changes: 64 additions & 3 deletions packages/core/test/overflow-list/overflowListTests.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { assert } from "chai";
import { mount } from "enzyme";
import * as React from "react";

import { spy } from "sinon";
import { IOverflowListProps, IOverflowListState, OverflowList } from "../../src/components/overflow-list/overflowList";

type OverflowProps = IOverflowListProps<ITestItem>;
Expand Down Expand Up @@ -86,6 +87,58 @@ describe("<OverflowList>", function(this) {
overflowList(45, { collapseFrom: "end" }).assertOverflowItems(4, 5);
});

describe("onOverflow", () => {
it("invoked on initial render if has overflow", async () => {
const list = await overflowList(22).waitForResize();
list.assertLastOnOverflowArgs([0, 1, 2, 3]);
});

it("not invoked on initial render if all visible", async () => {
const list = await overflowList(200).waitForResize();
assert.isTrue(list.onOverflow.notCalled, "not called");
});

it("invoked once per resize", async () => {
// initial render shows all items (empty overflow)
const list = await overflowList(200).waitForResize();
// assert that at given width, onOverflow receives given IDs
const tests = [
{ width: 15, overflowIds: [0, 1, 2, 3, 4] },
{ width: 55, overflowIds: [0] },
{ width: 25, overflowIds: [0, 1, 2, 3] },
{ width: 35, overflowIds: [0, 1, 2] },
];
for (const { overflowIds, width } of tests) {
(await list.setWidth(width).waitForResize()).assertLastOnOverflowArgs(overflowIds);
}
// ensure onOverflow is not called additional times.
assert.equal(list.onOverflow.callCount, tests.length, "should invoke once per resize");
});

it("not invoked if resize doesn't change overflow", async () => {
// show a few items
const list = await overflowList(22).waitForResize();
// small adjustments don't change overflow state, but it is recomputed internally.
// assert that the callback was not invoked because the appearance hasn't changed.
list.onOverflow.resetHistory();
await list.setWidth(25).waitForResize();
await list.setWidth(28).waitForResize();
await list.setWidth(29).waitForResize();
await list.setWidth(26).waitForResize();
await list.setWidth(22).waitForResize();
assert.isTrue(list.onOverflow.notCalled, "should not invoke");
});

it("invoked when items change", async () => {
const list = await overflowList(22).waitForResize();
// copy of same items so overflow state should end up the same.
await list.setProps({ items: [...ITEMS] }).waitForResize();
assert.isTrue(list.onOverflow.calledTwice, "should be called twice");
const [one, two] = list.onOverflow.args;
assert.sameDeepMembers(one, two, "items should be the same");
});
});

function renderOverflow(items: ITestItem[]) {
return <TestOverflow items={items} />;
}
Expand All @@ -95,9 +148,11 @@ describe("<OverflowList>", function(this) {
}

function overflowList(initialWidth = 45, props: Partial<OverflowProps> = {}) {
const onOverflow = spy();
const wrapper = mount<OverflowProps, IOverflowListState<ITestItem>>(
<OverflowList
items={ITEMS}
onOverflow={onOverflow}
overflowRenderer={renderOverflow}
visibleItemRenderer={renderVisibleItem}
style={{ width: initialWidth }}
Expand All @@ -113,22 +168,26 @@ describe("<OverflowList>", function(this) {
assert.equal(wrapper.find(TestOverflow).exists(), exists, "has overflow");
return harness;
},
/** Asserts that the last call to `onOverflow` received the given item IDs. */
assertLastOnOverflowArgs(ids: number[]) {
assert.sameMembers(onOverflow.lastCall.args[0].map((i: ITestItem) => i.id), ids);
},
/**
* Invokes both assertions below with the expected visible and
* overflow ids assuming `collapseFrom="start"`.
* overflow IDs assuming `collapseFrom="start"`.
*/
assertVisibleItemSplit(visibleCount: number) {
return harness
.assertOverflowItems(...IDS.slice(0, -visibleCount))
.assertVisibleItems(...IDS.slice(-visibleCount));
},
/** Assert ordered ids of overflow items. */
/** Assert ordered IDs of overflow items. */
assertOverflowItems(...ids: number[]) {
const overflowItems = wrapper.find(TestOverflow).prop("items");
assert.sameMembers(overflowItems.map(it => it.id), ids, "overflow items");
return harness;
},
/** Assert ordered ids of visible items. */
/** Assert ordered IDs of visible items. */
assertVisibleItems(...ids: number[]) {
const visibleItems = wrapper.find(TestItem).map(div => div.prop("id"));
assert.sameMembers(visibleItems, ids, "visible items");
Expand All @@ -150,6 +209,8 @@ describe("<OverflowList>", function(this) {
}, 30),
);
},

onOverflow,
wrapper,
};
return harness;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,10 @@ const COLLAPSE_FROM_RADIOS = [
const ITEMS: IMenuItemProps[] = [
{ href: "#", icon: "folder-close", text: "All files" },
{ href: "#", icon: "folder-close", text: "Users" },
{ href: "#", icon: "folder-close", text: "Jane Person" },
{ href: "#", icon: "folder-close", text: "My documents" },
{ href: "#", icon: "folder-close", text: "Classy dayjob" },
{ icon: "document", text: "How to crush it" },
{ href: "#", icon: "folder-close", text: "Janet" },
{ href: "#", icon: "folder-close", text: "Photos" },
{ href: "#", icon: "folder-close", text: "Wednesday" },
{ icon: "document", text: "image.jpg" },
];

export class OverflowListExample extends React.PureComponent<IExampleProps, IOverflowListExampleState> {
Expand Down