FormTokenField: add ability to auto select first matching suggestion for incomplete token #42527

dinhtungdu · 2022-07-19T10:26:32Z

What?

This PR adds a new experimental prop to <FormTokenField> allows users to select the first matching suggestion for an incomplete token by pressing the Enter key. This is meant to use in tandem with __experimentalValidateInput.

Why?

This is useful when <FormTokenField> is used as a dropdown selector. Users don't need to type the full value of the suggestion to select it with the Enter key.

For example, given a component with ['blue', 'red', 'green'] as the suggestion array, users can select blue by typing b or bl only and then press Enter.

How?

Testing Instructions

In the storybook, navigate to FormTokenField > Dropdown Selector story.
Type an then press Enter key.
Antarctica should be selected.

Screenshots or screencast

~~https://user-images.githubusercontent.com/5423135/179729142-c424170b-4eaf-4904-931c-2687633f1413.mov~~

Updated screencast

Screen.Recording.2022-07-25.at.09.47.14.mov

Changelog

FormTokenField: add __experimentalAutoSelectFirstMatch prop to auto select the first matching suggestion on typing.

…token

ciampo

Hey @dinhtungdu , thank you for opening this PR!

My main worry is about the UX — the user has no (visual) indication that the first matching suggestion is going to be automatically selected even if the user hasn't selected it explicitly.

Is there a particular reason for adding this functionality?

ciampo · 2022-07-20T10:44:13Z

packages/components/src/form-token-field/index.tsx

+		if (
+			matchingSuggestions.length > 0 &&
+			incompleteTokenValue.length > 0 &&
+			typeof __experimentalValidateInput !== 'undefined' &&


The __experimentalValidateInput prop is assigned a default value of () => true, and therefore this check will always be true.

I also don't understand the rationale for tying this new functionality to the __experimentalValidateInput — why is that prop necessary?

Thanks for the review! I fixed the condition in a796d58.

I also don't understand the rationale for tying this new functionality to the __experimentalValidateInput — why is that prop necessary?

This is needed because if no validation is passed to __experimentalValidateInput, any string can be tokenized by pressing Enter key (or space if tokenizeOnSpace is true). But you're right, we don't need that check because addNewToken() won't be executed if a matched suggestion is found. I deleted the unnecessary condition in 25052bd.

ciampo

I've posted a few more comments, but my main question about the utility of this new feature is still unaddressed:

My main worry is about the UX — the user has no (visual) indication that the first matching suggestion is going to be automatically selected even if the user hasn't selected it explicitly.

Is there a particular reason for adding this functionality?

Could you elaborate a bit more, please? Thank you!

packages/components/src/form-token-field/types.ts

packages/components/src/form-token-field/stories/index.tsx

dinhtungdu · 2022-07-22T00:22:12Z

Is there a particular reason for adding this functionality?

Could you elaborate a bit more, please? Thank you!

@ciampo I'm sorry for the late reply. I updated the PR to remove __experimentalValidateInput references.

The reason for this functionality is when FormTokenField is used as a dropdown selector, we want to choose a token from the list only. It's handy for users to press enter to select an option. This behavior is similar to Select2 (https://select2.org/getting-started/basic-usage).

the user has no (visual) indication that the first matching suggestion is going to be automatically selected

You're right. I will add a visual indicator to the first matching suggestion to make the behavior predictable to users.

dinhtungdu · 2022-07-22T00:44:18Z

@ciampo in c355596, I updated the component to auto-select the first matching suggestion when user type 2 or more characters in the input (I use inputHasMinimumChars). Can you take another look at this?

jameskoster · 2022-07-22T14:39:26Z

Yeah I think it's fairly important for there to be a visual indication that pressing Enter will select the item. The current behaviour seems fairly intuitive to me:

keyboard.mp4

Saving a keypress doesn't really seem worth the potential confusing behaviour.

Aljullu · 2022-07-22T16:01:47Z

Thanks @dinhtungdu for creating the PR and @ciampo and @jameskoster for taking a look and providing feedback.

I would like to add some more context on why we would like to introduce this new behavior to the FormTokenField component:

In WC Blocks, we are working on using FormTokenField in the frontend as part of the Filter Products by Attribute block (woocommerce/woocommerce-blocks#6647). We don't allow the shopper to introduce values that don't match one of the available options (we are using __experimentalValidateInput to achieve that). Right now, it feels a bit frustrating that the shopper needs to introduce the attribute name in the same way it appears in the options even though in many cases introducing a short part of it would be enough.

As an example, imagine a filter that allows selecting the memory of USB sticks. Ideally, the user should be able to type 64 and then press Enter so the 64 GB option is selected:

Kooha-07-22-2022-17-41-58.mp4

However, that behavior is not currently possible with FormTokenField. That's specially important when the options are long or use special characters.

I understand this behavior might not be necessary in some use-cases for the FormTokenField, but I do think it's a common pattern in many other apps. A couple of examples that come to my mind are URL bars in browsers and Slack apps tooltip:

Browsers auto-select the first option in the URL bar	Slack select the first app when typing `/`

I agree, however, that having a visual indication that the first option is selected would be advisable. Maybe that could be achieved adding the same styles as :hover options?

ciampo

Hey @Aljullu , I understand your case much better now!

We don't allow the shopper to introduce values that don't match one of the available options (we are using __experimentalValidateInput to achieve that).

Together with highlighting the first option, I actually think that this is another key aspect of the UX around this feature — I now understand a bit more why @dinhtungdu initially mentioned __experimentalValidateInput in relationship to this feature.

I almost wonder if, when __experimentalAutoSelectFirstMatch is enabled, we should always check for the token to be in the list of suggestions?

diff --git a/packages/components/src/form-token-field/index.tsx b/packages/components/src/form-token-field/index.tsx
index 01b05d7916..ac9b3bab92 100644
--- a/packages/components/src/form-token-field/index.tsx
+++ b/packages/components/src/form-token-field/index.tsx
@@ -423,7 +423,11 @@ export function FormTokenField( props: FormTokenFieldProps ) {
 	}
 
 	function addNewToken( token: string ) {
-		if ( ! __experimentalValidateInput( token ) ) {
+		if (
+			! __experimentalValidateInput( token ) ||
+			( __experimentalAutoSelectFirstMatch &&
+				! suggestions.includes( token ) )
+		) {
 			speak( messages.__experimentalInvalid, 'assertive' );
 			return;
 		}

Or, at least, suggest to implement __experimentalValidateInput accordingly

dinhtungdu · 2022-07-25T03:03:48Z

I agree, however, that having a visual indication that the first option is selected would be advisable. Maybe that could be achieved adding the same styles as :hover options?

@Aljullu @jameskoster I addressed it in #42527 (comment). I'm sorry I didn't record a new screencast. I updated the PR description with the new screencast, (crossposting here for your convenience).

Screen.Recording.2022-07-25.at.09.47.14.mov

I almost wonder if, when __experimentalAutoSelectFirstMatch is enabled, we should always check for the token to be in the list of suggestions?

@ciampo I did this initially. But now I think we shouldn't do this for the most flexibility. When __experimentalAutoSelectFirstMatch is enabled, the first match (if it exists) will be selected automatically, before the logic to add an incomplete token is executed. Users can validate the token using __experimentalValidateInput if needed:

Users type the keyword
2a. If there are matches, the first match is selected automatically on typing. Users press Enter to select the match.
2b. If there is no match, the input is tokenized or not based on the __experimentalValidateInput.

jameskoster · 2022-07-25T09:49:29Z

This looks much better to the eye – If the first result has focus then pressing Enter to select it seems fine to me. I do wonder about the a11y implications though.

ciampo · 2022-07-25T14:13:29Z

This looks much better to the eye – If the first result has focus then pressing Enter to select it seems fine to me. I do wonder about the a11y implications though.

Looking at the code changes, I don't think that this PR introduced any regressions, as it uses existing internal functions like setSelectedSuggestionIndex. Having said that, it's completely possible that this component could do better under the a11y point of view (regardless of the changes included in this PR)

Aljullu · 2022-08-10T12:13:09Z

@jameskoster @ciampo is there anything we can do to move this PR forward?

ciampo · 2022-08-10T12:29:50Z

@jameskoster @ciampo is there anything we can do to move this PR forward?

I'll be having another look at this PR soon. Unfortunately I've been very busy lately and my response time on PR reviews have become considerably longer.

Aljullu · 2022-08-10T12:30:49Z

No problem! Thanks @ciampo!

ciampo

Code changes are looking good.

Before the final approval, let's rebase on top of latest trunk, since this PR has been open for some time and there are merge conflicts.

It would also be good to add a unit test for the expected behavior when __experimentalAutoSelectFirstMatch is used

Something like this (looks like this test is still written using `react-dom`, even though we're in the process of migrating our tests to RTL

diff --git a/packages/components/src/form-token-field/test/index.js b/packages/components/src/form-token-field/test/index.js
index 3be06711fc..10f6e3270c 100644
--- a/packages/components/src/form-token-field/test/index.js
+++ b/packages/components/src/form-token-field/test/index.js
@@ -396,6 +396,35 @@ describe( 'FormTokenField', () => {
 			sendKeyDown( keyCodes.enter );
 			expect( wrapper.state.tokens ).toEqual( [ 'foo', 'bar' ] );
 		} );
+
+		it( 'should automatically select the first matching suggestions when __experimentalAutoSelectFirstMatch is set to true', () => {
+			setUp( { __experimentalAutoSelectFirstMatch: true } );
+			wrapper.setState( {
+				isExpanded: true,
+			} );
+			expect( getSuggestionsText() ).toEqual( [] );
+
+			const searchText = 'so';
+
+			act( () => {
+				setText( searchText );
+			} );
+
+			const expectedFirstMatchTokens =
+				fixtures.matchingSuggestions[ searchText ][ 0 ];
+
+			expect( getSelectedSuggestion() ).toEqual(
+				expectedFirstMatchTokens
+			);
+
+			sendKeyDown( keyCodes.enter );
+
+			expect( wrapper.state.tokens ).toEqual( [
+				'foo',
+				'bar',
+				expectedFirstMatchTokens.join( '' ),
+			] );
+		} );
 	} );
 
 	describe( 'removing tokens', () => {

dinhtungdu · 2022-08-16T10:43:35Z

@ciampo I added the unit test per your suggestion. Thank you so much! Can you take another look at this? The failed React Native E2E test is unrelated, I don't have permission to rerun it.

ciampo

LGTM, thank you for your patience on this one!

Can you just:

rebase on top of latest trunk
add a CHANGELOG entry under the ### Experimental section ?

I'll merge when that's done

dinhtungdu · 2022-08-19T06:48:40Z

@ciampo I update my branch with latest trunk.

add a CHANGELOG entry under the ### Experimental section ?

I don't see the ### Experimental section. I guess you meant the screenshot section, so I added the changelog entry to the end of the PR description.

ciampo · 2022-08-19T08:09:42Z

Apologies if my suggestion wasn't clear enough — I meant the @wordpress/components package's CHANGELOG:

diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md
index d2da776b33..854c823db4 100644
--- a/packages/components/CHANGELOG.md
+++ b/packages/components/CHANGELOG.md
@@ -34,6 +34,10 @@
 -   `ColorPalette`: Refactor away from `_.uniq()` ([#43330](https://github.com/WordPress/gutenberg/pull/43330/)).
 -   `Guide`: Refactor away from `_.times()` ([#43374](https://github.com/WordPress/gutenberg/pull/43374/)).
 
+### Experimental
+
+-   `FormTokenField`: add `__experimentalAutoSelectFirstMatch` prop to auto select the first matching suggestion on typing ([#42527](https://github.com/WordPress/gutenberg/pull/42527/)).
+
 ## 19.17.0 (2022-08-10)
 
 ### Bug Fix

dinhtungdu · 2022-08-19T08:42:56Z

@ciampo Thanks for your clarification! I added the changelog entry and update the PR with the latest trunk again.

packages/components/CHANGELOG.md

ciampo · 2022-08-19T08:57:53Z

@ciampo Thanks for your clarification! I added the changelog entry and update the PR with the latest trunk again.

Awesome! I'll merge when CI is ✅

Thank you again!

dinhtungdu added 2 commits July 19, 2022 16:50

add: ability to auto select first matching suggestion for incomplete …

2598100

…token

add dropdown selector story

9c4089d

dinhtungdu requested a review from ajitbohra as a code owner July 19, 2022 10:26

dinhtungdu mentioned this pull request Jul 19, 2022

FormTokenField: Suggestion is expanded on load when __experimentalExpandOnFocus is set to true #42528

Closed

mirka added [Package] Components /packages/components [Type] Enhancement A suggestion for improvement. labels Jul 19, 2022

mirka requested review from mirka and ciampo July 19, 2022 20:27

ciampo reviewed Jul 20, 2022

View reviewed changes

ciampo changed the title ~~Add: ability to auto select first matching suggestion for incomplete token~~ FormTokenField: add ability to auto select first matching suggestion for incomplete token Jul 20, 2022

ciampo assigned dinhtungdu Jul 20, 2022

ciampo requested a review from chad1008 July 20, 2022 10:51

dinhtungdu added 3 commits July 20, 2022 18:15

fix the input validation condition

a796d58

remove unnecessary check

25052bd

docs: update prop comment

56f1fff

ciampo reviewed Jul 21, 2022

View reviewed changes

packages/components/src/form-token-field/types.ts Outdated Show resolved Hide resolved

packages/components/src/form-token-field/types.ts Outdated Show resolved Hide resolved

packages/components/src/form-token-field/stories/index.tsx Outdated Show resolved Hide resolved

dinhtungdu added 2 commits July 22, 2022 07:12

docs: update readme

30fe7e8

fix: remove __experimentalValidateInput from stories

5c74bbd

fix: preselect first match when input has minimum characters

c355596

dinhtungdu requested a review from ciampo July 22, 2022 00:42

jameskoster requested a review from a team July 22, 2022 14:39

ciampo reviewed Jul 22, 2022

View reviewed changes

jameskoster added the Needs Accessibility Feedback Need input from accessibility label Jul 25, 2022

ciampo reviewed Aug 11, 2022

View reviewed changes

dinhtungdu added 3 commits August 15, 2022 18:13

Merge branch 'trunk' into add/form-token-field-auto-select-first-match

32d41d3

fix cs

ea4b261

add unit test

12f2fb5

dinhtungdu requested a review from ciampo August 16, 2022 10:41

ciampo approved these changes Aug 18, 2022

View reviewed changes

Merge branch 'trunk' into add/form-token-field-auto-select-first-match

a58a6d8

dinhtungdu added 2 commits August 19, 2022 15:40

update changelog

7d7aead

Merge branch 'trunk' into add/form-token-field-auto-select-first-match

ae04926

ciampo reviewed Aug 19, 2022

View reviewed changes

packages/components/CHANGELOG.md Outdated Show resolved Hide resolved

Update packages/components/CHANGELOG.md

337edf8

ciampo merged commit 0a51f28 into WordPress:trunk Aug 19, 2022

github-actions bot added this to the Gutenberg 14.0 milestone Aug 19, 2022

Aljullu mentioned this pull request Dec 11, 2023

Filter by Attribute: auto-select first match when typing in the dropdown woocommerce/woocommerce#42546

Open

ntsekouras mentioned this pull request Jan 17, 2023

Audit Experimental APIs for WP 6.2 #47196

Closed

14 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FormTokenField: add ability to auto select first matching suggestion for incomplete token #42527

FormTokenField: add ability to auto select first matching suggestion for incomplete token #42527

dinhtungdu commented Jul 19, 2022 •

edited

Loading

ciampo left a comment

ciampo Jul 20, 2022

dinhtungdu Jul 20, 2022 •

edited

Loading

ciampo left a comment

dinhtungdu commented Jul 22, 2022

dinhtungdu commented Jul 22, 2022

jameskoster commented Jul 22, 2022

Aljullu commented Jul 22, 2022

ciampo left a comment

dinhtungdu commented Jul 25, 2022 •

edited

Loading

jameskoster commented Jul 25, 2022

ciampo commented Jul 25, 2022

Aljullu commented Aug 10, 2022

ciampo commented Aug 10, 2022 •

edited

Loading

Aljullu commented Aug 10, 2022

ciampo left a comment •

edited

Loading

dinhtungdu commented Aug 16, 2022

ciampo left a comment •

edited

Loading

dinhtungdu commented Aug 19, 2022 •

edited

Loading

ciampo commented Aug 19, 2022

dinhtungdu commented Aug 19, 2022

ciampo commented Aug 19, 2022

FormTokenField: add ability to auto select first matching suggestion for incomplete token #42527

FormTokenField: add ability to auto select first matching suggestion for incomplete token #42527

Conversation

dinhtungdu commented Jul 19, 2022 • edited Loading

What?

Why?

How?

Testing Instructions

Screenshots or screencast

Changelog

ciampo left a comment

Choose a reason for hiding this comment

ciampo Jul 20, 2022

Choose a reason for hiding this comment

dinhtungdu Jul 20, 2022 • edited Loading

Choose a reason for hiding this comment

ciampo left a comment

Choose a reason for hiding this comment

dinhtungdu commented Jul 22, 2022

dinhtungdu commented Jul 22, 2022

jameskoster commented Jul 22, 2022

Aljullu commented Jul 22, 2022

ciampo left a comment

Choose a reason for hiding this comment

dinhtungdu commented Jul 25, 2022 • edited Loading

jameskoster commented Jul 25, 2022

ciampo commented Jul 25, 2022

Aljullu commented Aug 10, 2022

ciampo commented Aug 10, 2022 • edited Loading

Aljullu commented Aug 10, 2022

ciampo left a comment • edited Loading

Choose a reason for hiding this comment

dinhtungdu commented Aug 16, 2022

ciampo left a comment • edited Loading

Choose a reason for hiding this comment

dinhtungdu commented Aug 19, 2022 • edited Loading

ciampo commented Aug 19, 2022

dinhtungdu commented Aug 19, 2022

ciampo commented Aug 19, 2022

dinhtungdu commented Jul 19, 2022 •

edited

Loading

dinhtungdu Jul 20, 2022 •

edited

Loading

dinhtungdu commented Jul 25, 2022 •

edited

Loading

ciampo commented Aug 10, 2022 •

edited

Loading

ciampo left a comment •

edited

Loading

ciampo left a comment •

edited

Loading

dinhtungdu commented Aug 19, 2022 •

edited

Loading