CustomSelect: adjust renderSelectedValue to fix sizing

[brookewp]

What?

When creating the component, we introduced logic to have better default values for rendered select values. However, this affected the sizing styles, which relied on the boolean hasCustomRenderProp to check if renderSelectedValue is true or not. In trunk, renderSelectedValue will always return true so the styles aren't applying as expected.

Why?

To ensure the component size can be changed via the size prop.

How?

To fix this, we can remove defaultRenderSelectedValue as the default value for renderSelectedValue and instead invoke the function only when renderSelectedValue is true.

If renderSelectedValue is false, the currentValue will be used, and if that is empty, Ariakit uses the first available value.

Testing Instructions

1. npm run storybook:dev
2. Go to CustomSelectControl V2 story
3. Ensure the styles for size small are applied to the 'Default' story (they won't apply to the other stories, where hasCustomRenderProp is true.

[ciampo]

Nice find! I believe that we can achieve the same result with fewer changes, and while keeping the defaultRenderSelectedValue external to the component's render function.

We could remove the default assignment to the renderSelectedValue (as you already did in this PR), and basically move the "use the default value if no value is provided" logic in the components' render function — something like

const computedRenderSelectedValue = renderSelectedValue ?? defaultRenderSelectedValue

We could then use computedRenderSelectedValue instead of renderSelectedValue in the JSX code.

What do you think?

[brookewp]

Nice find! I believe that we can achieve the same result with fewer changes, and while keeping the defaultRenderSelectedValue external to the component's render function.

We could remove the default assignment to the renderSelectedValue (as you already did in this PR), and basically move the "use the default value if no value is provided" logic in the components' render function — something like

const computedRenderSelectedValue = renderSelectedValue ?? defaultRenderSelectedValue

We could then use computedRenderSelectedValue instead of renderSelectedValue in the JSX code.

What do you think?

Sounds great! I've implemented your solution in: dc2635e

[ciampo]

LGTM 🚀

Good to merge after adding a CHANGELOG entry :)

[github-actions]

Flaky tests detected in 96f0890.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/7563884228
📝 Reported issues:

• [Flaky Test] User Mention: should insert mention between two other words #48774 in /test/e2e/specs/editor/various/autocomplete-and-mentions.spec.js

[ciampo]

There are some e2e failures (Firefox getting a 500 server error) that are not related to the changes from this PR, especially because the affect code is not used nor exported by the @wordpress/components package. Therefore, I'm going to merge this PR despite required checks not passing.