[labs/react] boolean attributes should allow false to be more react-like

[ashleyryan]

Description

A boolean attribute, by definition, is true when it exists on the element, regardless of the value. There is no hidden="false", the false value would be removing the attribute. React allows false values for things like checked, so it's confusing to have to do something like hidden={showElement ? undefined : true} on a boolean attribute.

We have this issue on our Modals.

Steps to Reproduce

set hidden={false} on a CdsModal and notice that it does not show, as expected.

Live Reproduction Link

https://stackblitz.com/edit/clarity-react-8em3m2?file=src%2FApp.tsx

Expected Results

The hidden prop can accept a value of false and behave like undefined.

Actual Results

false is the same as true

Browsers Affected

• Chrome
• Firefox
• Edge
• Safari 11
• Safari 10
• IE 11

[ashleyryan]

I've been looking into workarounds on our side some more and it looks like React will actually handle this properly in React 19 once they release the proper web component support. They have a merged PR, behind the experimental flag, that will convert boolAttr={false} to null instead of "false"

facebook/react#24541

[justinfagnani]

This is a good thing for us to do to match the behavior of React 19's WC support.

[taylor-vann]

I'd imaging something similar with disabled occurs. Thanks for submitting, will look over and reply

[ashleyryan]

In our case, the issue was that we had a CSS selector on [hidden] that hid the element. We were able to work around the issue by handling the false value in the selector.

But the "false" string value doesn't follow the convention of the hidden (or disabled attribute) like you mentioned, so yes, it will probably be an issue for that attribute as well. Thanks for looking into it!

[mjaggard]

It seems the workaround listed here doesn't work either. hidden={myBoolean ? true : undefined} still renders hidden in the DOM just without a value (which means hidden)

I've also tried hidden={myBoolean ? true : null} but that is invalid because Type 'true | null' is not assignable to type 'boolean | undefined'.

[taylor-vann]

Hey @ashleyryan @mjaggard! We have updates!

We've updated how we apply react props onto web components inside the wrapper.

Properties like hidden and id should now be handled correctly for react 17, 18, and experimental with v1.1.0

[taylor-vann]

Want to confirm that boolean properties like hidden work as expected in the wrapper.

However there are other attributes which are not boolean that take boolean values like id.
In react-land you can technically set <div id={false} /> and the id will be removed.

There are several other properties with string values that behave this way. This is very much not how the browser works.
It won't be possible to programmatically cover these edge cases. It would require a checklist.

However #3128 moves the web component wrapper much closer to react behavior than before.

[ashleyryan]

Thanks for this @taylor-vann - I no longer work on the project that was having the issue, but I'll pass it on to the team. I did clone the repo and checked it out really quick and it most of the jest tests have passed as expected when I remove the higher order component I build to convert false to undefined. One is failing still and I haven't had a chance to dig into it yet, but it looks good for the most part!

[taylor-vann]

@ashleyryan All but one test? I'll take that hahah nice :) Very curious about the last test.
The fact that it passed most of y'alls tests is really encouraging.

[ashleyryan]

Figured it out! It was an async timing issue where I was checking for the hidden attribute before the first render was finished. It passes, looks like our workaround can safely be removed. Thanks for fixing this!

I should note that I only had this issue with the hidden attribute on a few components - I didn't use anything like the id example you gave above. I also think we had a component with an attribute that took a "true" or "false" or undefined value as 3 separate things, so we should check that one as well. (But again I'm not on that team or at that company anymore, so I'll give them a nudge)

[taylor-vann]

Thanks so much @ashleyryan for the extra help, I really appreciate it.

Im going to mark this issue as resolved from #3128