From 57326507b886f3694bd43710e0b16078b4c467ff Mon Sep 17 00:00:00 2001
From: Jeff Daley <jeffdaley@users.noreply.github.com>
Date: Thu, 18 Jan 2024 10:19:45 -0500
Subject: [PATCH] Improve checkable item markup

---
 .../x/dropdown-list/checkable-item.hbs        |  6 ++--
 .../x/dropdown-list/checkable-item.ts         | 10 +++++-
 .../components/x/dropdown/list-item.scss      | 32 +++++++++++++------
 .../x/dropdown-list/checkable-item-test.ts    | 25 ++++++++++-----
 4 files changed, 52 insertions(+), 21 deletions(-)

diff --git a/web/app/components/x/dropdown-list/checkable-item.hbs b/web/app/components/x/dropdown-list/checkable-item.hbs
index eba082296..3185c6ada 100644
--- a/web/app/components/x/dropdown-list/checkable-item.hbs
+++ b/web/app/components/x/dropdown-list/checkable-item.hbs
@@ -1,6 +1,8 @@
 <div
   data-test-checkable-item
-  class="checkable-item checkmark-position--{{or @checkmarkPosition 'leading'}}
+  class="checkable-item
+    {{if this.countIsShown 'has-count'}}
+    checkmark-position--{{or @checkmarkPosition 'leading'}}
     w-full items-center gap-2.5"
   ...attributes
 >
@@ -25,7 +27,7 @@
     {{/if}}
   </div>
 
-  {{#if @count}}
+  {{#if this.countIsShown}}
     <div class="grid">
       <Hds::BadgeCount
         data-test-x-dropdown-list-checkable-item-count
diff --git a/web/app/components/x/dropdown-list/checkable-item.ts b/web/app/components/x/dropdown-list/checkable-item.ts
index e7cdde26e..2809134ec 100644
--- a/web/app/components/x/dropdown-list/checkable-item.ts
+++ b/web/app/components/x/dropdown-list/checkable-item.ts
@@ -20,7 +20,15 @@ interface XDropdownListCheckableItemComponentSignature {
   };
 }
 
-export default class XDropdownListCheckableItemComponent extends Component<XDropdownListCheckableItemComponentSignature> {}
+export default class XDropdownListCheckableItemComponent extends Component<XDropdownListCheckableItemComponentSignature> {
+  /**
+   * Whether the item has a count to display.
+   * True if the count is defined, including if it's 0.
+   */
+  protected get countIsShown(): boolean {
+    return this.args.count !== undefined;
+  }
+}
 
 declare module "@glint/environment-ember-loose/registry" {
   export default interface Registry {
diff --git a/web/app/styles/components/x/dropdown/list-item.scss b/web/app/styles/components/x/dropdown/list-item.scss
index 704ff1344..e1bf69f3e 100644
--- a/web/app/styles/components/x/dropdown/list-item.scss
+++ b/web/app/styles/components/x/dropdown/list-item.scss
@@ -34,21 +34,33 @@
 
 .checkable-item {
   @apply grid;
-  grid-template-columns: 16px auto 60px;
+  grid-template-columns: 16px auto;
+  grid-template-areas: "check label";
 
   &.checkmark-position--trailing {
-    grid-template-columns: auto 60px 16px;
+    grid-template-columns: auto 16px;
+    grid-template-areas: "label check";
+  }
 
-    .checkable-item-content {
-      @apply order-1;
-    }
+  &.has-count {
+    grid-template-columns: 16px auto 60px;
+    grid-template-areas: "check label count";
 
-    .checkable-item-count {
-      @apply order-2;
+    &.checkmark-position--trailing {
+      grid-template-areas: "label count check";
+      grid-template-columns: auto 60px 16px;
     }
+  }
 
-    .check {
-      @apply order-3;
-    }
+  .checkable-item-content {
+    grid-area: label;
+  }
+
+  .checkable-item-count {
+    grid-area: count;
+  }
+
+  .check {
+    grid-area: check;
   }
 }
diff --git a/web/tests/integration/components/x/dropdown-list/checkable-item-test.ts b/web/tests/integration/components/x/dropdown-list/checkable-item-test.ts
index 439e26552..45a98f1e9 100644
--- a/web/tests/integration/components/x/dropdown-list/checkable-item-test.ts
+++ b/web/tests/integration/components/x/dropdown-list/checkable-item-test.ts
@@ -5,7 +5,6 @@ import { hbs } from "ember-cli-htmlbars";
 import { CheckmarkPosition } from "hermes/components/x/dropdown-list/checkable-item";
 
 const ITEM = "[data-test-checkable-item]";
-const CONTENT = "[data-test-checkable-item-content]";
 const CHECK = "[data-test-x-dropdown-list-checkable-item-check]";
 const COUNT = "[data-test-x-dropdown-list-checkable-item-count]";
 
@@ -30,6 +29,11 @@ module("Integration | Component | x/dropdown-list", function (hooks) {
       />
     `);
 
+    assert
+      .dom(ITEM)
+      .doesNotHaveClass("has-count")
+      .hasStyle({ "grid-template-areas": '"check label"' });
+
     assert.dom(CHECK).hasClass("invisible");
     assert.dom("[data-test-x-dropdown-list-item-value]").hasText("foo");
     assert.dom(COUNT).doesNotExist();
@@ -38,8 +42,14 @@ module("Integration | Component | x/dropdown-list", function (hooks) {
 
     assert.dom(CHECK).hasClass("visible");
 
-    this.set("count", 1);
-    assert.dom(COUNT).hasText("1");
+    this.set("count", 0);
+
+    assert
+      .dom(ITEM)
+      .hasClass("has-count")
+      .hasStyle({ "grid-template-areas": '"check label count"' });
+
+    assert.dom(COUNT).hasText("0");
   });
 
   test("it can render the checkmark in a leading or trailing position", async function (this: XDropdownListCheckableItemTestContext, assert) {
@@ -49,7 +59,7 @@ module("Integration | Component | x/dropdown-list", function (hooks) {
       <X::DropdownList::CheckableItem
         @checkmarkPosition={{this.checkmarkPosition}}
         @isSelected={{true}}
-        @count={{1}}
+        @count={{0}}
         @value="foo"
       />
     `);
@@ -59,7 +69,8 @@ module("Integration | Component | x/dropdown-list", function (hooks) {
       .hasClass(
         "checkmark-position--leading",
         "default checkmark position is leading",
-      );
+      )
+      .hasStyle({ "grid-template-areas": '"check label count"' });
 
     this.set("checkmarkPosition", "trailing");
 
@@ -70,8 +81,6 @@ module("Integration | Component | x/dropdown-list", function (hooks) {
         "can render checkmark in trailing position",
       );
 
-    assert.dom(CONTENT).hasStyle({ order: "1" });
-    assert.dom(COUNT).hasStyle({ order: "2" });
-    assert.dom(CHECK).hasStyle({ order: "3" });
+    assert.dom(ITEM).hasStyle({ "grid-template-areas": '"label count check"' });
   });
 });