inputGroup - add InputGroupItem #9074
Conversation
|
Preview: https://patternfly-react-pr-9074.surge.sh A11y report: https://patternfly-react-pr-9074-a11y.surge.sh |
MariaAga
force-pushed
the
add-input-group-wrap
branch
2 times, most recently
from
8d48dd3
to
e5f4226
Compare
MariaAga
force-pushed
the
add-input-group-wrap
branch
2 times, most recently
from
457fadd
to
1eeaae4
Compare
packages/react-core/src/components/InputGroup/InputGroupItem.tsx
Outdated
Show resolved
Hide resolved
| /** Flag to indicate if the input group item is plain. */ | ||
| isBox?: boolean; | ||
| /** Flag to indicate if the input group item is plain. */ | ||
| isPlain?: boolean; |
There was a problem hiding this comment.
@srambach wdyt about the prop descriptions for these two? isBox I'm not entirely sure what would work best. "Flag to apply styling to non-form control input group items"?
There was a problem hiding this comment.
Edited the prop description to be (almost) the same as isBox prop on tabs
There was a problem hiding this comment.
I think the update is fine, but FWIW, this is documented in core - https://pf5.patternfly.org/components/input-group#usage
Adds appropriate styling for items that are not form controls.
packages/react-core/src/components/InputGroup/InputGroupItem.tsx
Outdated
Show resolved
Hide resolved
| @@ -3,6 +3,7 @@ import React from 'react'; | |||
| import { render, screen } from '@testing-library/react'; | |||
|
|
|||
| import { InputGroup } from '../InputGroup'; | |||
| import { InputGroupItem } from '../InputGroupItem'; | |||
There was a problem hiding this comment.
Not a blocker if we want to do it in a followup, but we should create a separate test file for InputGroupItem
MariaAga
force-pushed
the
add-input-group-wrap
branch
from
1eeaae4
to
52e3ecf
Compare
|
Still working on reviewing this, but one thing I've noticed so far is that some of the search inputs seem to be rendering shorter in this PR. In the below image this PR is on the left, v5 staging is on the right. Looks like it needs a
I've also noticed a few places where versioned class name changes are needed, but I'll put those together in a followup issue. |
|
Other than the above + needing a rebase |
|
Hmmm @kmcfaul I'm putting together a fix for the SearchInput issue now, while I'm doing it I could add new props for them as part of this breaking change. But scoping it as a followup would also probably not be a bad idea. Happy to go either way. |
|
Went with just having them set to fill for now at least since that customization isn't required for us to cut the alphas for testing. @thatblindgeye @kmcfaul if one of yall could give my changes a once over I would appreciate it. Convenience link: https://patternfly-react-pr-9074.surge.sh/components/search-input |
There was a problem hiding this comment.
Just one thing! This variant in <InputGroupText> is no longer valid - the plain modifier doesn't do anything to it. https://github.com/MariaAga/patternfly-react/blob/1fcc3a8128ec99288a592b354b2b23ebf8c08a67/packages/react-core/src/components/InputGroup/InputGroupText.tsx#L18
Because of the change done for PatternFly 5 at patternfly/patternfly-react#9074 Made by https://github.com/patternfly/pf-codemods
Because of the change done for PatternFly 5 at patternfly/patternfly-react#9074 Made by https://github.com/patternfly/pf-codemods
What: Closes #8264
Adds
InputGroupItemcomponent and wrapping each child ofinputGroupif the child is input text add isFill
if the child is
InputGroupTextadd isBox and sometimes isPlain