Sync checkboxes after defaulting to all (#909)

Closes getodk/central#543.
getodk · Dec 11, 2023 · ecc02c9 · ecc02c9
1 parent baa59d4
commit ecc02c9
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 29 deletions.
diff --git a/src/components/multiselect.vue b/src/components/multiselect.vue
@@ -219,28 +219,30 @@ const changeCheckbox = ({ target }) => {
   change(props.options[target.dataset.index].value);
 };
 
-// emittedValue holds the last value that has been emitted since
-// props.modelValue was last set. It equals `null` if no value has been emitted
-// since then, or if no value has ever been emitted. We use emittedValue for an
-// optimization in order to avoid an extra sync.
+// emittedValue is used for an optimization, to avoid an unnecessary sync.
+// Unless it is `null`, it holds the last value emitted since props.modelValue
+// was last set. It equals `null` if no value has been emitted since then (or if
+// no value has ever been emitted).
 let emittedValue = null;
 const emitIfChanged = () => {
   if (changes.size === 0) return;
   changes.clear();
 
-  // If props.defaultToAll is `true`, and no options are selected, then the
-  // selection falls back to all options. If props.modelValue is already all
-  // options, then we just toggle needsSync and return.
-  const needsDefault = props.defaultToAll && selected.size === 0;
-  if (needsDefault && props.modelValue.length === props.options.length) {
-    needsSync = true;
-    return;
+  if (props.defaultToAll && selected.size === 0) {
+    if (props.modelValue.length === props.options.length) {
+      // If props.modelValue is already all options, then there hasn't been a
+      // change. We don't need to emit a new value, but we do need to sync the
+      // checkboxes.
+      needsSync = true;
+    } else {
+      // Setting emittedValue to `null` so that checkboxes are synced.
+      emittedValue = null;
+      emit('update:modelValue', props.options.map(({ value }) => value));
+    }
+  } else {
+    emittedValue = [...selected];
+    emit('update:modelValue', emittedValue);
   }
-
-  emittedValue = needsDefault
-    ? props.options.map(({ value }) => value)
-    : [...selected];
-  emit('update:modelValue', emittedValue);
 };
 
 watch(

diff --git a/test/components/multiselect.spec.js b/test/components/multiselect.spec.js
@@ -243,22 +243,41 @@ describe('Multiselect', () => {
   });
 
   describe('defaultToAll prop is true', () => {
-    it('emits all option values if no option was left checked', async () => {
-      const component = mountComponent({
-        props: {
-          options: [{ value: 0 }, { value: 1 }],
-          modelValue: [0],
-          defaultToAll: true
-        },
-        attachTo: document.body
+    describe('some (not all) options were checked, then were unchecked', () => {
+      it('emits all option values', async () => {
+        const component = mountComponent({
+          props: {
+            options: [{ value: 0 }, { value: 1 }],
+            modelValue: [0],
+            defaultToAll: true
+          },
+          attachTo: document.body
+        });
+        await toggle(component);
+        await component.get('input[type="checkbox"]').setValue(false);
+        await toggle(component);
+        component.emitted('update:modelValue').should.eql([[[0, 1]]]);
+      });
+
+      it('checks all checkboxes again', async () => {
+        const component = mountComponent({
+          props: {
+            options: [{ value: 0 }, { value: 1 }],
+            modelValue: [0],
+            defaultToAll: true
+          },
+          attachTo: document.body
+        });
+        await toggle(component);
+        await component.get('input[type="checkbox"]').setValue(false);
+        await toggle(component);
+        await component.setProps({ modelValue: [0, 1] });
+        await toggle(component);
+        assertChecked(component, [true, true]);
       });
-      await toggle(component);
-      await component.get('input[type="checkbox"]').setValue(false);
-      await toggle(component);
-      component.emitted('update:modelValue').should.eql([[[0, 1]]]);
     });
 
-    describe('all options were checked, then were all unchecked', () => {
+    describe('all options were checked, then were unchecked', () => {
       it('does not emit an event', async () => {
         const component = mountComponent({
           props: {