Skip to content

fix(toolbar-action-list-overflow-menu): added tokens, new design spec#6057

Merged
mcoker merged 4 commits into
patternfly:v6from
mattnolting:chore-toolbar-action-list-overflow-menu-5713
Jan 17, 2024
Merged

fix(toolbar-action-list-overflow-menu): added tokens, new design spec#6057
mcoker merged 4 commits into
patternfly:v6from
mattnolting:chore-toolbar-action-list-overflow-menu-5713

Conversation

@mattnolting

@mattnolting mattnolting commented Nov 13, 2023 •
edited
Loading

Copy link
Copy Markdown
Collaborator

@patternfly-build

patternfly-build commented Nov 13, 2023 •
edited
Loading

Copy link
Copy Markdown
Collaborator

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

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

@mattnolting mattnolting force-pushed the chore-toolbar-action-list-overflow-menu-5713 branch 2 times, most recently from b5d17f6 to 39092dd Compare November 13, 2023 18:53
@srambach srambach linked an issue Nov 14, 2023 that may be closed by this pull request
Consume tokens in: Toolbar and Action list #5713
Closed
@mattnolting mattnolting force-pushed the chore-toolbar-action-list-overflow-menu-5713 branch from c5b3d4c to 9efd8ea Compare November 14, 2023 15:14
@mattnolting mattnolting marked this pull request as ready for review November 14, 2023 15:15
@evwilkin evwilkin mentioned this pull request Dec 13, 2023
Closed
@mattnolting mattnolting changed the title chore(toolbar-action-list-overflow-menu): added tokens, refactored to… chore(toolbar-action-list-overflow-menu): added tokens, new design spec Dec 15, 2023
@andrew-ronaldson

andrew-ronaldson commented Jan 8, 2024

Copy link
Copy Markdown
Collaborator

The preview seems to be broken on this one?

@mattnolting mattnolting changed the base branch from v6-old to v6 January 9, 2024 13:09
@mattnolting mattnolting force-pushed the chore-toolbar-action-list-overflow-menu-5713 branch 16 times, most recently from 038fcdd to 1b2a02f Compare January 12, 2024 19:13
srambach
srambach reviewed Jan 12, 2024
View reviewed changes

@srambach srambach left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to remove the test toolbar, and there are zindex tokens now, then I can't see anything else obvious, so I think we can move ahead! 🚗

Comment thread src/patternfly/components/Toolbar/toolbar.scss Outdated
@mattnolting mattnolting force-pushed the chore-toolbar-action-list-overflow-menu-5713 branch 3 times, most recently from a6cba8a to 0157db3 Compare January 16, 2024 14:26
@mattnolting mattnolting force-pushed the chore-toolbar-action-list-overflow-menu-5713 branch 5 times, most recently from 364e7c4 to 6952436 Compare January 16, 2024 15:28
thatblindgeye
thatblindgeye reviewed Jan 16, 2024
View reviewed changes

@thatblindgeye thatblindgeye left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a question related to resolved comment above. Seems like there could be other instances of using fallbacks where we could (should?) instead define vars and not need a fallback elsewhere in the various scss files.

Action list for example has line 32 row-gap: var(--#{$action-list}__group--RowGap, var(--#{$action-list}--RowGap));,

In other cases, where something like menu shares row-gap values, I much prefer applying this fallback method.

Would you mind providing a quick example of this? Not sure if it's me just not understanding or it still feeling like a Monday morning (or both) 😆

@mattnolting

mattnolting commented Jan 16, 2024 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

@thatblindgeye Using menu as an example, item, header, group-title, breadcrumb, and footer share menu__item's padding values. We could list these by instance, but this approach essentially does exactly that. It eliminates four lines of code for each use. In this example, ~ 36 lines of code are redundant.

Screenshot 2024-01-16 at 12 06 20 PM

Screenshot 2024-01-16 at 11 53 42 AM

Screenshot 2024-01-16 at 11 54 16 AM

Screenshot 2024-01-16 at 11 53 59 AM

Screenshot 2024-01-16 at 11 54 21 AM

Screenshot 2024-01-16 at 11 54 34 AM

Screenshot 2024-01-16 at 11 54 39 AM

Does that address your question?

thatblindgeye
thatblindgeye approved these changes Jan 16, 2024
View reviewed changes

@thatblindgeye thatblindgeye left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah gotcha! Thanks for the explanation, 💯% answers it

@mattnolting mattnolting force-pushed the chore-toolbar-action-list-overflow-menu-5713 branch from 6952436 to e5e2c00 Compare January 17, 2024 20:18
srambach
srambach approved these changes Jan 17, 2024
View reviewed changes

@srambach srambach left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😵‍💫

@mcoker mcoker changed the title chore(toolbar-action-list-overflow-menu): added tokens, new design spec fix(toolbar-action-list-overflow-menu): added tokens, new design spec Jan 17, 2024
@mcoker mcoker merged commit 252824f into patternfly:v6 Jan 17, 2024
@patternfly-build

patternfly-build commented Jan 17, 2024

Copy link
Copy Markdown
Collaborator

🎉 This PR is included in version 6.0.0-alpha.64 🎉

The release is available on:

Your semantic-release bot 📦🚀

@mcoker mcoker mentioned this pull request Feb 20, 2024
Closed
@mcoker mcoker mentioned this pull request Apr 1, 2024
Closed
@mcoker mcoker mentioned this pull request May 8, 2024
Closed
This was referenced May 21, 2024
Closed
Closed
@patternfly-build patternfly-build mentioned this pull request Apr 16, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@srambach srambach srambach approved these changes

@thatblindgeye thatblindgeye thatblindgeye approved these changes

@mcoker mcoker Awaiting requested review from mcoker

Assignees

No one assigned

Labels

released on @alpha

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Consume tokens in: Toolbar and Action list

6 participants

@mattnolting @patternfly-build @andrew-ronaldson @srambach @mcoker @thatblindgeye