KCheckbox: Wrap default's slot content in <label>

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 Releases are recorded as git tags in the [Github releases](https://github.com/learningequality/kolibri-design-system/releases) page.
 
+## Develop (version not yet known)
+- [#351] - Wrap `KCheckbox` default slot's content in <label>
+
+
 ## Version 1.4.x
 - [#185] - Handle arrow key navigation and improve focusOutline
 - [#338] - Allow for new 'nav' slot inline in the toolbar

docs/pages/kcheckbox.vue

@@ -3,7 +3,7 @@
   <DocsPageTemplate apiDocs>
 
     <DocsPageSection title="Overview" anchor="#overview">
-      <p>These are checkbox components:</p>
+      <p>The checkbox is generally used to select one of two possible values in a form.</p>
       <DocsShow>
         <KCheckbox
           label="Some label"
@@ -14,20 +14,25 @@
           :checked="false"
         />
       </DocsShow>
-      <p>
-        The checkbox is generally used to select one of two possible values in a form. It should not be used to make real-time changes; for this situation, use a <DocsLibraryLink component="KSwitch" /> component instead.
-      </p>
-      <p>Layout:</p>
+    </DocsPageSection>
+
+    <DocsPageSection title="Layout" anchor="#layout">
       <ul>
         <li>Aligned with container margin</li>
         <li>When used in a group, vertically stacked</li>
         <li>Hierarchical nesting is avoided</li>
       </ul>
-      <p>Labels:</p>
+    </DocsPageSection>
+
+    <DocsPageSection title="Guidelines" anchor="#guidelines">
       <ul>
         <li>Labels should be short and concise</li>
-        <li>The checked state should represent an affirmative value</li>
+        <li>Checkbox should not be used to make real-time changes; for this situation, use a <DocsLibraryLink component="KSwitch" /> component instead</li>
       </ul>
+    </DocsPageSection>
+
+    <DocsPageSection title="States" anchor="#states">
+      <p>The checked state represents an affirmative value.</p>
       <p>
         Checkboxes can also have a "partially-checked" or "indeterminate" state used in cases where the value is neither true nor false, such as when a subset of a topic is selected:
       </p>
@@ -42,6 +47,53 @@
       </p>
     </DocsPageSection>
 
+    <DocsPageSection title="Example: Label content" anchor="#example-labels">
+      Label content can be passed via the <code>label</code> property:
+
+      <!-- eslint-disable -->
+      <!-- prevent prettier from changing indentation -->
+      <DocsShowCode language="html">
+        &lt;KCheckbox label="First lesson" /&gt;
+      </DocsShowCode>
+      <!-- eslint-enable -->
+      <DocsShow>
+        <KCheckbox label="First lesson" />
+      </DocsShow>
+
+      In more complex situations, for example when another component is responsible for rendering the label, the default slot can be used:
+
+      <!-- eslint-disable -->
+      <!-- prevent prettier from changing indentation -->
+      <DocsShowCode language="html">
+        &lt;KCheckbox&gt;
+          &lt;KLabeledIcon icon="lesson" label="First lesson" /&gt;
+        &lt;/KCheckbox&gt;
+      </DocsShowCode>
+      <!-- eslint-enable -->
+      <DocsShow>
+        <KCheckbox>
+          <KLabeledIcon icon="lesson" label="First lesson" />
+        </KCheckbox>
+      </DocsShow>
+
+      <DocsDoNot>
+        <template #not>
+          <p>Don't wrap the default slot's content in <code>&lt;label&gt;</code>:</p>
+          <!-- eslint-disable -->
+          <!-- prevent prettier from changing indentation -->
+          <DocsShowCode language="html">
+            &lt;KCheckbox&gt;
+              &lt;label&gt;
+                &lt;KLabeledIcon icon="lesson" label="First lesson" /&gt;
+              &lt;/label&gt;
+            &lt;/KCheckbox&gt;
+          </DocsShowCode>
+          <!-- eslint-enable -->
+
+          <p>That would cause two nested <code>&lt;label&gt;</code> elements to be rendered as <code>KCheckbox</code> takes care of it already.</p>
+        </template>
+      </DocsDoNot>
+    </DocsPageSection>
   </DocsPageTemplate>
 
 </template>

lib/KCheckbox.vue

@@ -43,32 +43,25 @@
 
       </div>
 
-      <div
-        v-if="$slots.default"
-        class="k-checkbox-label"
-      >
-        <slot></slot>
-        <div v-if="description" class="description">
-          {{ description }}
-        </div>
-      </div>
-
-      <!-- In this case, we presume that there is a `label` prop given -->
       <label
-        v-else
         dir="auto"
         class="k-checkbox-label"
         :for="id"
         :class="{ 'visuallyhidden': !showLabel }"
         :style="labelStyle"
         @click.prevent
       >
-        {{ label }}
+        <template v-if="$slots.default">
+          <!-- @slot Optional slot as alternative to `label` prop -->
+          <slot></slot>
+        </template>
+        <template v-else>
+          {{ label }}
+        </template>
         <div v-if="description" class="description">
           {{ description }}
         </div>
       </label>
-
     </div>
   </div>
 

lib/__tests__/KCheckbox.spec.js

@@ -77,4 +77,15 @@ describe('KCheckbox component', () => {
       expect(wrapper.find('.disabled')).toBeTruthy();
     });
   });
+
+  it(`should render the default's slot content in <label>`, () => {
+    const wrapper = shallowMount(KCheckbox, {
+      slots: {
+        default: { template: '<span><span>Icon</span>Label</span>' },
+      },
+    });
+    const label = wrapper.find('label');
+    expect(label.exists()).toBeTruthy();
+    expect(label.html()).toContain('<span><span>Icon</span>Label</span>');
+  });
 });