fix(split/stack): allow pf-m-fill to apply to any direct child#7201
fix(split/stack): allow pf-m-fill to apply to any direct child#7201mcoker merged 3 commits intopatternfly:mainfrom
Conversation
| ## Documentation | ||
| ### Overview | ||
| The split layout is designed to position items horizontally, with one item filling the available horizontal space. | ||
| The split layout is designed to position items horizontally. |
There was a problem hiding this comment.
AFAIK there is no expectation that any (or just one) item(s) need to fill the space.
| | -- | -- | -- | | ||
| | `.pf-v6-l-split` | `<div>`, `<section>`, or `<article>` | Initiates the split layout. | | ||
| | `.pf-v6-l-split__item` | `<div>` | Initiates a split item. **Required** | | ||
| | `.pf-v6-l-split` | `*` | Initiates the split layout. | |
There was a problem hiding this comment.
This can be any element, <Split /> in pf-react allows a component prop to let you use any element here.
|
Preview: https://patternfly-pr-7201.surge.sh A11y report: https://patternfly-pr-7201-a11y.surge.sh |
| | `.pf-v6-l-split` | `<div>`, `<section>`, or `<article>` | Initiates the split layout. | | ||
| | `.pf-v6-l-split__item` | `<div>` | Initiates a split item. **Required** | | ||
| | `.pf-v6-l-split` | `*` | Initiates the split layout. | | ||
| | `.pf-v6-l-split__item` | `*` | Initiates a split item. | |
There was a problem hiding this comment.
This is not required - prior to this PR, it was only required for .pf-m-fill, but with this PR it isn't required at all.
And while <SplitItem> always renders as a <div> in react, I checked with @tlabaj and we want to allow for a component prop here, too, since without it you could end up with invalid HTML. For example, if you do something like <Stack component="span"><StackItem>, you'll get <span><div>.
Here's the react issue and PR for the component prop on the split/stack items
patternfly/patternfly-react#11169
patternfly/patternfly-react#11170
| .#{$split}__item.pf-m-fill { | ||
| flex-grow: 1; | ||
| @at-root .#{$split}__item.pf-m-fill, | ||
| & > .pf-m-fill { |
There was a problem hiding this comment.
Now this rule renders as
.pf-v6-l-stack__item.pf-m-fill,
.pf-v6-l-stack > .pf-m-fill {
flex-grow: 1;
}
|
Is there a react rendering of this that I can test out my expectations @mcoker ? |
|
@andrewballantyne sure! Here's a basic
|
|
Looks good to me @mcoker -- that's what I was looking for. I can't edit the sandboxes (don't have an account) but visually inspecting the examples looks to fit what I was after. Thanks! |
There was a problem hiding this comment.
Looks good - one question, would you want to adjust the custom workspace styling (that adds the dashed outline) to also work without the _item classes?
Also, is this change useful on the level layout?
|
@srambach sure I don't see why not. Added |
There was a problem hiding this comment.
I think in discussions we decided we don't need to add a pf-m-fill on the level layout, but keep the local workspace styling to show that __item isn't required.
|
🎉 This PR is included in version 6.1.0-prerelease.4 🎉 The release is available on: Your semantic-release bot 📦🚀 |
fixes #7190