Skip to content

Commit

Permalink
KCheckbox: Remove local state management, rely on given props
Browse files Browse the repository at this point in the history
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.

changelog
  • Loading branch information
nucleogenesis committed Aug 26, 2024
1 parent bf04095 commit 300ad9c
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 22 deletions.
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,16 @@ Changelog is rather internal in nature. See release notes for the public overvie

## Version 4.x.x (`release-v4` branch)

- [#744]
- **Description:** Removes internal state management for checked & indeterminate in KCheckbox.
- **Products impact:** Updated API
- **Addresses:** https://github.com/learningequality/studio/issues/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.

[#744]: [PR link](https://github.com/learningequality/kolibri-design-system/pull/744)

- [#737]
- **Description:** Bump KDS version to 4.4.1.
Expand Down
28 changes: 6 additions & 22 deletions lib/KCheckbox.vue
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@
type="checkbox"
class="k-checkbox-input"
:aria-checked="ariaChecked"
:checked="isCurrentlyChecked"
:indeterminate.prop="isCurrentlyIndeterminate"
:checked="checked"
:indeterminate.prop="indeterminate"
:disabled="disabled"
@click.stop="toggleCheck"
@focus="isActive = true"
Expand All @@ -24,13 +24,13 @@
>

<KIcon
v-if="isCurrentlyIndeterminate"
v-if="indeterminate"
:style="notBlank"
class="checkbox-icon"
icon="indeterminateCheck"
/>
<KIcon
v-else-if="!isCurrentlyIndeterminate && isCurrentlyChecked"
v-else-if="!indeterminate && checked"
:style="[ notBlank, activeOutline ]"
class="checkbox-icon"
icon="checked"
Expand Down Expand Up @@ -122,13 +122,11 @@
},
},
data: () => ({
isCurrentlyChecked: false,
isCurrentlyIndeterminate: false,
isActive: false,
}),
computed: {
ariaChecked() {
return this.isCurrentlyIndeterminate ? 'mixed' : this.isCurrentlyChecked ? 'true' : 'false';
return this.indeterminate ? 'mixed' : this.checked ? 'true' : 'false';
},
id() {
return `k-checkbox-${this._uid}`;
Expand All @@ -152,28 +150,14 @@
};
},
},
watch: {
checked(newCheckedState) {
this.isCurrentlyChecked = newCheckedState;
},
indeterminate(newIndeterminateState) {
this.isCurrentlyIndeterminate = newIndeterminateState;
},
},
created() {
this.isCurrentlyChecked = this.checked;
this.isCurrentlyIndeterminate = this.indeterminate;
},
methods: {
toggleCheck(event) {
if (!this.disabled) {
this.isCurrentlyChecked = !this.isCurrentlyChecked;
this.$refs.kCheckboxInput.focus();
this.isCurrentlyIndeterminate = false;
/**
* Emits change event
*/
this.$emit('change', this.isCurrentlyChecked, event);
this.$emit('change', !this.checked, event);
}
},
markInactive() {
Expand Down

0 comments on commit 300ad9c

Please sign in to comment.