Make use of the new @required decorator (#685)

* Make use of the new required decorator

* Apply suggestions from code review

Co-authored-by: clintcs <[email protected]>

* Remove unnecessary this.selectedOptions.length > 0 checks

* Update CONTRIBUTING.md

Co-authored-by: Dan Wenzel <[email protected]>

---------

Co-authored-by: clintcs <[email protected]>
Co-authored-by: Dan Wenzel <[email protected]>

Author: 3 people

.changeset/funny-windows-appear.md

@@ -0,0 +1,5 @@
+---
+'@crowdstrike/glide-core': patch
+---
+
+The `label` of a Dropdown Option with a checkbox is now displayed in a tooltip when the label is truncated.

.changeset/spicy-pumpkins-obey.md

@@ -0,0 +1,40 @@
+---
+'@crowdstrike/glide-core': minor
+---
+
+Required attributes now throw during development when missing. The following are now required:
+
+- `label`
+  - Accordion
+  - Button Group Button
+  - Button Group
+  - Button
+  - Checkbox Group
+  - Checkbox
+  - Drawer
+  - Dropdown Option
+  - Dropdown
+  - Icon Button
+  - Input
+  - Menu Button
+  - Menu Link
+  - Modal Icon Button
+  - Modal
+  - Radio Group Radio
+  - Radio Group
+  - Split Button Primary Button
+  - Split Button Primary Link
+  - Split Button Secondary Button
+  - Tag
+  - Textarea
+  - Toggle
+  - Tooltip
+  - Tree Item Icon Button
+  - Tree Item Menu
+  - Tree Item
+- `name`
+  - Tab Panel
+- `panel`
+  - Tab
+- `url`
+  - Split Button Primary Link

CONTRIBUTING.md

@@ -21,6 +21,7 @@
     - [Prefer using animations only when the user has no reduced motion preference](#prefer-using-animations-only-when-the-user-has-no-reduced-motion-preference)
   - [Prefer `rem`s](#prefer-rems)
   - [Throw when slotted content is missing or the wrong type](#throw-when-slotted-content-is-missing-or-the-wrong-type)
+  - [Throw when required properties are missing](#throw-when-required-properties-are-missing)
   - [Prefer conventions set by built-in elements](#prefer-conventions-set-by-built-in-elements)
   - [Prefer separate test files](#prefer-separate-test-files)
   - [Typing property decorators](#typing-property-decorators)
@@ -402,6 +403,22 @@ export default class GlideCoreExample extends LitElement {
 }
 ```
 
+### Throw when required properties are missing
+
+Some properties are required for accessibility or ensure proper functionality.
+When a property is required, use the `@required` decorator to assert its existence.
+
+```ts
+import required from './library/required.js';
+
+@customElement('glide-core-example')
+export default class GlideCoreExample extends LitElement {
+  @property()
+  @required
+  name: string;
+}
+```
+
 ### Prefer conventions set by built-in elements
 
 A built-in element is one that is provided by the platform, such as `<input />`.

src/accordion.test.basics.ts

@@ -21,6 +21,18 @@ it('is accessible', async () => {
   await expect(host).to.be.accessible();
 });
 
+it('throws when `label` is empty', async () => {
+  const spy = sinon.spy();
+
+  try {
+    await fixture(html`<glide-core-accordion>Content</glide-core-accordion>`);
+  } catch {
+    spy();
+  }
+
+  expect(spy.callCount).to.equal(1);
+});
+
 it('throws when subclassed', async () => {
   const spy = sinon.spy();
 

src/accordion.ts

@@ -8,6 +8,7 @@ import styles from './accordion.styles.js';
 import assertSlot from './library/assert-slot.js';
 import shadowRootMode from './library/shadow-root-mode.js';
 import final from './library/final.js';
+import required from './library/required.js';
 
 declare global {
   interface HTMLElementTagNameMap {
@@ -33,7 +34,9 @@ export default class GlideCoreAccordion extends LitElement {
 
   static override styles = styles;
 
-  @property({ reflect: true }) label?: string;
+  @property({ reflect: true })
+  @required
+  label?: string;
 
   @property({ reflect: true, type: Boolean })
   get open() {

src/button-group.button.test.basics.ts

@@ -98,6 +98,20 @@ it('is not tabbable when not selected', async () => {
   expect(radio?.tabIndex).to.equal(-1);
 });
 
+it('throws when `label` is empty', async () => {
+  const spy = sinon.spy();
+
+  try {
+    await fixture(
+      html`<glide-core-button-group-button></glide-core-button-group-button>`,
+    );
+  } catch {
+    spy();
+  }
+
+  expect(spy.callCount).to.equal(1);
+});
+
 it('throws when subclassed', async () => {
   const spy = sinon.spy();
 
@@ -114,6 +128,7 @@ it('throws when `icon-only` and no "icon" slot', async () => {
   await expectUnhandledRejection(() => {
     return fixture(
       html`<glide-core-button-group-button
+        label="Label"
         privateVariant="icon-only"
       ></glide-core-button-group-button>`,
     );

src/button-group.button.ts

@@ -7,6 +7,7 @@ import styles from './button-group.button.styles.js';
 import assertSlot from './library/assert-slot.js';
 import shadowRootMode from './library/shadow-root-mode.js';
 import final from './library/final.js';
+import required from './library/required.js';
 
 declare global {
   interface HTMLElementTagNameMap {
@@ -29,7 +30,8 @@ export default class GlideCoreButtonGroupButton extends LitElement {
   static override styles = styles;
 
   @property({ reflect: true })
-  label? = '';
+  @required
+  label?: string;
 
   @property({ type: Boolean, reflect: true })
   get selected() {

src/button-group.test.basics.ts

@@ -39,7 +39,7 @@ it('is accessible', async () => {
 
 it('sets the orientation of its buttons when horizontal', async () => {
   const host = await fixture(
-    html`<glide-core-button-group>
+    html`<glide-core-button-group label="Label">
       <glide-core-button-group-button
         label="One"
       ></glide-core-button-group-button>
@@ -58,7 +58,7 @@ it('sets the orientation of its buttons when horizontal', async () => {
 
 it('sets the orientation of its buttons when vertical', async () => {
   const host = await fixture(
-    html`<glide-core-button-group orientation="vertical">
+    html`<glide-core-button-group label="Label" orientation="vertical">
       <glide-core-button-group-button
         label="One"
       ></glide-core-button-group-button>
@@ -96,7 +96,7 @@ it('sets `privateVariant` on its buttons', async () => {
 
 it('selects the first button not disabled', async () => {
   const host = await fixture(
-    html`<glide-core-button-group>
+    html`<glide-core-button-group label="Label">
       <glide-core-button-group-button
         label="One"
         disabled
@@ -121,7 +121,7 @@ it('selects the first button not disabled', async () => {
 
 it('selects no buttons when all are disabled', async () => {
   const host = await fixture(
-    html`<glide-core-button-group>
+    html`<glide-core-button-group label="Label">
       <glide-core-button-group-button
         label="One"
         disabled
@@ -140,6 +140,24 @@ it('selects no buttons when all are disabled', async () => {
   expect(buttons[1].selected).to.be.false;
 });
 
+it('throws when `label` is empty', async () => {
+  const spy = sinon.spy();
+
+  try {
+    await fixture(
+      html`<glide-core-button-group>
+        <glide-core-button-group-button
+          label="One"
+        ></glide-core-button-group-button>
+      </glide-core-button-group>`,
+    );
+  } catch {
+    spy();
+  }
+
+  expect(spy.callCount).to.equal(1);
+});
+
 it('throws when subclassed', async () => {
   const spy = sinon.spy();
 

src/button-group.test.events.ts

@@ -7,7 +7,7 @@ import './button-group.js';
 
 it('dispatches a "selected" event when a button is clicked and not already selected', async () => {
   const host = await fixture(
-    html`<glide-core-button-group>
+    html`<glide-core-button-group label="Label">
       <glide-core-button-group-button
         label="One"
       ></glide-core-button-group-button>
@@ -34,7 +34,7 @@ it('dispatches a "selected" event when a button is clicked and not already selec
 
 it('does not dispatch a "selected" event when an already selected button is clicked', async () => {
   const host = await fixture(
-    html`<glide-core-button-group>
+    html`<glide-core-button-group label="Label">
       <glide-core-button-group-button
         label="One"
         selected
@@ -56,7 +56,7 @@ it('does not dispatch a "selected" event when an already selected button is clic
 
 it('dispatches "selected" events when arrowing', async () => {
   const host = await fixture(
-    html`<glide-core-button-group>
+    html`<glide-core-button-group label="Label">
       <glide-core-button-group-button
         label="One"
         selected
@@ -104,7 +104,7 @@ it('dispatches "selected" events when arrowing', async () => {
 
 it('dispatches a "selected" event when a button is selected via Space', async () => {
   const host = await fixture(
-    html`<glide-core-button-group>
+    html`<glide-core-button-group label="Label">
       <glide-core-button-group-button
         label="One"
         selected
@@ -131,7 +131,7 @@ it('dispatches a "selected" event when a button is selected via Space', async ()
 
 it('does not dispatch a "selected" event when a button is selected programmatically', async () => {
   const host = await fixture(
-    html`<glide-core-button-group>
+    html`<glide-core-button-group label="Label">
       <glide-core-button-group-button
         label="One"
         selected
@@ -160,7 +160,7 @@ it('does not dispatch a "selected" event when a button is selected programmatica
 
 it('does not dispatch a "selected" event when an already selected button is selected via Space', async () => {
   const host = await fixture(
-    html`<glide-core-button-group>
+    html`<glide-core-button-group label="Label">
       <glide-core-button-group-button
         label="One"
         selected
@@ -183,7 +183,7 @@ it('does not dispatch a "selected" event when an already selected button is sele
 
 it('does not dispatch a "selected" event a button is selected programmatically', async () => {
   const host = await fixture(
-    html`<glide-core-button-group>
+    html`<glide-core-button-group label="Label">
       <glide-core-button-group-button
         label="One"
         selected

src/button-group.test.focus.ts

@@ -5,7 +5,7 @@ import './button-group.js';
 
 it('moves focus when arrowing', async () => {
   const host = await fixture(
-    html`<glide-core-button-group>
+    html`<glide-core-button-group label="Label">
       <glide-core-button-group-button
         label="One"
       ></glide-core-button-group-button>