From 9acbe322d5546a101a0af5ad7876db11c1459419 Mon Sep 17 00:00:00 2001 From: Trevor Mehard Date: Tue, 25 Feb 2020 20:27:08 -0500 Subject: [PATCH] [select] feat(Suggest): sync activeItem with selectedItem on popover close (#3934) --- .../src/components/query-list/queryList.tsx | 38 +++++++++++-------- .../select/src/components/select/suggest.tsx | 19 +++++++++- packages/select/test/suggestTests.tsx | 31 ++++++++++++++- 3 files changed, 70 insertions(+), 18 deletions(-) diff --git a/packages/select/src/components/query-list/queryList.tsx b/packages/select/src/components/query-list/queryList.tsx index 0f19dc094b..3be262da16 100644 --- a/packages/select/src/components/query-list/queryList.tsx +++ b/packages/select/src/components/query-list/queryList.tsx @@ -30,6 +30,12 @@ import { } from "../../common"; export interface IQueryListProps extends IListItemsProps { + /** + * Initial active item, useful if the parent component is controlling its selectedItem but + * not activeItem. + */ + initialActiveItem?: T; + /** * Callback invoked when user presses a key, after processing `QueryList`'s own key events * (up/down to navigate active item). This callback is passed to `renderer` and (along with @@ -171,7 +177,7 @@ export class QueryList extends AbstractComponent2, IQueryL activeItem: props.activeItem !== undefined ? props.activeItem - : getFirstEnabledItem(filteredItems, props.itemDisabled), + : props.initialActiveItem ?? getFirstEnabledItem(filteredItems, props.itemDisabled), createNewItem, filteredItems, query, @@ -289,6 +295,21 @@ export class QueryList extends AbstractComponent2, IQueryL } } + public setActiveItem(activeItem: T | ICreateNewItem | null) { + this.expectedNextActiveItem = activeItem; + if (this.props.activeItem === undefined) { + // indicate that the active item may need to be scrolled into view after update. + this.shouldCheckActiveItemInViewport = true; + this.setState({ activeItem }); + } + + if (isCreateNewItem(activeItem)) { + Utils.safeInvoke(this.props.onActiveItemChange, null, true); + } else { + Utils.safeInvoke(this.props.onActiveItemChange, activeItem, false); + } + } + /** default `itemListRenderer` implementation */ private renderItemList = (listProps: IItemListRendererProps) => { const { initialContent, noResults } = this.props; @@ -486,21 +507,6 @@ export class QueryList extends AbstractComponent2, IQueryL return getFirstEnabledItem(this.state.filteredItems, this.props.itemDisabled, direction, startIndex); } - private setActiveItem(activeItem: T | ICreateNewItem | null) { - this.expectedNextActiveItem = activeItem; - if (this.props.activeItem === undefined) { - // indicate that the active item may need to be scrolled into view after update. - this.shouldCheckActiveItemInViewport = true; - this.setState({ activeItem }); - } - - if (isCreateNewItem(activeItem)) { - Utils.safeInvoke(this.props.onActiveItemChange, null, true); - } else { - Utils.safeInvoke(this.props.onActiveItemChange, activeItem, false); - } - } - private isCreateItemRendered(): boolean { return ( this.canCreateItems() && diff --git a/packages/select/src/components/select/suggest.tsx b/packages/select/src/components/select/suggest.tsx index ef4eee906a..4217d574b8 100644 --- a/packages/select/src/components/select/suggest.tsx +++ b/packages/select/src/components/select/suggest.tsx @@ -125,10 +125,10 @@ export class Suggest extends React.PureComponent, ISuggestSt public render() { // omit props specific to this component, spread the rest. const { disabled, inputProps, popoverProps, ...restProps } = this.props; - return ( extends React.PureComponent, ISuggestSt this.setState({ selectedItem: this.props.selectedItem }); } + if (this.state.isOpen === false && prevState.isOpen === true) { + // just closed, likely by keyboard interaction + // wait until the transition ends so there isn't a flash of content in the popover + setTimeout(() => { + this.maybeResetActiveItemToSelectedItem(); + }, this.props.popoverProps?.transitionDuration ?? Popover.defaultProps.transitionDuration); + } + if (this.state.isOpen && !prevState.isOpen && this.queryList != null) { this.queryList.scrollActiveItemIntoView(); } @@ -320,4 +328,13 @@ export class Suggest extends React.PureComponent, ISuggestSt Utils.safeInvokeMember(this.props.inputProps, "onKeyUp", evt); }; }; + + private maybeResetActiveItemToSelectedItem() { + const shouldResetActiveItemToSelectedItem = + this.props.activeItem === undefined && this.state.selectedItem !== null && !this.props.resetOnSelect; + + if (this.queryList !== null && shouldResetActiveItemToSelectedItem) { + this.queryList.setActiveItem(this.props.selectedItem ?? this.state.selectedItem); + } + } } diff --git a/packages/select/test/suggestTests.tsx b/packages/select/test/suggestTests.tsx index bde12edff7..e754b3e317 100644 --- a/packages/select/test/suggestTests.tsx +++ b/packages/select/test/suggestTests.tsx @@ -22,7 +22,7 @@ import * as sinon from "sinon"; import { IFilm, renderFilm, TOP_100_FILMS } from "../../docs-app/src/examples/select-examples/films"; import { ISuggestProps, ISuggestState, Suggest } from "../src/components/select/suggest"; -import { IItemRendererProps } from "../src/index"; +import { IItemRendererProps, QueryList } from "../src/index"; import { selectComponentSuite } from "./selectComponentSuite"; describe("Suggest", () => { @@ -99,6 +99,35 @@ describe("Suggest", () => { assert.strictEqual(scrollActiveItemIntoViewSpy.callCount, 1, "should call scrollActiveItemIntoView"); }); + it("sets active item to the selected item when the popover is closed", done => { + // transition duration shorter than timeout below to ensure it's done + const wrapper = suggest({ selectedItem: TOP_100_FILMS[10], popoverProps: { transitionDuration: 5 } }); + const queryList = ((wrapper.instance() as Suggest) as any).queryList as QueryList; // private ref + + assert.deepEqual( + queryList.state.activeItem, + wrapper.state().selectedItem, + "QueryList activeItem should be set to the controlled selectedItem if prop is provided", + ); + + simulateFocus(wrapper); + assert.isTrue(wrapper.state().isOpen); + + const newActiveItem = TOP_100_FILMS[11]; + queryList.setActiveItem(newActiveItem); + assert.deepEqual(queryList.state.activeItem, newActiveItem); + + simulateKeyDown(wrapper, Keys.ESCAPE); + assert.isFalse(wrapper.state().isOpen); + + wrapper.update(); + wrapper.find(QueryList).update(); + setTimeout(() => { + assert.deepEqual(queryList.state.activeItem, wrapper.state().selectedItem); + done(); + }, 10); + }); + function checkKeyDownDoesNotOpenPopover(wrapper: ReactWrapper, which: number) { simulateKeyDown(wrapper, which); assert.isFalse(wrapper.state().isOpen, "should not open popover");