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
| 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. |
wise-king-sullyman
left a comment
wise-king-sullyman
left a comment
There was a problem hiding this comment.
My previous comment about SearchInput is the only thing I see that needs some tweaking.
|
Other than the above + needing a rebase |
kmcfaul
left a comment
kmcfaul
left a comment
There was a problem hiding this comment.
lgtm other than the SearchInput issue.
Maybe as a follow up, would it be worth allowing users to customize some of these new props by piping them through the wrapper component (like SearchInput for example)?
|
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 |
thatblindgeye
left a comment
thatblindgeye
left a comment
There was a problem hiding this comment.
Noticing a visual bug currently on the SearchInput when it has focus:
It's on any example using that "submit" arrow button
thatblindgeye
left a comment
thatblindgeye
left a comment
There was a problem hiding this comment.
Some snapshots just need updating but otherwise lgtm
mcoker
left a comment
mcoker
left a comment
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
8 participants
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