Fix: Refactor tag observer to custom modifier, add tests for overflow

Author: dchyun

packages/components/src/components/hds/tag/index.hbs

@@ -8,8 +8,7 @@
   @size="100"
   @weight="medium"
   @color="primary"
-  {{did-insert this.didInsert}}
-  {{will-destroy this.willDestroyNode}}
+  {{this._setUpObserver}}
   ...attributes
 >
   {{#if this.onDismiss}}

packages/components/src/components/hds/tag/index.ts

@@ -5,8 +5,8 @@
 
 import Component from '@glimmer/component';
 import { tracked } from '@glimmer/tracking';
-import { action } from '@ember/object';
 import { assert } from '@ember/debug';
+import { modifier } from 'ember-modifier';
 
 import { HdsTagColorValues } from './types.ts';
 import type { HdsTagColors } from './types.ts';
@@ -30,6 +30,22 @@ export default class HdsTag extends Component<HdsTagSignature> {
   @tracked private _isTextOverflow!: boolean;
   private _observer!: ResizeObserver;
 
+  private _setUpObserver = modifier((element: HTMLElement) => {
+    // Used to detect when text is clipped to one line, and tooltip should be added
+    this._observer = new ResizeObserver((entries) => {
+      entries.forEach((entry) => {
+        this._isTextOverflow = this._isOverflow(
+          entry.target.querySelector('.hds-tag__text-container')!
+        );
+      });
+    });
+    this._observer.observe(element);
+
+    return () => {
+      this._observer.disconnect();
+    };
+  });
+
   /**
    * @param onDismiss
    * @type {function}
@@ -110,32 +126,7 @@ export default class HdsTag extends Component<HdsTagSignature> {
     return classes.join(' ');
   }
 
-  @action
-  didInsert(element: HTMLElement): void {
-    // Used to detect when text is clipped to one line, and tooltip should be added
-    this._observer = new ResizeObserver((entries) => {
-      requestAnimationFrame(() => {
-        entries.forEach((entry) => {
-          this._isTextOverflow = this._isOverflow(
-            entry.target.querySelector('.hds-tag__text-container')!
-          );
-        });
-      });
-    });
-    this._observer.observe(element);
-  }
-
-  @action
-  willDestroyNode(): void {
-    super.willDestroy();
-    this._observer.disconnect();
-  }
-
   private _isOverflow(el: Element): boolean {
-    if (el.scrollHeight > el.clientHeight) {
-      return true;
-    } else {
-      return false;
-    }
+    return el.scrollHeight > el.clientHeight;
   }
 }

showcase/tests/integration/components/hds/tag/index-test.js

@@ -25,6 +25,7 @@ module('Integration | Component | hds/tag/index', function (hooks) {
     await render(hbs`<Hds::Tag @text="My tag" />`);
     assert.dom('button.hds-tag__dismiss').doesNotExist();
   });
+
   test('it should render the "dismiss" button if a callback function is passed to the @onDismiss argument', async function (assert) {
     this.set('NOOP', () => {});
     await render(hbs`<Hds::Tag @text="My tag" @onDismiss={{this.NOOP}} />`);
@@ -44,6 +45,7 @@ module('Integration | Component | hds/tag/index', function (hooks) {
       .dom('button.hds-tag__dismiss')
       .hasAttribute('aria-label', 'Please dismiss My tag');
   });
+
   // COLOR
 
   test('it should render the primary color as the default if no @color prop is declared when the text is a link', async function (assert) {
@@ -52,12 +54,14 @@ module('Integration | Component | hds/tag/index', function (hooks) {
     );
     assert.dom('#test-link-tag').hasClass('hds-tag--color-primary');
   });
+
   test('it should render the correct CSS color class if the @color prop is declared when the text is a link', async function (assert) {
     await render(
       hbs`<Hds::Tag @text="My text tag" @href="/" @color="secondary" id="test-link-tag"/>`
     );
     assert.dom('#test-link-tag').hasClass('hds-tag--color-secondary');
   });
+
   test('it should throw an assertion if an incorrect value for @color is provided when the text is a link', async function (assert) {
     const errorMessage =
       '@color for "Hds::Tag" must be one of the following: primary, secondary; received: foo';
@@ -70,6 +74,7 @@ module('Integration | Component | hds/tag/index', function (hooks) {
       throw new Error(errorMessage);
     });
   });
+
   test('it should throw an assertion if @color is provided without @href or @route', async function (assert) {
     const errorMessage =
       '@color can only be applied to "Hds::Tag" along with either @href or @route';
@@ -82,4 +87,22 @@ module('Integration | Component | hds/tag/index', function (hooks) {
       throw new Error(errorMessage);
     });
   });
+
+  // OVERFLOW
+
+  test('it should not render a tooltip if the text does not overflow', async function (assert) {
+    await render(hbs`
+        <Hds::Tag @text="My text tag" id="test-tag"/>
+    `);
+    assert.dom('.hds-tooltip-button').doesNotExist();
+  });
+
+  test('it should render a tooltip if the text overflows', async function (assert) {
+    await render(hbs`
+      <div style="width: 50px;">
+        <Hds::Tag @text="This is a very long text that should go on multiple lines" id="test-tag"/>
+      </div>
+    `);
+    assert.dom('.hds-tooltip-button').exists();
+  });
 });