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

[QueryList] Initialize activeItem state correctly based on prop. #3290

Merged
merged 3 commits into from
Jan 18, 2019
Merged
Show file tree
Hide file tree
Changes from 2 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
9 changes: 8 additions & 1 deletion packages/select/src/components/query-list/queryList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,14 @@ export class QueryList<T> extends React.Component<IQueryListProps<T>, IQueryList
super(props, context);
const { query = "" } = this.props;
const filteredItems = getFilteredItems(query, this.props);
this.state = { activeItem: getFirstEnabledItem(filteredItems, this.props.itemDisabled), filteredItems, query };
this.state = {
activeItem:
props.activeItem !== undefined
giladgray marked this conversation as resolved.
Show resolved Hide resolved
? props.activeItem
: getFirstEnabledItem(filteredItems, this.props.itemDisabled),
filteredItems,
query,
};
}

public render() {
Expand Down
65 changes: 64 additions & 1 deletion packages/select/test/queryListTests.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,13 @@ import * as sinon from "sinon";
// this is an awkward import across the monorepo, but we'd rather not introduce a cyclical dependency or create another package
import { IQueryListProps } from "@blueprintjs/select";
import { IFilm, renderFilm, TOP_100_FILMS } from "../../docs-app/src/examples/select-examples/films";
import { IQueryListRendererProps, IQueryListState, ItemListPredicate, ItemListRenderer, QueryList } from "../src/index";
import {
IQueryListRendererProps,
IQueryListState,
ItemListPredicate,
ItemListRenderer,
QueryList
} from "../src/index";
giladgray marked this conversation as resolved.
Show resolved Hide resolved

describe("<QueryList>", () => {
const FilmQueryList = QueryList.ofType<IFilm>();
Expand Down Expand Up @@ -89,6 +95,8 @@ describe("<QueryList>", () => {
filmQueryList.setState({ query: "query" });
filmQueryList.setState({ activeItem: undefined });
assert.equal(testProps.onActiveItemChange.callCount, 0);

filmQueryList.unmount();
UselessPickles marked this conversation as resolved.
Show resolved Hide resolved
});

it("ensure onActiveItemChange is not called updating props and query doesn't change", () => {
Expand All @@ -102,6 +110,8 @@ describe("<QueryList>", () => {
const filmQueryList: ReactWrapper<IQueryListProps<IFilm>> = mount(<FilmQueryList {...props} />);
filmQueryList.setProps(props);
assert.equal(testProps.onActiveItemChange.callCount, 0);

filmQueryList.unmount();
});

it("ensure activeItem changes on query change", () => {
Expand All @@ -119,6 +129,59 @@ describe("<QueryList>", () => {
query: "123",
});
assert.deepEqual(filmQueryList.state().activeItem, TOP_100_FILMS[1]);

filmQueryList.unmount();
});
});

describe("activeItem state initialization", () => {
it("initializes to first filtered item when uncontrolled", () => {
const props: IQueryListProps<IFilm> = {
...testProps,
// Filter down to only item at index 11, so item at index 11 should be
// chosen as default activeItem
itemPredicate: (_query, item) => item === TOP_100_FILMS[11],
query: "123",
};

const filmQueryList: ReactWrapper<IQueryListProps<IFilm>, IQueryListState<IFilm>> = mount(
giladgray marked this conversation as resolved.
Show resolved Hide resolved
<FilmQueryList {...props} />,
);

assert(filmQueryList.state().activeItem === TOP_100_FILMS[11]);

giladgray marked this conversation as resolved.
Show resolved Hide resolved
filmQueryList.unmount();
});

it("initializes to controlled activeItem prop (non-null)", () => {
const props: IQueryListProps<IFilm> = {
...testProps,
// List is not filtered, and item at index 11 is explicitly chosen as activeItem
activeItem: TOP_100_FILMS[11],
};

const filmQueryList: ReactWrapper<IQueryListProps<IFilm>, IQueryListState<IFilm>> = mount(
<FilmQueryList {...props} />,
);

assert(filmQueryList.state().activeItem === TOP_100_FILMS[11]);

filmQueryList.unmount();
});

it("initializes to controlled activeItem prop (null)", () => {
const props: IQueryListProps<IFilm> = {
...testProps,
activeItem: null,
};

const filmQueryList: ReactWrapper<IQueryListProps<IFilm>, IQueryListState<IFilm>> = mount(
<FilmQueryList {...props} />,
);

assert(filmQueryList.state().activeItem === null);

filmQueryList.unmount();
giladgray marked this conversation as resolved.
Show resolved Hide resolved
});
});

Expand Down