8000 fix(menu-toggle): dropped toggle from toggle-toggle by mattnolting · Pull Request #6361 · patternfly/patternfly · GitHub
[go: up one dir, main page]

Skip to content

fix(menu-toggle): dropped toggle from toggle-toggle#6361

Merged
mcoker merged 3 commits intopatternfly:v6from
mattnolting:fix-menu-toggle-caret
Mar 4, 2024
Merged

fix(menu-toggle): dropped toggle from toggle-toggle#6361
mcoker merged 3 commits intopatternfly:v6from
mattnolting:fix-menu-toggle-caret

Conversation

@mattnolting
Copy link
Collaborator

followup for #6349 (comment)

@patternfly-build
Copy link
Collaborator
patternfly-build commented Feb 27, 2024

Copy link
Contributor
@mcoker mcoker left a comment

Choose a reason for hiding this comment

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

I have a few questions that look like they were part of the PR to add the small variant (#6349)

Also I noticed the padding on the small plain button and small plain menu toggle are a little different. The button uses xs left/right padding, and the menu toggle uses sm left/right padding. Should those match? @lboehling @andrew-ronaldson


// * Menu toggle icon
--#{$menu-toggle}__icon--Height: calc(var(--#{$menu}__item--FontSize) * var(--#{$menu}__item--LineHeight));
--#{$menu-toggle}__icon--MinHeight: calc(var(--#{$menu}__item--FontSize) * var(--#{$menu}__item--LineHeight));
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to specify a height for this and the __toggle-icon?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated in #6349

{{#if menu-toggle--HasKebab}}
{{#> menu-toggle-toggle-icon}}
{{#> menu-toggle-icon}}
<svg class="pf-v5-svg" viewBox="0 0 192 512" fill="currentColor" aria-hidden="true" role="img" width="1em" height="1em"><path d="M96 184c39.8 0 72 32.2 72 72s-32.2 72-72 72-72-32.2-72-72 32.2-72 72-72zM24 80c0 39.8 32.2 72 72 72s72-32.2 72-72S135.8 8 96 8 24 40.2 24 80zm0 352c0 39.8 32.2 72 72 72s72-32.2 72-72-32.2-72-72-72-72 32.2-72 72z"></path></svg>
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious why this was changed to an SVG instead of the FA icon?

@mattnolting mattnolting force-pushed the fix-menu-toggle-caret branch from fe3a897 to 86cd879 Compare February 28, 2024 22:01
@mattnolting mattnolting force-pushed the fix-menu-toggle-caret branch from 86cd879 to eec851d Compare March 1, 2024 20:58
@mattnolting mattnolting force-pushed the fix-menu-toggle-caret branch from eec851d to a6f310e Compare March 4, 2024 03:43
@mcoker mcoker merged commit 60035b1 into patternfly:v6 Mar 4, 2024
@patternfly-build
Copy link
Collaborator

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

The release is available on:

Your semantic-release bot 📦🚀

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.

4 participants

0