[Labs] new TagInput component! #1232

giladgray · 2017-06-14T00:23:13Z

Addresses #118, #1064

Changes proposed in this pull request:

TagInput renders Tags inside an input, followed by an actual text input. The container is merely styled to look like a Blueprint input; the actual editable element appears after the last tag. Clicking anywhere on the container will focus the text input for seamless interaction.

Reviewers should focus on:

API design. onAdd and onRemove seem sufficient in my testing--i was able to implement a basic multi-select with little trouble.

Screenshot

blueprint-bot · 2017-06-14T00:34:19Z

move code to subdirectory, use "fake" names

Preview: documentation
Coverage: core | datetime

giladgray · 2017-06-14T00:23:52Z

packages/core/src/components/tag/tag.tsx

    /**
     * Click handler for remove button.
     * Button will only be rendered if this prop is defined.
     */
-    onRemove?: (e: React.MouseEvent<HTMLSpanElement>) => void;
+    onRemove?: (e: React.MouseEvent<HTMLButtonElement>) => void;


previous type was incorrect, as the onClick is applied to a <button>

giladgray · 2017-06-14T00:24:20Z

packages/core/src/components/tag/tag.tsx

@@ -14,12 +14,12 @@ import { isFunction } from "../../common/utils";

 import * as Classes from "../../common/classes";

-export interface ITagProps extends IProps, IIntentProps, React.HTMLProps<HTMLSpanElement> {
+export interface ITagProps extends IProps, IIntentProps, React.HTMLAttributes<Tag> {


💥 HTMLAttributes does not include ref so it avoids the nasty { ref, ...htmlInputProps } spread we've had to do! i'll go refactor other usages separately.

cmslewis · 2017-06-14T20:41:37Z

Stress testing (not much we should do to mitigate this maybe, just FYI):

cmslewis

Looks and feels great overall. Just have a couple questions.

cmslewis · 2017-06-14T20:57:58Z

packages/labs/src/tagInput/tagInput.tsx

+        input: (ref: HTMLInputElement) => {
+            this.inputElement = ref;
+            const refHandler = this.props.inputProps.ref;
+            if (Utils.isFunction(refHandler)) {


Can we use Utils.safeInvoke?

great Q!

typings problems with safeInvoke: refHandler is typed string | (ref) => void so i must use isFunction for the type guard. (note that this is exactly what safeInvoke does inside, but it expects functions. I think we can relax those types and maybe also make it a type guard?)

seems there's not much to be done to improve safeInvoke usage. if i make it accept non-functions then it loses all type safety :(. adding code comment instead.

cmslewis · 2017-06-14T21:01:55Z

packages/labs/src/tagInput/tagInput.tsx

+
+    private handleContainerClick = () => {
+        if (this.inputElement != null) {
+            this.inputElement.focus();


If I'm blurred and then click a tag's remove button, is it still expected that the input focus?

yes cuz you've clicked in the .pt-input (the thing that looks like a text input)

cmslewis · 2017-06-14T21:03:19Z

packages/labs/src/tagInput/tagInput.tsx

+    private handleInputKeyDown = (event: React.KeyboardEvent<HTMLInputElement>) => {
+        const { value } = event.currentTarget;
+        if (event.which === Keys.ENTER && value.length > 0) {
+            Utils.safeInvoke(this.props.onAdd, value);


Elsewhere, we setState before invoking the callback. Is the switch here intentional?

cmslewis · 2017-06-14T21:04:42Z

packages/labs/src/tagInput/tagInput.tsx

+    }
+
+    private handleRemoveTag = (event: React.MouseEvent<HTMLSpanElement>) => {
+        // using data attribute to simplify callback logic -- one handler for all children


… just last)

llorca · 2017-06-14T22:19:50Z

Note: aware that .pt-tag.pt-active isn't great visually, I'll push better styles for it later

blueprint-bot · 2017-06-14T22:54:58Z

fix lint

Preview: documentation | table
Coverage: core | datetime

giladgray · 2017-06-15T17:23:32Z

packages/labs/src/tagInput/tagInput.tsx

+        input: (ref: HTMLInputElement) => {
+            this.inputElement = ref;
+            const refHandler = this.props.inputProps.ref;
+            if (Utils.isFunction(refHandler)) {


great Q!

typings problems with safeInvoke: refHandler is typed string | (ref) => void so i must use isFunction for the type guard. (note that this is exactly what safeInvoke does inside, but it expects functions. I think we can relax those types and maybe also make it a type guard?)

giladgray · 2017-06-15T17:25:02Z

packages/labs/src/tagInput/tagInput.tsx

+    public render() {
+        const { inputProps, tagProps, values } = this.props;
+
+        const classes = classNames(Classes.INPUT, "pt-tag-input", {


note: no constant

giladgray · 2017-06-15T17:25:48Z

packages/core/src/components/tag/_common.scss

+    //   .pt-dark & {
+    //     background-color: rgba($gray5, 0.2);
+    //   }
+    // }


giladgray · 2017-06-15T17:27:51Z

packages/labs/src/tagInput/tagInput.tsx

+     * or a function to customize props per tag.
+     *
+     * If you define `onRemove` here then you will have to implement your own tag removal
+     * handling as the `TagInput` `onRemove` prop will never be invoked.


...TagInput's own handler will never...

giladgray · 2017-06-15T17:28:01Z

packages/labs/src/tagInput/tagInput.tsx

+    };
+
+    public state: ITagInputState = {
+        activeIndex: -1,


const NONE

giladgray · 2017-06-15T17:28:44Z

packages/labs/src/tagInput/tagInput.tsx

+
+    private handleContainerClick = () => {
+        if (this.inputElement != null) {
+            this.inputElement.focus();


yes cuz you've clicked in the .pt-input (the thing that looks like a text input)

blueprint-bot · 2017-06-15T18:43:52Z

🌳 remove logs

Preview: documentation | table
Coverage: core | datetime

blueprint-bot · 2017-06-15T22:50:03Z

add docs about keyboard interactions

Preview: documentation
Coverage: core | datetime

Gilad Gray added 3 commits June 13, 2017 14:17

initial TagInput implementation

1435bb6

docs + polish

48f7c71

tests! sufficient coverage 👍

c9d67b4

giladgray requested a review from adidahiya June 14, 2017 00:23

move code to subdirectory, use "fake" names

7bebcd9

giladgray commented Jun 14, 2017

View reviewed changes

cmslewis approved these changes Jun 14, 2017

View reviewed changes

Gilad Gray added 2 commits June 14, 2017 15:15

add Tag active prop and .pt-active style support

0de6fef

left/right arrows to navigate through tags list and remove _any_ (not…

c6545df

… just last)

fix lint

5166bc1

Gilad Gray added 2 commits June 15, 2017 11:26

refactor keyboard logic and introduce NONE constant

41a9e1f

set explicit width to break long words

e5ccb76

giladgray commented Jun 15, 2017

View reviewed changes

Gilad Gray added 2 commits June 15, 2017 11:31

notes, remove commented code

f219fda

🌳 remove logs

7275e31

Gilad Gray added 3 commits June 15, 2017 15:37

Merge branch 'master' of github.com:palantir/blueprint into gg/taginput

6e74699

rename dir to tag-input

960f305

add docs about keyboard interactions

d441a73

llorca merged commit abb4f5d into master Jun 16, 2017

llorca deleted the gg/taginput branch June 16, 2017 23:58

cmslewis mentioned this pull request Jun 22, 2017

Prepare Release 1.21.0 #1270

Merged

This was referenced Sep 15, 2021

[Snyk] Security upgrade webpack-cli from 3.3.12 to 4.0.0 majacQ/blueprint#48

Open

[Snyk] Security upgrade webpack-cli from 3.3.12 to 4.0.0 majacQ/blueprint#49

Open

[Snyk] Security upgrade webpack-cli from 3.3.12 to 4.0.0 majacQ/blueprint#50

Open

snyk-bot mentioned this pull request Dec 25, 2022

[Snyk] Security upgrade webpack-cli from 3.3.12 to 4.0.0 majacQ/blueprint#192

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Labs] new TagInput component! #1232

[Labs] new TagInput component! #1232

giladgray commented Jun 14, 2017

blueprint-bot commented Jun 14, 2017

giladgray Jun 14, 2017

giladgray Jun 14, 2017

cmslewis commented Jun 14, 2017

cmslewis left a comment

cmslewis Jun 14, 2017

giladgray Jun 15, 2017

giladgray Jun 15, 2017

cmslewis Jun 14, 2017

giladgray Jun 15, 2017

cmslewis Jun 14, 2017

cmslewis Jun 14, 2017

llorca commented Jun 14, 2017

blueprint-bot commented Jun 14, 2017

giladgray Jun 15, 2017

giladgray Jun 15, 2017

giladgray Jun 15, 2017

giladgray Jun 15, 2017

giladgray Jun 15, 2017

giladgray Jun 15, 2017

blueprint-bot commented Jun 15, 2017

blueprint-bot commented Jun 15, 2017

[Labs] new TagInput component! #1232

[Labs] new TagInput component! #1232

Conversation

giladgray commented Jun 14, 2017

Addresses #118, #1064

Changes proposed in this pull request:

Reviewers should focus on:

Screenshot

blueprint-bot commented Jun 14, 2017

move code to subdirectory, use "fake" names

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cmslewis commented Jun 14, 2017

cmslewis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

llorca commented Jun 14, 2017

blueprint-bot commented Jun 14, 2017

fix lint

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blueprint-bot commented Jun 15, 2017

🌳 remove logs

blueprint-bot commented Jun 15, 2017

add docs about keyboard interactions