Select / ComboBox Open State isn't Persisted into Button's isPressed Render Prop

[ArrayKnight]

Provide a general summary of the issue here

A bit of an undocumented feature that doesn't seem to work completely as intended. Select and ComboBox both keep the button in the pressed state if the popover is in an open state:

https://github.com/adobe/react-spectrum/blob/main/packages/react-aria-components/src/Select.tsx#L170

https://github.com/adobe/react-spectrum/blob/main/packages/react-aria-components/src/ComboBox.tsx#L185

In the render props, this doesn't work and the isOpen render prop from the Select / ComboBox must be used

However, the data attributes on the Button do reflect the persisted pressed state. This inconsistency in state representation is undesirable

🤔 Expected Behavior?

It would be nice to isolate render functions as much as possible and not have them wrapped around so much of the children components. So, it would be desirable to be able to use the isPressed render prop from the Button to change icons based on state

😯 Current Behavior

IsPressed render prop from Button corresponds to user interaction and is not overridden by state provided by Select/Combobox as seems to be intended

💁 Possible Solution

No response

🔦 Context

No response

🖥️ Steps to Reproduce

https://codesandbox.io/p/sandbox/musing-maxwell-y3p2hy?workspaceId=9ca74201-e2f5-4f45-ad8e-200e6382b9bd

Version

RAC @ 1.3.1

What browsers are you seeing the problem on?

Chrome

If other, please specify.

No response

What operating system are you using?

Mac & Windows

🧢 Your Company/Team

No response

🕷 Tracking Issue

No response

[snowystinger]

Thanks for the issue, we're aware of the inconsistency and have been talking about ways to fix it without it being a breaking change.

[ArrayKnight]

Yea, it's a tricky corner you're in. Seems like there's only a few options, all with their drawbacks:

1. Update the render prop to match the data attribute. This could be considered a bug fix, but could potentially be a breaking change for some edge case
2. Update the data attribute to match the render prop (there by eliminating this feature). Similar solution reasoning as #1
3. Add a new isOpen render prop & data attribute to Button that carries this state through from props. This doesn't make a whole lot of sense since the Button shouldn't really understand the concept of open since it has no directly associated state. You'd end up likely pairing this solution with #2 anyways for overall consistency
4. Make the concept of render props mergeable. Essentially making it so that Select / ComboBox do not pass isOpen through the isPressed prop, but rather Select / ComboBox render props are passed down through a render props context. Any other component that implements render props, consumes this context and merges their own render props on top. This makes it so a component has access to all render props of all of its parents (so long as they weren't overridden by multiple instances of the same component)

That could look like:

// This wouldn't be a breaking change, but it would be a bit of
// a type nightmare in terms of an implementor not knowing
// which values are actually being fulfilled and by which parent
type FlatRenderProps = Partial<ButtonRenderProps> & Partial<SelectRenderProps> & ... // so on for all components

// This would be a breaking change since all render props
// are currently top level properties. But it has the advantage
// of making sure properties are clearly delineated
type NestedRenderProps = {
  button?: ButtonRenderProps;
  select?: SelectRenderProps;
  ... // so on for all components
}

[devongovett]

Looks like this was fixed in #7002. Issue doesn't reproduce anymore. https://codesandbox.io/p/devbox/racpopover-forked-88lspk?file=%2Fsrc%2FApp.tsx%3A31%2C42&workspaceId=ws_E77RHrAnKQ4LSjLHhBuDiz