fix: ensure menu-button's menu width fits the largest menu item #3109
Conversation
chrisolsen
force-pushed
the
chris/menu-button-width-fix
branch
6 times, most recently
from
047d30b to
fb51ff2
Compare
| on:_open={open} | ||
| padded="false" | ||
| tabindex="-1" | ||
| maxwidth="" |
There was a problem hiding this comment.
Can we add a comment or even better with maxwidth="none" (add a new option "none" for Popover), otherwise later we will forget why we have this added at the first place?
chrisolsen
force-pushed
the
chris/menu-button-width-fix
branch
from
fb51ff2 to
b282270
Compare
chrisolsen
force-pushed
the
chris/menu-button-width-fix
branch
from
b282270 to
f8bd734
Compare
| import { By } from "@angular/platform-browser"; | ||
| import { fireEvent } from "@testing-library/dom"; | ||
|
|
||
| @Component({ |
There was a problem hiding this comment.
This test failed because after angular 20 upgrade, testbed likes to have everything standalone.
After this change it will pass the test:
| padding: 0 var(--goa-button-padding-lr); | ||
| gap: var(--goa-button-gap); | ||
| align-items: center; /* for leading and trailing icon vertical alignment */ | ||
| white-space: nowrap; |
There was a problem hiding this comment.
Because of this, long menu items don't wrap, and the Popover itself doesn't respect the max-width set, so it can go grow forever. This especially leads to issues with mobile, where the menu can literally be off screen if it's large enough.
There was a problem hiding this comment.
Also because of this, the Popover can expand to infinite width potentially, as there's nothing that can be used to control its width, so it just grows infinitely to fit the content.
| slot="target" | ||
| leadingicon={leadingIcon} | ||
| {type} | ||
| trailingicon={_icon} |
There was a problem hiding this comment.
The acceptance criteria on the story asks that trailingicon can be changed by the user as well, so it doesn't necessarily have to be the chevron-down, this would instead be itss default.
| font: var(--goa-button-text); | ||
| height: var(--goa-button-height); | ||
| letter-spacing: var(--goa-button-letter-spacing); | ||
| padding: 0 var(--goa-button-padding-lr); |
There was a problem hiding this comment.
For some reason, I'm not sure why. If you use a longer menu action then has room to display on the right side, in Angular it then displays it going to the left, but in React, it seems to display it centred if it can.
Angular:
React:
| style("width", width), | ||
| style("min-width", minwidth), | ||
| style("max-width", width ? `max(${width}, ${maxwidth})` : maxwidth), | ||
| style("max-width", maxwidth === "none" ? "" : maxwidth), |
There was a problem hiding this comment.
With this change, Dropdown is impacted. The Popover in Dropdown no longer expands to the width of the Dropdown, but instead has a max width of 320px now. I think this might be because Dropdown passes _popoverMaxWidth to the width attribute of Popover, instead of maxwidth.
This PR:
Previously:
Before (the change)
After (the change)
Make sure that you've checked the boxes below before you submit the PR
Steps needed to test