Empty state message displays inside scrollbar with custom SelectPanel height

[francinelucca]

When using a custom height in SelectPanel (e.g., height: medium), the empty state message is improperly rendered within a scrollbar, even when there is ample space to display the message without a scrollbar. This can be observed in the storybook example for height variations and scroll (https://primer.style/react/storybook/?path=/story/components-selectpanel-examples--height-variations-and-scroll&globals=featureFlags.primer_react_css_modules_ga:!true;featureFlags.primer_react_select_panel_with_modern_action_list:!true). If you type a term such as "dddd" into the search bar, the empty state appears inside a scrollable area, which is not expected.

See attached screenshot for reference:

Expected behavior:

• The empty state message should display without being constrained by an unnecessary scrollbar when there is enough space in the panel.

Steps to reproduce:

1. Open the storybook example for SelectPanel height variations and scroll.
2. Set the height to medium.
3. Type a string like "dddd" in the searchbar to trigger the empty state message.
4. Note that the empty state message appears within a scrollable area, even if there's enough space for it to display fully.

Environment:

• Storybook: https://primer.style/react/storybook/?path=/story/components-selectpanel-examples--height-variations-and-scroll&globals=featureFlags.primer_react_css_modules_ga:!true;featureFlags.primer_react_select_panel_with_modern_action_list:!true

[KirankumarAmbati]

@francinelucca We have min-height set for the SelectPanelMessage component here. Removing it works as expected.

Tried removing it and tested normal & overflow scenarios. Seems like things are working as expected. Please let me know if that's the correct fix, I can raise a PR for the same.

[francinelucca]

@KirankumarAmbati it'd be good to still set a min-height on it, I wonder if the problem is the min() condition, maybe we can try removing the default 324px and see where that gets us?

[KirankumarAmbati]

@francinelucca

tldr; Removing 324px would not help us in this scenario since the minHeight calculation gives us 282px, which is less than the default one and gets preferred in min calculation.

Explanation: For the first example in the storybook, we are not setting any maxHeight overlap prop. As per this code, the maxHeight is automatically considered as large, which is 432px (from here). And the calculation to calculate minHeight is done here.

In this scenario, minHeight becomes 432 - 150 = 282px and min(282px, 324px) => 282px.

[github-actions]

Uh oh! @KirankumarAmbati, at least one image you shared is missing helpful alt text. Check #6220 (comment) to fix the following violations:

• Images should have meaningful alternative text (alt text) at line 9

Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image.

Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs.

🤖 Beep boop! This comment was added automatically by github/accessibility-alt-text-bot.

[francinelucca]

Ahh I see, thanks for explaining that.
Could we use the height then for the calculation, instead of the maxHeight? And then if there's no height set then maybe it doesn't need a minHeight

[KirankumarAmbati]

@francinelucca Sounds good! I'll try this and will update my findings.

[KirankumarAmbati]

@francinelucca I just created a draft PR. Removed the minHeight altogether and still didnt observe any difference in the UI between current vs new. Attached screen grabs in the PR. Please have a look. If it is looking good, I will move it for review. If not, I'll modify the changes. Thanks!