Skip to content

docs(TextInputGroup): add auto-complete search demo#6661

Merged
tlabaj merged 5 commits intopatternfly:mainfrom
wise-king-sullyman:auto-complete-search-demo
Dec 6, 2021
Merged

docs(TextInputGroup): add auto-complete search demo#6661
tlabaj merged 5 commits intopatternfly:mainfrom
wise-king-sullyman:auto-complete-search-demo

Conversation

@wise-king-sullyman
Copy link
Copy Markdown
Collaborator

@wise-king-sullyman wise-king-sullyman commented Nov 30, 2021

What: Closes #6586

Additional issues: N/A

I moved the attribute-value filtering demo to a separate file to clean the .md demo file up, but I did not make any changes to it. Only the copy starting on line 29 in the markdown file and AutoCompleteSearch.js are truly being added in this PR.

Direct link to the preview demo for convenience: https://patternfly-react-pr-6661.surge.sh/components/text-input-group/react-demos

@patternfly-build
Copy link
Copy Markdown
Collaborator

patternfly-build commented Nov 30, 2021

@nicolethoen
Copy link
Copy Markdown
Contributor

This looks really good!
Is there a way to use the keyboard to return to the input once focus is in the menu?

Copy link
Copy Markdown
Member

@mcarrano mcarrano left a comment

Choose a reason for hiding this comment

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

This is looking good @wise-king-sullyman . My only ask is whether it's possible to use the Tab key to autocomplete the string when there is only one suggestion remaining like what's done in this Search input demo: https://patternfly-react-pr-6661.surge.sh/components/search-input/react-demos#search-with-autocomplete

@wise-king-sullyman
Copy link
Copy Markdown
Collaborator Author

@nicolethoen choosing a selection will automatically return focus to the input. I suppose I could add an onKeyDown to the menu and have escape while focused on the menu also close the menu and return focus to the input without making a selection.

That seems to be about how it works in Select. I'll make that change shortly unless you think there's a better interaction/solution here.

@wise-king-sullyman
Copy link
Copy Markdown
Collaborator Author

@mcarrano Yes that should be possible. I'll add that.

@wise-king-sullyman
Copy link
Copy Markdown
Collaborator Author

@mcarrano I see that in search input tab to complete still tabs over to the clear button and out of the input, is that the interaction you'd like to see here?

Since enter will still need to be hit afterwards to enter the selection as a chip in this interaction, I think it makes more sense to prevent the tabbing over and have focus stay in the input.

@mcarrano
Copy link
Copy Markdown
Member

mcarrano commented Dec 2, 2021

Yes, I agree with you @wise-king-sullyman .

@nicolethoen I believe you did the Search input demo implementation. Does what @wise-king-sullyman suggests make sense to you, too?

@nicolethoen
Copy link
Copy Markdown
Contributor

nicolethoen commented Dec 2, 2021

@wise-king-sullyman I agree with you abut close the menu with esc.

esc while the menu is open with focus, should return focus to the input and close the menu.

@mcarrano we dont have the ghost autocomplete design implemented in the text input group like we do for search input (at least not yet). so I don't know if it's intuitive that hitting tab will have a side effect. Hitting tab only has a side effect when you have one possible suggestion left and it gives you the ghost suggestion in the input. I don't think it applies here at this time.

@wise-king-sullyman
Copy link
Copy Markdown
Collaborator Author

@mcarrano I've added your suggestion in c0b2fde, if you would like it removed after considering @nicolethoen's comment about the lack of ghosting vs Search input please let me know and I'll revert the commit.

I feel like the interaction is still nice even without the ghosting after using it, and I added a note to the demo description to let users know the functionality exists.

@nicolethoen
Copy link
Copy Markdown
Contributor

@wise-king-sullyman
I think the autocomplete with the tab - even without the ghost row - is pretty slick.
I take back my concerns :)

Copy link
Copy Markdown
Contributor

@mcoker mcoker left a comment

Choose a reason for hiding this comment

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

🏆

Copy link
Copy Markdown
Member

@mcarrano mcarrano left a comment

Choose a reason for hiding this comment

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

@wise-king-sullyman @nicolethoen I'm OK to keep the tabbed auto-complete behavior in there, even if there is no ghosting. Is that something we can add here in a future sprint? Should we open an issue for it?

@nicolethoen
Copy link
Copy Markdown
Contributor

@mcarrano I will add an issue to core with a follow up issue in react for the ghosting in the new text input group

Copy link
Copy Markdown
Contributor

@jessiehuff jessiehuff left a comment

Choose a reason for hiding this comment

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

This seems pretty good by keyboard. When I tried it with VO, I was able to type into the input, but I wasn't able to enter into the menu to select anything. When I looked at the DOM, I think it's because the menu is in a completely different place in the DOM, and since focus never moves, there's really no way a user would be able to tell a menu is open unless they navigate through the entire page. (They might even be confused why there's a random menu at the bottom of the page because it's out of context.) If we want to address this in a separate issue/PR that's good with me though!

@wise-king-sullyman
Copy link
Copy Markdown
Collaborator Author

@jessiehuff bf6c2b1 moved the menu to follow the text input group immediately in the DOM, I think that should improve VO usability but I'm not an expert in VO usage so it may not be the ideal approach/implementation to that issue.

If it isn't please let me know and I'll take another swing at it, though I agree that it may be best to do it under a different PR.

Copy link
Copy Markdown
Contributor

@jessiehuff jessiehuff left a comment

Choose a reason for hiding this comment

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

LGTM! :)

Copy link
Copy Markdown
Contributor

@tlabaj tlabaj left a comment

Choose a reason for hiding this comment

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

LGTM

@tlabaj tlabaj merged commit 25a1b79 into patternfly:main Dec 6, 2021
@wise-king-sullyman wise-king-sullyman deleted the auto-complete-search-demo branch December 6, 2021 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Demonstrate auto-complete search

7 participants