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] Create-able Select components (a different approach) #3381

Merged
merged 32 commits into from
Feb 28, 2019
Merged
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
6668d07
Add option to create new item
Jan 21, 2019
3fcd343
Properly clear input on item click
Jan 21, 2019
b42418f
Move props to queryList
Jan 21, 2019
2548a01
Lint and test
Jan 21, 2019
bccb5e2
Lint and docs
Jan 21, 2019
684919c
Properly handle keyboard events with create item
shuyangli Feb 6, 2019
ff20ae6
Merge branch 'develop' into sl/1710-create-able-multiselect
shuyangli Feb 6, 2019
6964bbb
Minor cleanup
shuyangli Feb 6, 2019
4596f78
Factor out getActiveItem
shuyangli Feb 6, 2019
01a1538
createItem => createNewItem
Feb 20, 2019
d99d570
Fix existing tests
Feb 20, 2019
6b676d5
Use real item list in filtering test
Feb 20, 2019
2aa4b54
More type-safe active item getter
Feb 20, 2019
321cd03
Add tests for interacting with create item
Feb 20, 2019
4c47f03
Add docs for creating items
Feb 20, 2019
59a86aa
Merge branch 'develop' into sl/1710-create-able-multiselect
Feb 20, 2019
f54513f
Try a non-breaking API that adds a new generic type C
cmslewis Feb 26, 2019
cde4086
Actually, just create a reserved type with a __brand, and incorporate…
cmslewis Feb 26, 2019
4ea4706
Update examples
cmslewis Feb 26, 2019
f86db48
Update tests
cmslewis Feb 26, 2019
dfbcedd
Update function comments
cmslewis Feb 26, 2019
16b9fe5
Avoid breaking the API, fix tests
cmslewis Feb 26, 2019
d1fd4fe
Update docs
cmslewis Feb 26, 2019
2925fec
Make the brand even more specific, and add a deep-comparison check
cmslewis Feb 26, 2019
3e6d62f
Fix unintentional code changes
cmslewis Feb 26, 2019
015e06f
Fix lint in unrelated files?
cmslewis Feb 27, 2019
6ecdfa0
Respond to @adidahiya CR
cmslewis Feb 27, 2019
631b86e
Fix examples: Add created item to list of items (to enable deselection)
cmslewis Feb 28, 2019
3e020ed
Hide 'Create item' option if query already appears in items
cmslewis Feb 28, 2019
f126a9d
Write tests
cmslewis Feb 28, 2019
1f7b3bb
Share created-item logic in Select and Suggest components too
cmslewis Feb 28, 2019
b9e363f
Small optimization: compare created item to filteredItems, not items
cmslewis Feb 28, 2019
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
8 changes: 6 additions & 2 deletions packages/docs-app/src/components/resources.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,15 @@ const RESOURCES: IResourceProps[] = [
export const Resources: React.SFC = () => (
<>
<div className="blueprint-resources">
{RESOURCES.map(resource => <ResourceCard key={resource.fileName} {...resource} />)}
{RESOURCES.map(resource => (
<ResourceCard key={resource.fileName} {...resource} />
))}
</div>
<Callout title="Missing fonts?" intent="warning">
Download Apple's San Francisco font directly from the source:{" "}
<a href="https://developer.apple.com/fonts/" target="_blank" rel="noopener noreferrer">https://developer.apple.com/fonts/</a>
<a href="https://developer.apple.com/fonts/" target="_blank" rel="noopener noreferrer">
https://developer.apple.com/fonts/
</a>
</Callout>
</>
);
Expand Down
27 changes: 27 additions & 0 deletions packages/docs-app/src/examples/select-examples/films.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,20 @@ export const renderFilm: ItemRenderer<IFilm> = (film, { handleClick, modifiers,
);
};

export const renderCreateFilmOption = (
query: string,
active: boolean,
handleClick: React.MouseEventHandler<HTMLElement>,
) => (
<MenuItem
icon="add"
text={`Create "${query}"`}
active={active}
onClick={handleClick}
shouldDismissPopover={false}
/>
);

export const filterFilm: ItemPredicate<IFilm> = (query, film) => {
return `${film.rank}. ${film.title.toLowerCase()} ${film.year}`.indexOf(query.toLowerCase()) >= 0;
};
Expand Down Expand Up @@ -182,3 +196,16 @@ export const filmSelectProps = {
itemRenderer: renderFilm,
items: TOP_100_FILMS,
};

export function createFilm(title: string): IFilm {
return {
rank: 100 + Math.floor(Math.random() * 100 + 1),
title,
year: new Date().getFullYear(),
};
}

export function areFilmsEqual(filmA: IFilm, filmB: IFilm) {
// Compare only the titles (ignoring case) just for simplicity.
return filmA.title.toLowerCase() === filmB.title.toLowerCase();
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,19 @@ import * as React from "react";
import { Button, H5, Intent, ITagProps, MenuItem, Switch } from "@blueprintjs/core";
import { Example, IExampleProps } from "@blueprintjs/docs-theme";
import { ItemRenderer, MultiSelect } from "@blueprintjs/select";
import { filmSelectProps, IFilm, TOP_100_FILMS } from "./films";
import { areFilmsEqual, createFilm, filmSelectProps, IFilm, renderCreateFilmOption, TOP_100_FILMS } from "./films";

const FilmMultiSelect = MultiSelect.ofType<IFilm>();

const INTENTS = [Intent.NONE, Intent.PRIMARY, Intent.SUCCESS, Intent.DANGER, Intent.WARNING];

export interface IMultiSelectExampleState {
allowCreate: boolean;
createdItems: IFilm[];
films: IFilm[];
hasInitialContent: boolean;
intent: boolean;
items: IFilm[];
openOnKeyDown: boolean;
popoverMinimal: boolean;
resetOnSelect: boolean;
Expand All @@ -27,15 +30,19 @@ export interface IMultiSelectExampleState {

export class MultiSelectExample extends React.PureComponent<IExampleProps, IMultiSelectExampleState> {
public state: IMultiSelectExampleState = {
allowCreate: false,
createdItems: [],
films: [],
hasInitialContent: false,
intent: false,
items: filmSelectProps.items,
openOnKeyDown: false,
popoverMinimal: true,
resetOnSelect: true,
tagMinimal: false,
};

private handleAllowCreateChange = this.handleSwitchChange("allowCreate");
private handleKeyDownChange = this.handleSwitchChange("openOnKeyDown");
private handleResetChange = this.handleSwitchChange("resetOnSelect");
private handlePopoverMinimalChange = this.handleSwitchChange("popoverMinimal");
Expand All @@ -44,7 +51,7 @@ export class MultiSelectExample extends React.PureComponent<IExampleProps, IMult
private handleInitialContentChange = this.handleSwitchChange("hasInitialContent");

public render() {
const { films, hasInitialContent, tagMinimal, popoverMinimal, ...flags } = this.state;
const { allowCreate, films, hasInitialContent, tagMinimal, popoverMinimal, ...flags } = this.state;
const getTagProps = (_value: string, index: number): ITagProps => ({
intent: this.state.intent ? INTENTS[index % INTENTS.length] : Intent.NONE,
minimal: tagMinimal,
Expand All @@ -56,6 +63,8 @@ export class MultiSelectExample extends React.PureComponent<IExampleProps, IMult
// explicit undefined (not null) for default behavior (show full list)
undefined
);
const maybeCreateNewItemFromQuery = allowCreate ? createFilm : undefined;
const maybeCreateNewItemRenderer = allowCreate ? renderCreateFilmOption : null;

const clearButton = films.length > 0 ? <Button icon="cross" minimal={true} onClick={this.handleClear} /> : null;

Expand All @@ -64,8 +73,11 @@ export class MultiSelectExample extends React.PureComponent<IExampleProps, IMult
<FilmMultiSelect
{...filmSelectProps}
{...flags}
createNewItemFromQuery={maybeCreateNewItemFromQuery}
createNewItemRenderer={maybeCreateNewItemRenderer}
initialContent={initialContent}
itemRenderer={this.renderFilm}
itemsEqual={areFilmsEqual}
noResults={<MenuItem disabled={true} text="No results." />}
onItemSelect={this.handleFilmSelect}
popoverProps={{ minimal: popoverMinimal }}
Expand Down Expand Up @@ -96,6 +108,11 @@ export class MultiSelectExample extends React.PureComponent<IExampleProps, IMult
checked={this.state.hasInitialContent}
onChange={this.handleInitialContentChange}
/>
<Switch
label="Allow creating new films"
checked={this.state.allowCreate}
onChange={this.handleAllowCreateChange}
/>
<H5>Tag props</H5>
<Switch
label="Minimal tag style"
Expand Down Expand Up @@ -150,11 +167,31 @@ export class MultiSelectExample extends React.PureComponent<IExampleProps, IMult
}

private selectFilm(film: IFilm) {
this.setState({ films: [...this.state.films, film] });
const { createdItems, films, items } = this.state;
const isNewlyCreatedItem = !this.arrayContainsFilm(items, film);
this.setState({
createdItems: isNewlyCreatedItem ? this.addFilmToArray(createdItems, film) : createdItems,
// Avoid re-creating an item that is already selected (the "Create
// Item" option will be shown even if it matches an already selected
// item).
films: !this.arrayContainsFilm(films, film) ? [...films, film] : films,
// Add a created film to `items` so that the film can be deselected.
items: isNewlyCreatedItem ? this.addFilmToArray(items, film) : items,
});
}

private deselectFilm(index: number) {
this.setState({ films: this.state.films.filter((_film, i) => i !== index) });
const { createdItems, films, items } = this.state;

const film = films[index];
const wasItemCreatedByUser = this.arrayContainsFilm(createdItems, film);

// Delete the item if the user manually created it.
this.setState({
createdItems: wasItemCreatedByUser ? this.deleteFilmFromArray(createdItems, film) : createdItems,
films: films.filter((_film, i) => i !== index),
items: wasItemCreatedByUser ? this.deleteFilmFromArray(items, film) : items,
});
}

private handleFilmSelect = (film: IFilm) => {
Expand All @@ -173,4 +210,16 @@ export class MultiSelectExample extends React.PureComponent<IExampleProps, IMult
}

private handleClear = () => this.setState({ films: [] });

private arrayContainsFilm(films: IFilm[], filmToFind: IFilm): boolean {
return films.find(f => f.title === filmToFind.title) !== undefined;
}

private addFilmToArray(films: IFilm[], filmToAdd: IFilm) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This reconciliation logic now falls on the users—is it possible to keep this inside the multiselect component? i.e. wrap the passed-in onItemSelect and move the logic for maintaining the union of {passed-in items, newly-created items}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@shuyangli It's a good thought, but it seems like the component should remain as data-agnostic as possible. Tracking created items would violate that. @adidahiya what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In particular, a major blocker for doing that is that if a user passes in a totally different set of items, wiping away the ones from before, then our components won't have any way to know if they should clear their internally tracked list of createdItems (which would be stored in this.state, presumably). The mutually dependent props.items and state.createdItems could easily get out of sync and cause more problems than they solve.

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah I agree with @cmslewis here, I would prefer to do less data handling in the component and keep it more presentation-focused. I know it's pretty verbose for consumers this way, but we can always add additional conveniences on top of this approach later on after we see concrete use cases of this feature

return [...films, filmToAdd];
}

private deleteFilmFromArray(films: IFilm[], filmToDelete: IFilm) {
return films.filter(film => film !== filmToDelete);
}
}
34 changes: 26 additions & 8 deletions packages/docs-app/src/examples/select-examples/omnibarExample.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,22 +20,25 @@ import {
} from "@blueprintjs/core";
import { Example, handleBooleanChange, IExampleProps } from "@blueprintjs/docs-theme";
import { Omnibar } from "@blueprintjs/select";
import { filmSelectProps, IFilm } from "./films";
import { areFilmsEqual, createFilm, filmSelectProps, IFilm, renderCreateFilmOption } from "./films";

const FilmOmnibar = Omnibar.ofType<IFilm>();

export interface IOmnibarExampleState {
allowCreate: boolean;
isOpen: boolean;
resetOnSelect: boolean;
}

@HotkeysTarget
export class OmnibarExample extends React.PureComponent<IExampleProps, IOmnibarExampleState> {
public state: IOmnibarExampleState = {
allowCreate: false,
isOpen: false,
resetOnSelect: true,
};

private handleAllowCreateChange = handleBooleanChange(allowCreate => this.setState({ allowCreate }));
private handleResetChange = handleBooleanChange(resetOnSelect => this.setState({ resetOnSelect }));

private toaster: Toaster;
Expand All @@ -59,15 +62,13 @@ export class OmnibarExample extends React.PureComponent<IExampleProps, IOmnibarE
}

public render() {
const options = (
<>
<H5>Props</H5>
<Switch label="Reset on select" checked={this.state.resetOnSelect} onChange={this.handleResetChange} />
</>
);
const { allowCreate } = this.state;

const maybeCreateNewItemFromQuery = allowCreate ? createFilm : undefined;
const maybeCreateNewItemRenderer = allowCreate ? renderCreateFilmOption : null;

return (
<Example options={options} {...this.props}>
<Example options={this.renderOptions()} {...this.props}>
<span>
<Button text="Click to show Omnibar" onClick={this.handleClick} />
{" or press "}
Expand All @@ -77,6 +78,9 @@ export class OmnibarExample extends React.PureComponent<IExampleProps, IOmnibarE
<FilmOmnibar
{...filmSelectProps}
{...this.state}
createNewItemFromQuery={maybeCreateNewItemFromQuery}
createNewItemRenderer={maybeCreateNewItemRenderer}
itemsEqual={areFilmsEqual}
noResults={<MenuItem disabled={true} text="No results." />}
onItemSelect={this.handleItemSelect}
onClose={this.handleClose}
Expand All @@ -86,6 +90,20 @@ export class OmnibarExample extends React.PureComponent<IExampleProps, IOmnibarE
);
}

protected renderOptions() {
return (
<>
<H5>Props</H5>
<Switch label="Reset on select" checked={this.state.resetOnSelect} onChange={this.handleResetChange} />
<Switch
label="Allow creating new films"
checked={this.state.allowCreate}
onChange={this.handleAllowCreateChange}
/>
</>
);
}

private handleClick = (_event: React.MouseEvent<HTMLElement>) => {
this.setState({ isOpen: true });
};
Expand Down
17 changes: 15 additions & 2 deletions packages/docs-app/src/examples/select-examples/selectExample.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,12 @@ import * as React from "react";
import { Button, H5, MenuItem, Switch } from "@blueprintjs/core";
import { Example, IExampleProps } from "@blueprintjs/docs-theme";
import { Select } from "@blueprintjs/select";
import { filmSelectProps, IFilm, TOP_100_FILMS } from "./films";
import { areFilmsEqual, createFilm, filmSelectProps, IFilm, renderCreateFilmOption, TOP_100_FILMS } from "./films";

const FilmSelect = Select.ofType<IFilm>();

export interface ISelectExampleState {
allowCreate: boolean;
film: IFilm;
filterable: boolean;
hasInitialContent: boolean;
Expand All @@ -27,6 +28,7 @@ export interface ISelectExampleState {

export class SelectExample extends React.PureComponent<IExampleProps, ISelectExampleState> {
public state: ISelectExampleState = {
allowCreate: false,
disableItems: false,
disabled: false,
film: TOP_100_FILMS[0],
Expand All @@ -38,6 +40,7 @@ export class SelectExample extends React.PureComponent<IExampleProps, ISelectExa
resetOnSelect: false,
};

private handleAllowCreateChange = this.handleSwitchChange("allowCreate");
private handleDisabledChange = this.handleSwitchChange("disabled");
private handleFilterableChange = this.handleSwitchChange("filterable");
private handleInitialContentChange = this.handleSwitchChange("hasInitialContent");
Expand All @@ -48,21 +51,26 @@ export class SelectExample extends React.PureComponent<IExampleProps, ISelectExa
private handleResetOnSelectChange = this.handleSwitchChange("resetOnSelect");

public render() {
const { disabled, disableItems, film, minimal, ...flags } = this.state;
const { allowCreate, disabled, disableItems, film, minimal, ...flags } = this.state;

const initialContent = this.state.hasInitialContent ? (
<MenuItem disabled={true} text={`${TOP_100_FILMS.length} items loaded.`} />
) : (
undefined
);
const maybeCreateNewItemFromQuery = allowCreate ? createFilm : undefined;
const maybeCreateNewItemRenderer = allowCreate ? renderCreateFilmOption : null;

return (
<Example options={this.renderOptions()} {...this.props}>
<FilmSelect
{...filmSelectProps}
{...flags}
createNewItemFromQuery={maybeCreateNewItemFromQuery}
createNewItemRenderer={maybeCreateNewItemRenderer}
disabled={disabled}
itemDisabled={this.isItemDisabled}
itemsEqual={areFilmsEqual}
initialContent={initialContent}
noResults={<MenuItem disabled={true} text="No results." />}
onItemSelect={this.handleValueChange}
Expand Down Expand Up @@ -110,6 +118,11 @@ export class SelectExample extends React.PureComponent<IExampleProps, ISelectExa
checked={this.state.disableItems}
onChange={this.handleItemDisabledChange}
/>
<Switch
label="Allow creating new items"
checked={this.state.allowCreate}
onChange={this.handleAllowCreateChange}
/>
<H5>Popover props</H5>
<Switch
label="Minimal popover style"
Expand Down
Loading