Text Input Group issues

[dlabrecq]

I added PatternFly's new Text Input Group component to Cost Management. The type-ahead is more usable than the Select component, but there are limitations. For example, we cannot add place holder text and an aria label. I also had to take some liberties regarding how keyboard navigation works.

Below are the observations I made while implementing the Text Input Group component.

Component Issues

1. Cannot add place holder text
2. . TextInputGroupMain does not clear input text for a value of undefined — must be an empty string

Example issues

1. Example doesn't use aira labels
2. Input focus box not rendered correctly without clear button — must conditionally render TextInputGroupUtilities
3. Input value cleared when menu has focus and escape key is pressed — should just close the menu and set focus on the text input for further editing
4. The tab key doesn't appear to work at all -- code looks like it was intended to select a menu option?

Cost Management implementation

1. Added “No results found” label instead of always showing “Suggestions”
2. Made the tab key clear the input value and navigate to the next tab index
3. A lot of keyboard navigation code must be added manually — painstaking to get working correctly

[nicolethoen]

Awesome feedback, @dlabrecq! Thank you!

[tlabaj]

I opened issue #4577 for the core work to support the place holder text.

[wise-king-sullyman]

@dlabrecq I have a few questions:

Component Issues

Cannot add place holder text and aria label

Where are you wanting an aria label to be added?

TextInputGroup and TextInputGroupUtilities can take aria labels as additional props and will place them on the divs they return, TextInputGroupMain currently passes any aria labels it receives to the text input that it contains.

TextInputGroupMain does not clear input text for a value of undefined — must be an empty string

TextInputGroupMain could be edited to set the input value as an empty string when its value prop is undefined (or falsy in general) and I wouldn't be opposed to that change, but this might be something better handled on the implementation side. @tlabaj do you have an opinion on this?

Example issues

Input focus box not rendered correctly without clear button — must conditionally render TextInputGroupUtilities

TextInputGroupUtilitiesbeing conditionally rendered is fully optional behavior dependant on the composition of TextInputGroup. Is this something you'd like to see in a demo/example?

Input value cleared when menu has focus and escape key is pressed — should just close the menu and set focus on the text input for further editing

I wouldn't be opposed to changing the escape interaction in the auto-complete search demo, but clearing the input is how typeahead Select works which this part of the interaction was modeled after. @mcarrano is this a change you'd like to see?

The tab key doesn't appear to work at all -- code looks like it was intended to select a menu option?

Tab seems to be working as I expect/describe in the auto-complete search demo so I'm not sure that I understand this issue?

Cost Management implementation

Added “No results found” label instead of always showing “Suggestions”

That functionality is already been built into the auto-complete search demo, so again I'm not sure that I understand this issue?

Made the tab key clear the input value and navigate to the next tab index

The current tab behavior in the auto-complete search demo was something @mcarrano specifically requested. Is this something you'd like to see changed?

A lot of keyboard navigation code must be added manually — painstaking to get working correctly

I'd be very interested to see the additional keyboard navigation functionality that you had to add if you can link it/tell me where I can find it. I'm more than happy to do what I can to improve the keyboard navigation of the component/demos.

[mcarrano]

My best attempt to answer your questions @wise-king-sullyman ...

Input value cleared when menu has focus and escape key is pressed — should just close the menu and set focus on the text input for further editing.

This makes sense to me.

The current tab behavior in the auto-complete search demo was something @mcarrano specifically requested.

Yes, the intent was to make this work like an auto-complete text field so the user can use the tab key to autofill the string. Why do you expect the tab key to clear the field @dlabrecq ?

Would be great to get your input on these interactions, also, @jessiehuff .

[dlabrecq]

@mcarrano Pressing the escape key closes the dropdown, while focus should remain on the text field. This is useful in order to change the current text input value. At least, this is how I implemented it for Cost management -- the example doesn't do that.

Pressing the tab key (while in the dropdown area) moves focus from the text input to the next HTML element. IMO, it looks odd to leave a value in the text input after moving focus elsewhere. That is, considering it does not apply to anything in view.

That said, I can change Cost Management if this behavior is not desirable. However, the current example's tab behavior doesn't make sense to me.

Tab key:

Escape key:

[mcarrano]

Pressing the escape key closes the dropdown, while focus should remain on the text field. This is useful in order to change the current text input value. At least, this is how I implemented it for Cost management -- the example doesn't do that.

@dlabrecq I'm a bit confused about what the complaint is here. I agree that this is how it should work. Isn't the example working that way or am I missing something?

Pressing the tab key (while in the dropdown area) moves focus from the text input to the next HTML element. IMO, it looks odd to leave a value in the text input after moving focus elsewhere. That is, considering it does not apply to anything in view.

The intent is for this to work like an autocomplete search field as in the demo here: https://www.patternfly.org/v4/components/search-input/react-demos#search-with-autocomplete It will make more sense when the ghost text is added. Hopefully in the next release. But it does look like there's a slight bug in the Text Input Group example where the focus is not moving upon pressing Tab @wise-king-sullyman . It may be that this is not the correct control for your use case @dlabrecq . Seems like you want something more like a type-ahead select that requires the value to be a member of a closed list? This demo was intended to be for something more like a Google search where the input string is unbound.