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

[select] feat(Suggest): sync activeItem with selectedItem on popover close #3934

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 22 additions & 16 deletions packages/select/src/components/query-list/queryList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@ import {
} from "../../common";

export interface IQueryListProps<T> extends IListItemsProps<T> {
/**
* 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
Expand Down Expand Up @@ -171,7 +177,7 @@ export class QueryList<T> extends AbstractComponent2<IQueryListProps<T>, IQueryL
activeItem:
props.activeItem !== undefined
? props.activeItem
: getFirstEnabledItem(filteredItems, props.itemDisabled),
: props.initialActiveItem ?? getFirstEnabledItem(filteredItems, props.itemDisabled),
createNewItem,
filteredItems,
query,
Expand Down Expand Up @@ -289,6 +295,21 @@ export class QueryList<T> extends AbstractComponent2<IQueryListProps<T>, 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<T>) => {
const { initialContent, noResults } = this.props;
Expand Down Expand Up @@ -486,21 +507,6 @@ export class QueryList<T> extends AbstractComponent2<IQueryListProps<T>, 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() &&
Expand Down
19 changes: 18 additions & 1 deletion packages/select/src/components/select/suggest.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,10 @@ export class Suggest<T> extends React.PureComponent<ISuggestProps<T>, ISuggestSt
public render() {
// omit props specific to this component, spread the rest.
const { disabled, inputProps, popoverProps, ...restProps } = this.props;

return (
<this.TypedQueryList
{...restProps}
initialActiveItem={this.props.selectedItem ?? undefined}
onItemSelect={this.handleItemSelect}
ref={this.refHandlers.queryList}
renderer={this.renderQueryList}
Expand All @@ -142,6 +142,14 @@ export class Suggest<T> extends React.PureComponent<ISuggestProps<T>, 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();
}
Expand Down Expand Up @@ -320,4 +328,13 @@ export class Suggest<T> extends React.PureComponent<ISuggestProps<T>, 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);
}
}
}
31 changes: 30 additions & 1 deletion packages/select/test/suggestTests.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down Expand Up @@ -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<IFilm>) as any).queryList as QueryList<IFilm>; // 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<any, any>, which: number) {
simulateKeyDown(wrapper, which);
assert.isFalse(wrapper.state().isOpen, "should not open popover");
Expand Down