KCheckbox: Remove local state management, rely on given props

[nucleogenesis]

Description

KCheckbox had local data which are initialized to the given props for indeterminate and checked and then updated in a watcher. This resulted in that clicking a checkbox would always toggle it's isCurrentlyChecked value and setting isCurrentlyIndeterminate to false.

The changes here instead have KCheckbox simply use the given prop values and does nothing to manage any internal state around whether it shows indeterminate and/or checked.

This results in users of KCheckbox needing to be mindful of how they manage the values they pass into KCheckbox's props as the component will now reflect their values at all time.

Issue addressed

This permits Studio's clipboard to have the ability to tell KCheckbox what it should be looking like.

Changelog

• Description: Removes internal state management for checked & indeterminate in KCheckbox.
• Products impact: Updated API
• Addresses: Clipboard indeterminate selection incorrect studio#4636
• Components: KCheckbox
• Breaking: No
• Impacts a11y: No
• Guidance: If you use KCheckbox, it is your responsibility to handle the change event and update whether or not the given checked and indeterminate props reflect the reality that you expect.

Steps to test

Click all of the checkboxes and the states of indeterminacy vs checked-ness -- particularly in nested situations.

The most common in Kolibri is content selection. In Studio, this issue was found while debugging a problem with the Clipboard.

KOLIBRI:

• Resource selection during import
• During Quiz creation
• During Lesson creation

STUDIO:

• Clipboard
• Channel editor

(optional) Implementation notes

At a high level, how did you implement this?

Does this introduce any tech-debt items?

Testing checklist

• Contributor has fully tested the PR manually
• If there are any front-end changes, before/after screenshots are included
• Critical and brittle code paths are covered by unit tests
• The change is described in the changelog section above

Reviewer guidance

• Is the code clean and well-commented?
• Are there tests for this change?
• Are all UI components LTR and RTL compliant (if applicable)?
• Add other things to check for here

After review

• The changelog item has been pasted to the CHANGELOG.md

Comments

[AlexVelezLl]

Code changes make sense to me. But I wonder if removing the local state management is a breaking change.

I would suggest to make a different PR to remove the internal state targeting to develop and explore there its implications. And leave this PR with the removal of the this.isCurrentlyIndeterminate = false; line when we are toggling the check, which I suspect would be the needed change to let the consumer have total control in the combination of the four possible combinations of checked/unchecked/indeterminate/non-indeterminate states (as we have a watcher for the checked and indeterminate states.

For example, this change breaks the docs page as it relies on the KCheckbox internal state:

Compartir.pantalla.-.2024-08-26.17_00_12.mp4

And there could be some other places where we could be relying on the KCheckbox internal state too (also because the checked prop isnt even marked as required).

[MisRob]

Leaving a note here that before merging it'd be good to revert CHANGELOG.md changes since the new GH action will now update the CHANGELOG.md after merging this PR by pasting the changelog from the PR description. Otherwise we'd have duplicate entries.

[AlexVelezLl]

lgtm!

[AlexVelezLl]

Thank you @nucleogenesis!