-
Notifications
You must be signed in to change notification settings - Fork 357
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
docs(text-input-group): Adds React docs. #10934
Conversation
Preview: https://patternfly-react-pr-10934.surge.sh A11y report: https://patternfly-react-pr-10934-a11y.surge.sh |
Note: this pr demonstrates a spacing issue that I've seen with some of our React docs, where there's no blank line between lines of text. This wasn't the case with v5, so I'm not sure what's causing that styling. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple comments below. For the spacing issue, looks like this line of code may be removing that spacing between paragraphs https://github.com/patternfly/patternfly-org/blob/c2104a3bd19ab5c6fe8a891f1a28322bd4029d3e/packages/documentation-framework/templates/mdx.css#L1-L3
|
||
To prevent users from making edits to a text input group, you can disable it using `isDisabled`. | ||
|
||
When you disable a text input group, add an `aria-label` to provide context to assistive technologies. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typically an aria-label
will be needed for any TextInputGroup, disabled or not, if there's no label
element tied to the input
itself.
We could move this line to the basic example and tweak it a little, or we could maybe just remove it and have it live int he accessibility docs once added, wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah okay, fair point! Let's have it in the accessibility docs then
|
||
To help users identify the purpose of an input group, you can add an icon. To do this, import your icon and pass it to `icon` within `<TextInputGroupMain>`. | ||
|
||
You can also add a clear button that allows users remove their previous input. Pass your button into `<TextInputGroupUtilities>`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about updating the example name to match closer to Core, "With icon and utilities" (Core uses "Utilities and icon with placeholder text", which maybe this example could also showcase placeholder as well)?
Depending on that, maybe the description could then be updated to state more along the lines that additional utilities/actions can be rendered, and that the example shows how one can implement a "clear" button?
Closes #10761