-
Notifications
You must be signed in to change notification settings - Fork 98
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Toolbar component review #6701
Comments
Hidden/visible update:Duplicate selectors should be removed
Toolbar content display update:Retain this.
Min width update:Due to the complexity and size of this feature, it can be simplified to
Alignment update:Alignment modifiers work as expected, we should retain them.
Flex grow update:Flex grow modifier work as expected, we should retain it.
Alignment update:Alignment should be isolated to
Inset update:Insets can be isolated to
Wrapping update:Until we have a better approach to handling overflow, this is the simplest alternative as it allows overflowing
Gap update:
Margin update:Remove these.
|
🎉 This issue has been resolved in version 6.0.0-alpha.168 🎉 The release is available on: Your semantic-release bot 📦🚀 |
closed via #6838 |
Looking at some changes in #6057 and issues that have resulted, the toolbar styles should be reviewed as a whole for feature parity with react and cleaned up wherever possible. For example, it was determined in patternfly/patternfly-react#10418 (comment) that we didn't need the
.pf-m-margin-inline-[start/end]-[none/xs/sm/md/lg/xl/2xl/3xl/4xl]-on-[sm/md/lg/xl/2xl]
modifiers that were added. A quick test of pulling that out slims the toolbar stylesheet down from 2921 lines and 128k in size to 2273 lines and 96k. For comparison, the v5 toolbar stylesheet is around 50k, and the v6 stylesheet around 130k.--
Starting with the modifiers that generate a lot of CSS, review each of them for the following
Here is a rough list of the modifiers in question, but verify this in case there are any I missed:
Hidden/visible - supported on
content
,content-section
,group
,item
,group.pf-m-label-group-container
,group.pf-m-label-group
Why is it on
.#{$toolbar}__group
3 times?patternfly/src/patternfly/components/Toolbar/toolbar.scss
Lines 67 to 72 in 378c9b1
patternfly/src/patternfly/components/Toolbar/toolbar.scss
Lines 77 to 79 in 378c9b1
Width/min-width vars
patternfly/src/patternfly/components/Toolbar/toolbar.scss
Lines 189 to 191 in 378c9b1
Alignment on
item
andgroup
patternfly/src/patternfly/components/Toolbar/toolbar.scss
Lines 305 to 307 in 378c9b1
patternfly/src/patternfly/components/Toolbar/toolbar.scss
Lines 309 to 312 in 378c9b1
patternfly/src/patternfly/components/Toolbar/toolbar.scss
Lines 314 to 316 in 378c9b1
patternfly/src/patternfly/components/Toolbar/toolbar.scss
Lines 318 to 320 in 378c9b1
patternfly/src/patternfly/components/Toolbar/toolbar.scss
Lines 323 to 325 in 378c9b1
Alignment on
group
,item
,content
,content-section
patternfly/src/patternfly/components/Toolbar/toolbar.scss
Lines 355 to 357 in 378c9b1
Insets on
toolbar
,content
, andcontent-section
patternfly/src/patternfly/components/Toolbar/toolbar.scss
Lines 330 to 339 in 378c9b1
Wrapping on
group
,item
,content
,content-section
patternfly/src/patternfly/components/Toolbar/toolbar.scss
Lines 346 to 348 in 378c9b1
patternfly/src/patternfly/components/Toolbar/toolbar.scss
Lines 350 to 352 in 378c9b1
Gaps on
group
,item
,content
,content-section
patternfly/src/patternfly/components/Toolbar/toolbar.scss
Lines 361 to 364 in 378c9b1
patternfly/src/patternfly/components/Toolbar/toolbar.scss
Lines 369 to 371 in 378c9b1
patternfly/src/patternfly/components/Toolbar/toolbar.scss
Lines 373 to 375 in 378c9b1
Margins on
group
,item
,content
,content-section
- remove thesepatternfly/src/patternfly/components/Toolbar/toolbar.scss
Lines 377 to 383 in 378c9b1
Show/hide on
group.pf-m-toggle-group
patternfly/src/patternfly/components/Toolbar/toolbar.scss
Lines 389 to 399 in 378c9b1
patternfly/src/patternfly/components/Toolbar/toolbar.scss
Lines 402 to 411 in 378c9b1
The text was updated successfully, but these errors were encountered: