feat(Button/MenuToggle): added support for hamburger/settings animations by thatblindgeye · Pull Request #11861 · patternfly/patternfly-react

thatblindgeye · 2025-06-05T21:36:17Z

What: Closes #11370

Commit 1 adds component logic and new examples.
Commit 2 updates existing examples/demos to use these new implementations.
Commit 6 adds new tests for these new features
Commit 7 makes updates from 6/10/25 animations sync meeting to not restrict the variant nor children for either type of new button (can compare to Commit 1 and 6)
Commit 8 cleans up some examples and prop verbiage

Questiosn:

What do we want to do with the "with icons" MenuToggle example? It's currently using the CogIcon, same as the new "settings" toggle. I guess it depends if we would want to ship the "settings" toggle with an icon hardcoded, or expect consumers to still pass in their own icon and isSettings just applies the necessary modifier class.
Would we want to keep the Hamburger button example I added that includes a potential approach for making the animation dynamic (the 4th button in that example)?
For the PageToggleButton component, would we plan in the next breaking change to force the new hamburger icon with animations, or do we want to allow a consumer to pass their own icon in there?

Assumptions I had made in the code that can use design input:

A "hamburger" and "settings" button are setup so that a consumer won't be able to override the icon nor the variant -- they will always render as plain buttons with their respective icons. A "hamburger" button also cannot be given any text content, but a "settings" button can have text in addition to the icon.
- Main question here is if we want to allow consumers to change the variant for either of these types of buttons.
Related to above, the "settings" MenuToggle currently does allow the variant to be customized, but overrides the icon prop (so a consumer is stuck with the CogIcon we ship a "settings" MenuToggle with, but it can be a plain, primary, secondary, etc MenuToggle).

Additional issues:

patternfly-build · 2025-06-05T21:37:40Z

Preview: https://patternfly-react-pr-11861.surge.sh

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

andrew-ronaldson · 2025-06-06T00:01:40Z

Would we want to keep the Hamburger button example I added that includes a potential approach for making the animation dynamic (the 4th button in that example)?

We could have a custom button section for the favorite, settings, hamburger and future wacky buttons.

For the PageToggleButton component, would we plan in the next breaking change to force the new hamburger icon with animations, or do we want to allow a consumer to pass their own icon in there?

I don't know that we'd force a full change but what if in the next breaking change the animated one is the default and the old version a variant?

thatblindgeye · 2025-06-06T14:11:06Z

packages/react-core/src/components/ClipboardCopy/ClipboardCopyToggle.tsx

    id={id}
    aria-labelledby={`${id} ${textId}`}
-    aria-controls={contentId}
+    aria-controls={isExpanded ? contentId : undefined}


This update needs to be made as it resolves a critical axe error where the aria-controls attr references a non-existent ID (since the expandable content is dynamically rendered)

thatblindgeye · 2025-06-10T18:23:53Z

From 6/10/25 animations sync meeting: Remove logic that restricts variant and children, replace cog icon in "with icons" with plus icon, remove 4th hamburger button example

rebeccaalpert

Code looks fine to me!

Not worth blocking imho since this is large already, but demo screenshots appear to be PF5?

srambach · 2025-06-11T20:58:05Z

At 1200px, the arrow version of the hamburger briefly flashes - is there a > vs. >= sort of thing going on that could be smoothed out?

2025-06-11_16-52-46.mp4

andrew-ronaldson

🍔

kaylachumley

looking good

thatblindgeye · 2025-06-13T12:24:37Z

Per discussion between @rebeccaalpert , @mcoker and myself yesterday, we decided to keep this custom hamburger icon in the codebase (just moved to the Button directory) since it's such a specific icon that requires being wrapped in a pf-v[version]-c-button element. When attempted to be added to the react-icons package as a custom icon, the icon was rendering as essentially nothing visually in the preview build, since the icons there aren't wrapped in that button class.

mcoker

LGTM! Left a couple of nit comments, but they don't seem important. Maybe something to follow-up on

mcoker · 2025-06-13T14:48:09Z

packages/react-core/src/components/Button/hamburgerIcon.tsx

+// Because this is such a specific icon that requires being wrapped in a pf-v[current version]-c-button element,
+// we don't want to export this to consumers nor include it in the react-icons package as a custom icon.
+export const hamburgerIcon = (
+  <svg viewBox="0 0 10 10" className="pf-v6-c-button--hamburger-icon pf-v6-svg" width="1em" height="1em">


nit - can you import the react-tokens for these classes and use token.name to populate the classes instead of hardcoding them?

mcoker · 2025-06-13T14:51:40Z

packages/react-core/src/components/MenuToggle/examples/MenuToggleSettings.tsx

+import { MenuToggle, Flex } from '@patternfly/react-core';
+
+export const MenuToggleSettings: React.FunctionComponent = () => (
+  <Flex>


nit - just an inconsistency, but I noticed the updated custom icon example puts {' '} between the items for a gap instead of wrapping them in <Flex />.

patternfly-build · 2025-06-13T17:44:30Z

Your changes have been released in:

@patternfly/[email protected]
@patternfly/[email protected]
@patternfly/[email protected]
@patternfly/[email protected]
@patternfly/[email protected]
@patternfly/[email protected]
@patternfly/[email protected]
[email protected]
@patternfly/[email protected]
@patternfly/[email protected]
@patternfly/[email protected]
@patternfly/[email protected]

Thanks for your contribution! 🎉

thatblindgeye changed the title ~~Iss11370 setting hamburg btns~~ feat(Button/MenuToggle): added support for hamburger/settings animations Jun 5, 2025

thatblindgeye requested review from andrew-ronaldson, kmcfaul, mcoker and nicolethoen June 5, 2025 21:36

andrew-ronaldson requested a review from kaylachumley June 6, 2025 00:01

thatblindgeye commented Jun 6, 2025

View reviewed changes

thatblindgeye marked this pull request as ready for review June 6, 2025 14:14

nicolethoen approved these changes Jun 6, 2025

View reviewed changes

thatblindgeye mentioned this pull request Jun 9, 2025

react-icons - extend createIcon for multi-path & add hamburger icon #11858

Closed

rebeccaalpert approved these changes Jun 11, 2025

View reviewed changes

thatblindgeye mentioned this pull request Jun 11, 2025

Update React screenshots for Q2 release #11866

Closed

thatblindgeye force-pushed the iss11370_SettingHamburgBtns branch from 27f3073 to 1ef2e90 Compare June 11, 2025 21:49

andrew-ronaldson approved these changes Jun 12, 2025

View reviewed changes

kaylachumley approved these changes Jun 12, 2025

View reviewed changes

mcoker approved these changes Jun 13, 2025

View reviewed changes

thatblindgeye force-pushed the iss11370_SettingHamburgBtns branch from 7f8c60a to f4f6553 Compare June 13, 2025 15:28

edonehoo approved these changes Jun 13, 2025

View reviewed changes

Eric Olkowski added 7 commits June 13, 2025 13:01

feat(Button/MenuToggle): added support for hamburger/settings animations

2f30ccf

Updated existing examples and demos

721e9bb

Bumped core versions, fixed build failure

998a5e0

Updated failing tests and a11y failures

542fd01

Updated existing tests

521f26c

Added new tests

81fa095

Removed restriction of variant and children content

2c0278c

Eric Olkowski added 6 commits June 13, 2025 13:02

Updated examples and prop descriptions

ed089c4

Bumped core version to resolve hamburger icon shift on hover

c5f5c0b

Fixed width bug thanks to Coker and Sarah

22286d6

Doesnt look like anything to me

3bfc851

Moved location of custom hamburger icon

dfeab37

Cleaned up from rebase and feedback

e106295

thatblindgeye force-pushed the iss11370_SettingHamburgBtns branch from f4f6553 to e106295 Compare June 13, 2025 17:02

Updated yarn lock

5b7b557

rebeccaalpert merged commit cf3d84e into patternfly:main Jun 13, 2025
13 checks passed

thatblindgeye mentioned this pull request Jun 13, 2025

Button - update console error for accessible name and move aria-expanded location #11871

Open

Conversation

thatblindgeye commented Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

patternfly-build commented Jun 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andrew-ronaldson commented Jun 6, 2025

Uh oh!

thatblindgeye Jun 6, 2025

Choose a reason for hiding this comment

Uh oh!

thatblindgeye commented Jun 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rebeccaalpert left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

srambach commented Jun 11, 2025

Uh oh!

andrew-ronaldson left a comment

Choose a reason for hiding this comment

Uh oh!

kaylachumley left a comment

Choose a reason for hiding this comment

Uh oh!

thatblindgeye commented Jun 13, 2025

Uh oh!

mcoker left a comment

Choose a reason for hiding this comment

Uh oh!

mcoker Jun 13, 2025

Choose a reason for hiding this comment

Uh oh!

mcoker Jun 13, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

patternfly-build commented Jun 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

thatblindgeye commented Jun 5, 2025 •

edited

Loading

patternfly-build commented Jun 5, 2025 •

edited

Loading

thatblindgeye commented Jun 10, 2025 •

edited

Loading

rebeccaalpert left a comment •

edited

Loading