Conversation
|
Preview: https://patternfly-pr-6838.surge.sh A11y report: https://patternfly-pr-6838-a11y.surge.sh |
| align-items: baseline; | ||
|
|
||
| // - Toolbar icon button group - Toolbar filter group | ||
| &.pf-m-icon-button-group { |
There was a problem hiding this comment.
This modifier was updated to pf-m-action-group-plain
| .#{$toolbar}__content, | ||
| .#{$toolbar}__content-section, | ||
| .#{$toolbar}__expandable-content { | ||
| row-gap: var(--#{$toolbar}__content--RowGap); | ||
| column-gap: var(--#{$toolbar}--ColumnGap); | ||
| } | ||
|
|
||
| // - Toolbar content - Toolbar expandable content | ||
| .#{$toolbar}__content, | ||
| .#{$toolbar}__expandable-content { | ||
| padding-block-start: var(--#{$toolbar}__content--PaddingBlockStart, var(--#{$toolbar}__content--PaddingBlock)); | ||
| padding-block-end: var(--#{$toolbar}__content--PaddingBlockEnd, var(--#{$toolbar}__content--PaddingBlock)); | ||
| padding-inline-start: var(--#{$toolbar}__content--PaddingInlineStart, var(--#{$toolbar}__content--PaddingInline)); | ||
| padding-inline-end: var(--#{$toolbar}__content--PaddingInlineEnd, var(--#{$toolbar}__content--PaddingInline)); | ||
| } |
There was a problem hiding this comment.
These row/column-gap, padding settings were separated and given their own variables.
0659ddf to
dc262b4
Compare
6455ad1 to
fb714be
Compare
fb714be to
12343ca
Compare
a384179 to
378e57e
Compare
378e57e to
eabaade
Compare
There was a problem hiding this comment.
Just a couple of comments, otherwise lgtm!
|
|
||
| // - Toolbar action group | ||
| &.pf-m-action-group { | ||
| column-gap: var(--#{$toolbar}__group--m-action-group--ColumnGap); |
There was a problem hiding this comment.
the global gap spacer on the action group needs to be updated to: global/spacer/gap/action-to-action: 16px/spacer-300
👍
|
|
||
| // - Toolbar filter group | ||
| &.pf-m-filter-group { | ||
| column-gap: var(--#{$toolbar}__group--m-filter-group--ColumnGap); |
There was a problem hiding this comment.
This might be a little nit and I think this would actually be another core issue for the filter group component directly but, the semantic gap spacer will need updated to match this screenshot. visually it looks right, just needs the updated semantic naming i believe
81d7263 to
0d3997a
Compare
There was a problem hiding this comment.
added some small comments on semantic spacers but everything looks pretty good!
|
|
||
| // - Toolbar action group inline | ||
| &.pf-m-action-group-inline { | ||
| column-gap: var(--#{$toolbar}__group--m-action-group-inline--ColumnGap); |
There was a problem hiding this comment.
not sure if this would be considered a action to action or a gap group horizontal? @lboehling need some assistance here!
a948626 to
1f7b8a4
Compare
… removed erroneous css
67a3b2e to
29c3990
Compare
There was a problem hiding this comment.
LGTM, left a couple of comments, but neither are blockers for this PR IMO and can be followed up on for the final release if we need to.
| align-items: start; | ||
|
|
||
| // push pagination to inline-end when flex items are not wrapped | ||
| &:has(.pf-m-pagination:last-child) { |
There was a problem hiding this comment.
targeting .pf-m-pagination as a child anywhere will run the risk of targeting some other component if we or users add a .pf-m-pagination modifier - can we add whatever toolbar classes .pf-m-pagination will go on to this selector?
| > .#{$toolbar}__group, | ||
| > .#{$toolbar}__item { | ||
| &:nth-last-child(2) { | ||
| flex: 1; | ||
| } | ||
| } |
There was a problem hiding this comment.
This :nth-last-child(2) pseudo seems like it could be fragile? Can you point to an example that shows what this is targeting?
|
🎉 This PR is included in version 6.0.0-alpha.187 🎉 The release is available on: Your semantic-release bot 📦🚀 |
closes #6827
This PR
Adds:
.pf-m-action-group.pf-m-action-group-plain.pf-m-action-group-inlineUpdates:
paddingBlock, paddingInlineBlockfallbackspaddingBlockStart/End,paddingInlineStart/EndvariablesRemoves: