Skip to content

fix(TextInputGroup): address various issues#6774

Merged
tlabaj merged 5 commits intopatternfly:mainfrom
wise-king-sullyman:text-input-group-issues
Jan 20, 2022
Merged

fix(TextInputGroup): address various issues#6774
tlabaj merged 5 commits intopatternfly:mainfrom
wise-king-sullyman:text-input-group-issues

Conversation

@wise-king-sullyman
Copy link
Copy Markdown
Collaborator

@wise-king-sullyman wise-king-sullyman commented Jan 12, 2022

On auto complete search:

  • undoes default tab behavior prevention

  • undoes input clearing on escape when focused on menu

  • adds input clearing on tab when focused on menu

  • adds placeholder text and custom aria label

  • clarifies reasoning for conditionally rendered search icon

On text input group main:

  • sets falsy input values as an empty string

  • exposes value and placeholder props

What: Closes #6697

Additional issues:

Direct link to the auto complete search preview

on auto complete search:

undoes default tab behavior prevention

undoes input clearing on escape when focused on menu

adds input clearing on tab when focused on menu

adds placeholder text and custom aria label

clarifies reasoning for conditionally rendered search icon

on text input group main:

sets falsy input values as an empty string

exposes value and placeholder props
@patternfly-build
Copy link
Copy Markdown
Collaborator

patternfly-build commented Jan 12, 2022

@tlabaj tlabaj requested a review from dlabrecq January 12, 2022 21:59
Copy link
Copy Markdown
Member

@dlabrecq dlabrecq left a comment

Choose a reason for hiding this comment

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

Per @nicolethoen, tab behavior should mimic the Search Input example.

Within the menu of Text Input Group, pressing the tab key moves focus to another element in the page. In this case, the menu remained open, which is not expected. It seems the user must press the escape key first?

For Search Input, pressing the tab key returns focus to the input. From there, I can tab to the clear button.

Text Input Group example

chrome-capture

Search Input example

chrome-capture (1)

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

Tab now closes the menu and focuses on the text input. Additionally, I saw that this demo was missing the search input auto complete demo functionality of closing the menu when Enter or Space were hit so I added that as well to bring these demos more in line with each other.

Copy link
Copy Markdown
Member

@dlabrecq dlabrecq left a comment

Choose a reason for hiding this comment

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

LGTM

@nicolethoen
Copy link
Copy Markdown
Contributor

I think if focus is on the input and user hits tab to complete the autocomplete, the menu should close when the focus moves to the next focusable element (the close button)

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

0b266d4 was added as an alternate solution to patternfly/patternfly#4588. This commit demonstrates how consumers can conditionally render the TextInputGroupUtilities component itself as well as its children.

@mmenestr
Copy link
Copy Markdown
Collaborator

LGTM!

@tlabaj tlabaj merged commit 010058a into patternfly:main Jan 20, 2022
@patternfly-build
Copy link
Copy Markdown
Collaborator

Your changes have been released in:

  • eslint-plugin-patternfly-react@4.30.6
  • @patternfly/react-catalog-view-extension@4.42.6
  • @patternfly/react-charts@6.44.6
  • @patternfly/react-code-editor@4.32.6
  • @patternfly/react-console@4.42.6
  • @patternfly/react-core@4.191.6
  • @patternfly/react-docs@5.52.6
  • @patternfly/react-icons@4.42.6
  • @patternfly/react-inline-edit-extension@4.36.6
  • demo-app-ts@4.151.6
  • @patternfly/react-integration@4.153.6
  • @patternfly/react-log-viewer@4.36.6
  • @patternfly/react-styles@4.41.6
  • @patternfly/react-table@4.60.6
  • @patternfly/react-tokens@4.43.6
  • @patternfly/react-topology@4.38.6
  • @patternfly/react-virtualized-extension@4.38.6
  • transformer-cjs-imports@4.29.6

Thanks for your contribution! 🎉

@wise-king-sullyman wise-king-sullyman deleted the text-input-group-issues branch January 20, 2022 18:05
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.

Text Input Group issues

7 participants