SAP / ui5-webcomponents-react

A wrapper implementation for React of the UI5 Web Components that are compliant with the SAP Fiori User Experience
https://sap.github.io/ui5-webcomponents-react/
Apache License 2.0
426 stars 94 forks source link

CheckBox: Visible checked state of the Checkbox changes even checked property is set to specific value #5971

Open sadie100 opened 1 week ago

sadie100 commented 1 week ago

Describe the bug

Even if I fix the checked property of the CheckBox to false(or true), the visible checked state of the CheckBox changes every time when the user clicks on it. This is unexpected, because the basic html checkbox input(input type='checkbox') acts as frozen if the checked property is fixed with certain value. (I added this case on the StackBlitz example for compare)

If this is by design and not a bug, please consider fixing the feature. We can't control the value now. I need to connect some data logic to the CheckBox's onChange property, and I want the 'checked' property to be controlled by our state. (So that we can cancel the check action if the data logic fails). But now the property is always changing regardless of 'checked', so there's no way to control the displayed checked property.

I've also checked the Switch component which has 'checked' property too, and it behaves the same way. I'd appreciate it if you could look at that as well.

Isolated Example

https://stackblitz.com/edit/github-kacj2o-fej5h6?file=src%2FApp.tsx

Reproduction steps

  1. Set the CheckBox checked property to certain value(true or false)
  2. Click the CheckBox

Expected Behaviour

If checked value is not changed, UI should not be changed.

Screenshots or Videos

No response

UI5 Web Components for React Version

1.29.0

UI5 Web Components Version

1.24.0

Browser

Chrome

Operating System

No response

Additional Context

No response

Relevant log output

No response

Organization

No response

Declaration

Lukas742 commented 1 week ago

HI @sadie100

to fully control the component's checked state you can call event.preventDefault in the change handler of the components:

      <CheckBox
        checked={false}
        onChange={(event) => {
          event.preventDefault();
        }}
      />

https://stackblitz.com/edit/github-kacj2o-6w3pho?file=src%2FApp.tsx

Does this solve your issue?

sadie100 commented 6 days ago

Ooh it works! Thanks for help ☺️

I hope there's some explanation in CheckBox's document that the event.preventDefault() handling is required to control the component. There may be a lot of users who think they can control the component without adding the code, because that's how the type='checkbox' input in the default html behaves. Also, other UI component libraries like MUI or ChakraUI also support controlling a CheckBox without that code snippet.

Lukas742 commented 4 days ago

Currently this information is not available everywhere, but the ui5-webcomponents already added it to their documentation. Since we're generating our docs from the docs of ui5-webcomponents, we'll need to update our script so it includes this information. Currently we're pretty busy with preparing our next major release, but I'll add it to our roadmap. Thank you for your feedback! :)

that's how the type='checkbox' input in the default html behaves

Btw, that's only applicable for React. ;) When using standard HTML without any framework, boolean handling works in the same way as for ui5-webcomponents: https://stackblitz.com/edit/github-ls4btp

This is something we don't catch with our wrapper because it would require too much modification of the web component (and its default behavior), and we wanted to keep it as basic as possible to keep maintenance efforts at a minimum and especially to avoid problems with updates. With React19, web component support will improve and perhaps the behavior will become more consistent then.

We'll keep this issue open until we outline if the default behavior of an event is preventable.