fix: properly apply minwidth set by Tableview column menu

[LFDanLu]

No issue, from slack via quarry

✅ Pull Request Checklist:

• Included link to corresponding React Spectrum GitHub Issue.
• Added/updated unit tests and storybook for this change (for new code or code which already has tests).
• Filled out test instructions.
• Updated documentation (if it already exists for this component).
• Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

Inspect the wrapping div around the S2 TableView column resizing menu. It should have a min width applied to it now

🧢 Your Project:

RSP

[LFDanLu]

react-spectrum/packages/@react-spectrum/s2/src/TableView.tsx

Line 825 in a911b98

<Menu onAction={onMenuSelect} styles={style({minWidth: 128})}>

The above min-width wasn't making it through, resulting in cases like:

Still trying to get the exact reproduction details, but it felt like said min width (and other user provided menu styles) should make it through to the Popover since they don't get added to the Menu in this case

[snowystinger]

This would mean that styles are duplicated on AriaMenu and on wrapper div

But I think we actually want the minWidth (and other styles) on the Popover? And only applied isPopover is true?

[LFDanLu]

we skip applying the styles on the menu if isPopover is true, hence why on prod the minWidth wasn't making it through already:

react-spectrum/packages/@react-spectrum/s2/src/Menu.tsx

Line 397 in a911b98

className={menu({size, isPopover}, isPopover ? null : styles)}>

the above change is also within a isPopover branch (hard to see from just the diff) so there shouldn't be a duplication I believe:

react-spectrum/packages/@react-spectrum/s2/src/Menu.tsx

Lines 404 to 417 in a911b98

	if (isPopover) {
	return (
	<Popover
	ref={ref}
	padding="none"
	hideArrow>
	<div
	style={UNSAFE_style}
	className={(UNSAFE_className || '') + wrappingDiv}>
	{content}
	</div>
	</Popover>
	);
	}

[snowystinger]

gah i thought i'd checked that but my brain missed line 404 somehow

still a bit weird it goes on the div inside the popover and isn't applied to the Popover itself, with our changes to Popover, maybe that extra div is unnecessary now?

[LFDanLu]

I don't quite remember all the details but this was attempted prior to f9fc374#diff-0c586b0d4f573495a7818f614c64cb7293dd6d34ebdbce6f92f3dcd72009a073, but then we decided return to clamping down on allowed Popover style overrides hence bringing back the inner div.

The approach used in Menu falls in line with "as per discussion with team, if someone wants to modify their popover internals, they are expected to add a inner wrapping div themselves and turn off padding like custom dialog"

[snowystinger]

Thanks for the history again

[rspbot]

Build successful! 🎉

• View the storybook
• View the S2 storybook
• View the S2 docs
• View the React Aria docs