From 8f86ea969e09dd1c8175271a5bb74f73fc6e617c Mon Sep 17 00:00:00 2001 From: Gilad Gray Date: Thu, 4 Oct 2018 12:38:08 -0700 Subject: [PATCH] [OverflowList] add onOverflow callback prop (#2975) * complete refactor to tests with a sweet harness * add onOverflow callback prop & isResizing state & test. * short example crumbs so they can all be visible * only invoke onOverflow when items change added some state fields to support this logic. complex stuff as it can happen over several renders. * more onOverflow tests & helper * more docs, combine if * reset lastOverflow with new props * simplify lastOverflow check - just length * lastOverflowCount defaults to 0 * fix name in comment --- .../components/overflow-list/overflowList.tsx | 52 +++++++++++++- .../test/overflow-list/overflowListTests.tsx | 67 ++++++++++++++++++- .../core-examples/overflowListExample.tsx | 8 +-- 3 files changed, 117 insertions(+), 10 deletions(-) diff --git a/packages/core/src/components/overflow-list/overflowList.tsx b/packages/core/src/components/overflow-list/overflowList.tsx index a785b1dd3b..9be5e9b638 100644 --- a/packages/core/src/components/overflow-list/overflowList.tsx +++ b/packages/core/src/components/overflow-list/overflowList.tsx @@ -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 */ +export enum OverflowDirection { + NONE, + GROW, + SHRINK, +} + export interface IOverflowListProps extends IProps { /** * Which direction the items should collapse from: start or end of the @@ -47,6 +55,13 @@ export interface IOverflowListProps 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 @@ -68,6 +83,13 @@ export interface IOverflowListProps extends IProps { } export interface IOverflowListState { + /** + * 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[]; } @@ -85,6 +107,8 @@ export class OverflowList extends React.PureComponent, } public state: IOverflowListState = { + direction: OverflowDirection.NONE, + lastOverflowCount: 0, overflow: [], visible: this.props.items, }; @@ -118,14 +142,25 @@ export class OverflowList extends React.PureComponent, ) { // reset visible state if the above props change. this.setState({ + direction: OverflowDirection.GROW, + lastOverflowCount: 0, overflow: [], visible: nextProps.items, }); } } - public componentDidUpdate() { + public componentDidUpdate(_prevProps: IOverflowListProps, prevState: IOverflowListState) { this.repartition(false); + 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() { @@ -166,10 +201,14 @@ export class OverflowList extends React.PureComponent, 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 => { @@ -184,10 +223,17 @@ export class OverflowList extends React.PureComponent, } const overflow = collapseFromStart ? [...state.overflow, next] : [next, ...state.overflow]; return { + // set SHRINK mode unless a GROW is already in progress. + // 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 }); } } } diff --git a/packages/core/test/overflow-list/overflowListTests.tsx b/packages/core/test/overflow-list/overflowListTests.tsx index 79ece99dde..8938852247 100644 --- a/packages/core/test/overflow-list/overflowListTests.tsx +++ b/packages/core/test/overflow-list/overflowListTests.tsx @@ -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; @@ -86,6 +87,58 @@ describe("", 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 ; } @@ -95,9 +148,11 @@ describe("", function(this) { } function overflowList(initialWidth = 45, props: Partial = {}) { + const onOverflow = spy(); const wrapper = mount>( ", 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"); @@ -150,6 +209,8 @@ describe("", function(this) { }, 30), ); }, + + onOverflow, wrapper, }; return harness; diff --git a/packages/docs-app/src/examples/core-examples/overflowListExample.tsx b/packages/docs-app/src/examples/core-examples/overflowListExample.tsx index 3538c08edb..62b51ba0a2 100644 --- a/packages/docs-app/src/examples/core-examples/overflowListExample.tsx +++ b/packages/docs-app/src/examples/core-examples/overflowListExample.tsx @@ -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 {