fix(toolbar): added center and baseline alignments #5201

mattnolting · 2022-10-31T14:40:30Z

closes #5030

This PR adds:

.pf-m-align-items-center to .pf-c-toolbar__content-section and .pf-c-toolbar__group
.pf-m-align-self-center to .pf-c-toolbar__group and .pf-c-toolbar__item
.pf-m-align-self-baseline to .pf-c-toolbar__group and .pf-c-toolbar__item

patternfly-build · 2022-10-31T14:45:17Z

Preview: https://patternfly-pr-5201.surge.sh

A11y report: https://patternfly-pr-5201-a11y.surge.sh

mattnolting · 2022-11-08T20:17:26Z

@mcoker rebased and ready

mcoker

Looks good! Left some comments. Also curious if we should update these to use the new vars

patternfly/src/patternfly/components/Toolbar/toolbar.scss

Line 437 in 78f6c2d

align-items: baseline;

patternfly/src/patternfly/components/Toolbar/toolbar.scss

Lines 170 to 175 in 78f6c2d

    
           .pf-c-toolbar__content, 
        
           .pf-c-toolbar__content-section, 
        
           .pf-c-toolbar__group, 
        
           .pf-c-toolbar__item { 
        
             align-self: stretch; 
        
           }

src/patternfly/components/Toolbar/toolbar.scss

mattnolting · 2022-11-09T19:59:05Z

@mcoker updated

mcoker · 2022-11-09T23:13:19Z

src/patternfly/components/Toolbar/toolbar.scss

@@ -13,9 +13,12 @@ $pf-c-toolbar--inset-map: build-spacer-map("none", "sm", "md", "lg", "xl", "2xl"
  // Item
  --pf-c-toolbar__item--Display: block;
  --pf-c-toolbar__item--MinWidth--base: auto;
+  --pf-c-toolbar__item--AlignSelf: baseline;


should this be auto? otherwise, setting align-items on the group doesn't seem like it will ever work, since align-self on the items will always override that.

mcoker · 2022-11-09T23:14:03Z

src/patternfly/components/Toolbar/examples/Toolbar.md

@@ -664,6 +664,10 @@ As the toolbar component is a hybrid layout and component, some of its elements
 | `.pf-m-visible{-on-[breakpoint]}` | `.pf-c-toolbar__content`, `.pf-c-toolbar__content-section`, `.pf-c-toolbar__item`, `.pf-c-toolbar__group` | Modifies toolbar element to be shown, at optional [breakpoint](/developer-resources/global-css-variables#breakpoint-variables-and-class-suffixes). |
 | `.pf-m-align-right{-on-[breakpoint]}` | `.pf-c-toolbar > *` | Modifies toolbar element to align right, at optional [breakpoint](/developer-resources/global-css-variables#breakpoint-variables-and-class-suffixes). |
 | `.pf-m-align-left{-on-[breakpoint]}` | `.pf-c-toolbar > *` | Modifies toolbar element to align left, at optional [breakpoint](/developer-resources/global-css-variables#breakpoint-variables-and-class-suffixes). |
+| `.pf-m-align-items-center` | `.pf-c-toolbar__content-section`, `.pf-c-toolbar__group` | Modifies toolbar element to vertically align children to center. |
+| `.pf-m-align-items-baseline` | `.pf-c-toolbar__content-section`, `.pf-c-toolbar__group` | Modifies toolbar element to vertically align children to center. |


Suggested change

| `.pf-m-align-items-baseline` | `.pf-c-toolbar__content-section`, `.pf-c-toolbar__group` | Modifies toolbar element to vertically align children to center. |

| `.pf-m-align-items-baseline` | `.pf-c-toolbar__content-section`, `.pf-c-toolbar__group` | Modifies toolbar element to vertically align children to baseline. |

mcoker · 2022-11-09T23:15:21Z

src/patternfly/components/Toolbar/toolbar.scss

+  &.pf-m-align-items-baseline {
+    --pf-c-toolbar__group--AlignItems: baseline;
+  }


Since this is the default, is there ever a use case for this class?

mcoker · 2022-11-09T23:16:26Z

src/patternfly/components/Toolbar/toolbar.scss

+  &.pf-m-align-items-baseline {
+    --pf-c-toolbar__content-section--AlignItems: baseline;
+  }


Same question here as for the group - since it's the default, what's the use case for this class?

mcoker

As mentioned on slack, 2 remaining questions:

just wanted to confirm we don't want to support align-self-baseline on groups and items anymore. Since their flex parents support align-items-center, I can see using that on the parent, and having a group/item as an exception using baseline alignment via align-self-baseline. Though I'm not sure if that's a real use case.
Since you can nest groups, the alignment will inherit. I'm assuming that isn't ideal?

mattnolting · 2022-11-16T13:37:08Z

@mcoker added baseline mods back as children can inherit variable update

mcoker

Looks good, everything works well.

One potential issue - if everything is set to baseline by default, baseline is going to try and align with stuff around it, meaning "baseline" could result in "center" alignment (visually) if the other items in the layout are center aligned. Is that expected? https://codepen.io/mcoker/pen/QWxOJZQ

I wonder if the alignment properties should inherit. I don't think I would expect them to. Looking at the flex layout, the alignment mods redefine the regular property, not custom property, so the changes aren't inherited. That might be the most clean, tho if we want to manage everything with custom properties, we could use a --base var to set the default within each element that takes an alignment selector. Here's an example - https://codepen.io/mcoker/pen/jOKaQXx?editors=1100

Or in this PR, something like:

.pf-c-toolbar {
  --pf-c-toolbar__group--AlignItems--base: baseline;
}
.pf-c-toolbar__group {
  --pf-c-toolbar__group--AlignItems: var(--pf-c-toolbar__group--AlignItems--base);
  align-items: var(--pf-c-group__AlignItems);
  &.pf-m-center {
    --pf-c-toolbar__group--AlignItems: center;
  }
}

mcoker · 2022-11-18T23:29:58Z

src/patternfly/components/Toolbar/toolbar.scss

@@ -1,7 +1,7 @@
 // Don't remove this magic comment. See gulpfile.js.
 // @import "../../sass-utilities/all";
 $pf-c-toolbar--breakpoint-map: build-breakpoint-map("base", "sm", "md", "lg", "xl", "2xl");
-$pf-c-toolbar--spacer-map: build-spacer-map("none", "sm", "md", "lg");
+$pf-c-toolbar--spacer-map: build-spacer-map();


Just want to verify this is on purpose?

No, not purposeful

mattnolting · 2022-11-22T15:16:42Z

Looks good, everything works well.

One potential issue - if everything is set to baseline by default, baseline is going to try and align with stuff around it, meaning "baseline" could result in "center" alignment (visually) if the other items in the layout are center aligned. Is that expected? https://codepen.io/mcoker/pen/QWxOJZQ

I wonder if the alignment properties should inherit. I don't think I would expect them to. Looking at the flex layout, the alignment mods redefine the regular property, not custom property, so the changes aren't inherited. That might be the most clean, tho if we want to manage everything with custom properties, we could use a --base var to set the default within each element that takes an alignment selector. Here's an example - https://codepen.io/mcoker/pen/jOKaQXx?editors=1100

Or in this PR, something like:
.pf-c-toolbar {
  --pf-c-toolbar__group--AlignItems--base: baseline;
}
.pf-c-toolbar__group {
  --pf-c-toolbar__group--AlignItems: var(--pf-c-toolbar__group--AlignItems--base);
  align-items: var(--pf-c-group__AlignItems);
  &.pf-m-center {
    --pf-c-toolbar__group--AlignItems: center;
  }
}

@mcoker I think updating the property (not custom property) is the way to go here. It's the simplest solution and it works well. Updated.

mcoker

LGTM 🥳

srambach · 2022-12-12T17:08:14Z

Two questions - what is the criteria for putting modifiers in the top section? Some are there but not all.
Second, is it worth making a demo of this, since it's not necessarily obvious how this alignment is useful in the case of error messages?

mattnolting · 2022-12-16T15:44:27Z

Two questions - what is the criteria for putting modifiers in the top section? Some are there but not all. Second, is it worth making a demo of this, since it's not necessarily obvious how this alignment is useful in the case of error messages?

@srambach I think the current alignment is a misimplementation. Center alignment would be the edge case for anything that doesn't share a height with selects, buttons, text inputs, etc. If we add a demo, it should highlight center alignment, rather than baseline. I would expect for that to be necessary for custom implementations, like including an icons, logos, text, etc.

srambach · 2023-01-04T14:19:35Z

Is it expected that the "toolbar with validation desktop" example doesn't wrap and overflows? It happens at a relative wide width. (around 1245px)
Also, could you fix the title of that and the other desktop demo to be "on desktop" or something like that?

srambach

Nice job 👍

patternfly-build · 2023-01-13T16:39:25Z

🎉 This PR is included in version 5.0.0-alpha.6 🎉

The release is available on:

Your semantic-release bot 📦🚀

mattnolting requested review from mcoker and srambach and removed request for mcoker November 7, 2022 15:51

mattnolting force-pushed the 5030-fix-toolbar-alignment branch from b87c1be to 78f6c2d Compare November 8, 2022 20:16

mattnolting changed the base branch from main to v5 November 8, 2022 20:16

srambach approved these changes Nov 9, 2022

View reviewed changes

mcoker reviewed Nov 9, 2022

View reviewed changes

src/patternfly/components/Toolbar/toolbar.scss Outdated Show resolved Hide resolved

src/patternfly/components/Toolbar/toolbar.scss Outdated Show resolved Hide resolved

src/patternfly/components/Toolbar/toolbar.scss Outdated Show resolved Hide resolved

mattnolting requested a review from mcoker November 9, 2022 19:59

mcoker requested changes Nov 9, 2022

View reviewed changes

mattnolting force-pushed the 5030-fix-toolbar-alignment branch 2 times, most recently from 808cac3 to 716298b Compare November 10, 2022 01:20

mattnolting requested a review from mcoker November 10, 2022 01:21

mcoker reviewed Nov 10, 2022

View reviewed changes

mcoker mentioned this pull request Nov 11, 2022

Toolbar - support for vertical alignment variations patternfly/patternfly-react#8364

Closed

mattnolting force-pushed the 5030-fix-toolbar-alignment branch 2 times, most recently from cfbbd0e to 38da709 Compare November 16, 2022 13:36

mattnolting requested a review from mcoker November 16, 2022 13:37

mcoker reviewed Nov 19, 2022

View reviewed changes

mattnolting requested a review from mcoker November 22, 2022 15:31

mcoker approved these changes Nov 23, 2022

View reviewed changes

mcoker requested a review from srambach November 23, 2022 00:52

mcoker mentioned this pull request Dec 14, 2022

Search Input - client side validation #5089

Closed

mattnolting added 7 commits January 4, 2023 08:16

fix(toolbar): removed align baseline

082e453

fix(toolbar): added back baseline mods

27a6719

fix(toolbar): updated custom props to reg prop

1310987

update

7c201e7

update

995af35

update

62f74e5

fix(toolbar): updated select width logic

2b5e336

mattnolting force-pushed the 5030-fix-toolbar-alignment branch from 21a4dd4 to 2b5e336 Compare January 4, 2023 13:16

fix(toolbar): updated filter group visibility

6fd2f63

srambach approved these changes Jan 13, 2023

View reviewed changes

mcoker merged commit 895ad50 into patternfly:v5 Jan 13, 2023

patternfly-build added the released on @alpha label Jan 13, 2023

nicolethoen mentioned this pull request Jan 16, 2023

Toolbar - explore alignment; error validation throws off alignment #5030

Closed

mattnolting added a commit to mattnolting/patternfly that referenced this pull request Feb 15, 2023

fix(toolbar): added center and baseline alignments (patternfly#5201)

ff3940c

mattnolting added a commit to mattnolting/patternfly that referenced this pull request Feb 15, 2023

fix(toolbar): added center and baseline alignments (patternfly#5201)

2b1c69a

mattnolting added a commit to mattnolting/patternfly that referenced this pull request Feb 22, 2023

fix(toolbar): added center and baseline alignments (patternfly#5201)

4042825

mattnolting added a commit to mattnolting/patternfly that referenced this pull request Feb 22, 2023

fix(toolbar): added center and baseline alignments (patternfly#5201)

1a6e32b

mattnolting added a commit to mattnolting/patternfly that referenced this pull request Feb 22, 2023

fix(toolbar): added center and baseline alignments (patternfly#5201)

916b1e1

mattnolting added a commit to mattnolting/patternfly that referenced this pull request Feb 22, 2023

fix(toolbar): added center and baseline alignments (patternfly#5201)

c5cbf00

mattnolting added a commit to mattnolting/patternfly that referenced this pull request Mar 2, 2023

fix(toolbar): added center and baseline alignments (patternfly#5201)

f05ade8

mattnolting added a commit to mattnolting/patternfly that referenced this pull request Mar 2, 2023

fix(toolbar): added center and baseline alignments (patternfly#5201)

fbfdbde

mattnolting added a commit to mattnolting/patternfly that referenced this pull request Mar 2, 2023

fix(toolbar): added center and baseline alignments (patternfly#5201)

41f84af

mattnolting added a commit to mattnolting/patternfly that referenced this pull request Mar 2, 2023

fix(toolbar): added center and baseline alignments (patternfly#5201)

a9fec9f

mattnolting added a commit to mattnolting/patternfly that referenced this pull request Mar 2, 2023

fix(toolbar): added center and baseline alignments (patternfly#5201)

0dccc03

mattnolting added a commit to mattnolting/patternfly that referenced this pull request Mar 2, 2023

fix(toolbar): added center and baseline alignments (patternfly#5201)

ca2f4fb

mcoker mentioned this pull request May 4, 2023

Bug - Toolbar - examples with selects misaligned on firefox #5532

Closed

mattnolting added a commit to mattnolting/patternfly that referenced this pull request May 18, 2023

fix(toolbar): added center and baseline alignments (patternfly#5201)

8160887

mattnolting added a commit to mattnolting/patternfly that referenced this pull request Dec 12, 2023

fix(toolbar): added center and baseline alignments (patternfly#5201)

219abe3

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(toolbar): added center and baseline alignments #5201

fix(toolbar): added center and baseline alignments #5201

mattnolting commented Oct 31, 2022

patternfly-build commented Oct 31, 2022 •

edited

mattnolting commented Nov 8, 2022

mcoker left a comment

mattnolting commented Nov 9, 2022

mcoker Nov 9, 2022

mcoker Nov 9, 2022

mcoker Nov 9, 2022

mcoker Nov 9, 2022

mcoker left a comment

mattnolting commented Nov 16, 2022

mcoker left a comment

mcoker Nov 18, 2022

mattnolting Nov 22, 2022

mattnolting commented Nov 22, 2022

mcoker left a comment

srambach commented Dec 12, 2022

mattnolting commented Dec 16, 2022

srambach commented Jan 4, 2023

srambach left a comment

patternfly-build commented Jan 13, 2023

	.pf-c-toolbar__content,
	.pf-c-toolbar__content-section,
	.pf-c-toolbar__group,
	.pf-c-toolbar__item {
	align-self: stretch;
	}

	\| `.pf-m-align-items-baseline` \| `.pf-c-toolbar__content-section`, `.pf-c-toolbar__group` \| Modifies toolbar element to vertically align children to center. \|
	\| `.pf-m-align-items-baseline` \| `.pf-c-toolbar__content-section`, `.pf-c-toolbar__group` \| Modifies toolbar element to vertically align children to baseline. \|

fix(toolbar): added center and baseline alignments #5201

fix(toolbar): added center and baseline alignments #5201

Conversation

mattnolting commented Oct 31, 2022

patternfly-build commented Oct 31, 2022 • edited

mattnolting commented Nov 8, 2022

mcoker left a comment

Choose a reason for hiding this comment

mattnolting commented Nov 9, 2022

mcoker Nov 9, 2022

Choose a reason for hiding this comment

mcoker Nov 9, 2022

Choose a reason for hiding this comment

mcoker Nov 9, 2022

Choose a reason for hiding this comment

mcoker Nov 9, 2022

Choose a reason for hiding this comment

mcoker left a comment

Choose a reason for hiding this comment

mattnolting commented Nov 16, 2022

mcoker left a comment

Choose a reason for hiding this comment

mcoker Nov 18, 2022

Choose a reason for hiding this comment

mattnolting Nov 22, 2022

Choose a reason for hiding this comment

mattnolting commented Nov 22, 2022

mcoker left a comment

Choose a reason for hiding this comment

srambach commented Dec 12, 2022

mattnolting commented Dec 16, 2022

srambach commented Jan 4, 2023

srambach left a comment

Choose a reason for hiding this comment

patternfly-build commented Jan 13, 2023

patternfly-build commented Oct 31, 2022 •

edited