calcite-components-examples icon indicating copy to clipboard operation
calcite-components-examples copied to clipboard

Bug: react's rendering is breaking radio group state

Open yannikmesserli opened this issue 2 years ago • 9 comments

When the re-rendering of a CalciteRadioGroup is delayed to update the checked value, it leaves the underlying calcite component in a double checked state: Screenshot 2022-11-16 at 08 42 25

Repro sample

This does not happen in blank calcite, check this out.

yannikmesserli avatar Nov 16 '22 07:11 yannikmesserli

This is a known issue described in the Frameworks Gotcha's section: https://developers.arcgis.com/calcite-design-system/frameworks/#boolean-attributes

Here is a working version of your sample https://codesandbox.io/s/ccr-boolean-gotcha-0464ie

Closing this issue, but if you have further questions feel free to continue the discussion

benelan avatar Nov 29 '22 21:11 benelan

Hi @benelan, thanks for the reply. I can still see the described bug in your " working version". If you click once on "Apple" and then, after it re-renders, back on "Orange", you can still see both checkbox being checked at once.

yannikmesserli avatar Dec 01 '22 15:12 yannikmesserli

The delay was due to the setTimeout you had in the codesandbox. I removed it from mine, are you still seeing an issue?

benelan avatar Dec 01 '22 23:12 benelan

Oh okay I see what you mean now. What is the use case for delaying the responsiveness of the UI? It looks like React's synthetic events are behaving differently

benelan avatar Dec 02 '22 01:12 benelan

Ok, I see, I wasn't clear enough about the encountered bug. The bug I am reporting here is that the component, when clicked, turns his UI to the "checked" mode without respecting the actual "checked" property (or its absence). So, you end up with BOTH items being checked in the example: 2022-12-02 10 43 34

Since this is not happening in plain calcite, this is probably due to the react rendering setting the checked value onclick.

yannikmesserli avatar Dec 02 '22 09:12 yannikmesserli

Yeah now I see the difference you're talking about. But what is the use case for adding the delay? React's rendering or synthetic events may execute in a different order, but without the delay the difference isn't noticable.

benelan avatar Dec 02 '22 21:12 benelan

But what is the use case for adding the delay?

In our app, the delay is caused by posting to the server and waiting for the result. Here, I am just adding the timeout to exhibit the bug.

yannikmesserli avatar Dec 07 '22 15:12 yannikmesserli

Why do you need to interact with the server to change the radio group UI state? In general creating UI lag isn't the best approach for indicating that something needs to load. Could the the radio group state change without delay, and then if the app needs something from a server it could display a loader or progress bar until it gets a response?

I'll reopen the issue and investigate after the v1 release because I can reproduce. But I think there can be some UI optimization in the app as well. Thanks for reporting by the way, and sorry for closing so quickly. We get a lot of issues/questions posted due to the Gotcha I linked, and I jumped the gun a bit because it looked similar.

benelan avatar Dec 09 '22 04:12 benelan

This seems to happen with React 18 using createRoot to render the app instead of ReactDOM.render(). Seems to work fine on React < 18 or when using React 18 with ReactDOM to render the app.

eriklharper avatar Sep 28 '23 21:09 eriklharper