8000 fix(menu-toggle): added support for validation/status by mcoker · Pull Request #6441 · patternfly/patternfly · GitHub
[go: up one dir, main page]

Skip to content

fix(menu-toggle): added support for validation/status#6441

Merged
mcoker merged 2 commits intopatternfly:v6from
mcoker:issue-6243
Mar 22, 2024
Merged

fix(menu-toggle): added support for validation/status#6441
mcoker merged 2 commits intopatternfly:v6from
mcoker:issue-6243

Conversation

@mcoker
Copy link
Contributor
@mcoker mcoker commented Mar 19, 2024

fixes #6243

@patternfly-build
Copy link
Collaborator
patternfly-build commented Mar 19, 2024

--#{$menu-toggle}__status-icon--MarginInlineEnd: var(--pf-t--global--spacer--md);

// Success
--#{$menu-toggle}--m-success--after--BorderBottomColor: var(--pf-t--global--border--color--status--success--default);
Copy link
Contributor

Choose a reason for hiding this comment

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

Same would apply to warning and danger vars + where they're used:

Suggested change
--#{$menu-toggle}--m-success--after--BorderBottomColor: var(--pf-t--global--border--color--status--success--default);
--#{$menu-toggle}--m-success--BorderColor: var(--pf-t--global--border--color--status--success--default);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

d'oh! copy/pasta fail 🤦‍♂️

@mcoker mcoker requested a review from thatblindgeye March 19, 2024 18:56
@mcoker mcoker linked an issue Mar 19, 2024 that may be closed by this pull request
Copy link
Collaborator
@andrew-ronaldson andrew-ronaldson left a comment

Choose a reason for hiding this comment

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

Tis a fine PR it is!

Copy link
@lboehling lboehling left a comment

Choose a reason for hiding this comment

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

I am noticing that there isn't as much padding to the right of the caret compared to the other menu toggles, but otherwise looks good!

Copy link
Member
@srambach srambach left a comment

Choose a reason for hiding this comment

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

Looks good. Definitely a candidate for a focus ring adjustment but that'll come later. 🚀

@srambach
Copy link
Member

I am noticing that there isn't as much padding to the right of the caret compared to the other menu toggles, but otherwise looks good!

I was trying to compare the caret spacing too, but decided design could weigh in on that! Can always tweak that later if it bugs anyone.

@mcoker mcoker merged commit 387c69d into patternfly:v6 Mar 22, 2024
@patternfly-build
Copy link
Collaborator

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

The release is available on:

Your semantic-release bot 📦🚀

@mcoker
Copy link
Contributor Author
mcoker commented Mar 22, 2024

Opened #6458 to track that spacing update from #6441 (review)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Menu toggle - support for validation/status in penta

6 participants

0