Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SearchInput - update input type to text #8651

Closed
thatblindgeye opened this issue Feb 3, 2023 · 4 comments · Fixed by #8957
Closed

SearchInput - update input type to text #8651

thatblindgeye opened this issue Feb 3, 2023 · 4 comments · Fixed by #8957
Assignees
Labels
Feature end user feature that requires design to define this, i.e. changes or augments the user experience. PF5
Milestone

Comments

@thatblindgeye
Copy link
Contributor

Is this a new component or an extension of an existing one?
SearchInput

Describe the feature
Based off of conversation in patternfly/patternfly#5210 and its linked PR.

tl;dr is type="search" for a text input can cause a browser rendered clear icon that isn't keyboard focusable to render, but not all browsers do (Firefox). Instead we should:

  • use type="text" so that no browser clear icon renders (should just involve removing instances of type="search" since SearchInput defaults to text)
  • ensure any examples using SearchInput have the PF clear button rendered when applicable. See search input basic for an example of this (after typing in the input).
  • Ensure examples have some sort of labeling (either text or aria) that conveys it as a search input

This would probably also include deprecating/removing the type prop on SearchInput to avoid dual clear icons rendering.

@thatblindgeye thatblindgeye added Feature end user feature that requires design to define this, i.e. changes or augments the user experience. needs triage labels Feb 3, 2023
@mcarrano mcarrano added this to the 2023.05 milestone Feb 17, 2023
@nicolethoen nicolethoen added the PF5 label Mar 7, 2023
@gitdallas
Copy link
Contributor

@thatblindgeye @nicolethoen - i'm also seeing that we use type="search" in other places, like directly on an input element within the Select filterBox and on TextInput components in the PrimaryDetail demos.

should anything be done about these?

@gitdallas
Copy link
Contributor

also, on the last point... should i deprecate/remove the type prop?

  /** Type of the input */
  type?:
    | 'text'
    | 'date'
    | 'datetime-local'
    | 'email'
    | 'month'
    | 'number'
    | 'password'
    | 'search'
    | 'tel'
    | 'time'
    | 'url';

@thatblindgeye
Copy link
Contributor Author

I feel like it'd make sent to update those other uses of type="search" to type="text", and also adding an aria-label="Search" (or "Filter" or whatever action is most applicable) if there isn't already one on the input element/component.

@nicolethoen would we be able to just remove the type prop from SearchInput, or should we deprecate it and add a disclaimer that "text should be used"? Most of the values for the prop may not be relevant for the component.

@nicolethoen
Copy link
Contributor

I think we determined that the difference between type search and text is really just the presence (or not) of the close button in the input. So as long as it still has a close button that is added by patternfly and the screen reader still calls the input a search in the examples, then I see no reason why someone would want to set it back to being type search. it's still a breaking change in case people are looking for type search in any of their css or test selectors, so we'd need to warn the users of the change via a code mod.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature end user feature that requires design to define this, i.e. changes or augments the user experience. PF5
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants