refacto: remove sort logic from pix-table

1024pix · Dec 9, 2024 · 59372f0 · 59372f0
1 parent 8e78f54
commit 59372f0
Show file tree

Hide file tree

Showing 7 changed files with 135 additions and 105 deletions.
diff --git a/addon/components/pix-table-basic-column.hbs b/addon/components/pix-table-basic-column.hbs
@@ -1,6 +1,7 @@
 <PixTableColumn
   @context={{@context}}
-  @sort={{@sort}}
+  @onSort={{@onSort}}
+  @sortOrder={{@sortOrder}}
   @ariaLabelDefaultSort={{@ariaLabelDefaultSort}}
   @ariaLabelSortAsc={{@ariaLabelSortAsc}}
   @ariaLabelSortDesc={{@ariaLabelSortDesc}}

diff --git a/addon/components/pix-table-column.hbs b/addon/components/pix-table-column.hbs
@@ -6,7 +6,7 @@
         <PixIconButton
           @ariaLabel={{this.iconLabel}}
           @iconName={{this.iconName}}
-          @triggerAction={{this.sort}}
+          @triggerAction={{@onSort}}
           @size="small"
         />
       {{/if}}

diff --git a/addon/components/pix-table-column.js b/addon/components/pix-table-column.js
@@ -1,32 +1,45 @@
 import Component from '@glimmer/component';
-import { action } from '@ember/object';
+import { warn } from '@ember/debug';
 
 export default class PixTableColumn extends Component {
-  id = crypto.randomUUID();
-
   get displayHeader() {
-    return this.args.context.type === 'header';
+    return this.args.context === 'header';
   }
 
   get sortable() {
-    return Boolean(this.args.sort);
+    return Boolean(this.args.onSort);
+  }
+
+  get sortOrder() {
+    if (this.args.sortOrder === undefined) {
+      return undefined;
+    }
+    const correctOrders = ['asc', 'desc', null];
+    warn(
+      'PixTableBasicColumn: you need to provide a valid sortOrder',
+      correctOrders.includes(this.args.sortOrder),
+      {
+        id: 'pix-ui.table-basic-column.sortOrder.not-valid',
+      },
+    );
+    return this.args.sortOrder;
   }
 
   get iconName() {
-    if (this.args.context.currentSortedColumnId !== this.id) {
+    if (!this.sortOrder) {
       return 'sort';
     }
-    if (this.args.context.sortOrder === 'asc') {
+    if (this.sortOrder === 'asc') {
       return 'sortAsc';
     }
     return 'sortDesc';
   }
 
   get iconLabel() {
-    if (this.args.context.currentSortedColumnId !== this.id) {
+    if (!this.sortOrder) {
       return this.args.ariaLabelDefaultSort;
     }
-    if (this.args.context.sortOrder === 'asc') {
+    if (this.sortOrder === 'asc') {
       return this.args.ariaLabelSortDesc;
     }
     return this.args.ariaLabelSortAsc;
@@ -36,17 +49,12 @@ export default class PixTableColumn extends Component {
     if (!this.sortable) {
       return undefined;
     }
-    if (this.args.context.currentSortedColumnId !== this.id) {
+    if (!this.sortOrder) {
       return 'none';
     }
-    if (this.args.context.sortOrder === 'asc') {
+    if (this.sortOrder === 'asc') {
       return 'ascending';
     }
     return 'descending';
   }
-
-  @action
-  sort() {
-    this.args.context.toggleSort(this.id, this.args.sort);
-  }
 }
diff --git a/addon/components/pix-table.hbs b/addon/components/pix-table.hbs
@@ -3,14 +3,13 @@
     <caption class="screen-reader-only">{{this.caption}}</caption>
     <thead class={{this.headerClass}}>
       <tr>
-        {{yield null this.headerContext to="columns"}}
+        {{yield null "header" to="columns"}}
       </tr>
     </thead>
     <tbody>
-      {{this.headerContext.date}}
-      {{#each this.computedData as |row|}}
+      {{#each @data as |row|}}
         <tr>
-          {{yield row this.cellContext to="columns"}}
+          {{yield row "cell" to="columns"}}
         </tr>
       {{/each}}
     </tbody>

diff --git a/addon/components/pix-table.js b/addon/components/pix-table.js
@@ -1,17 +1,7 @@
 import Component from '@glimmer/component';
 import { warn } from '@ember/debug';
-import { tracked } from '@glimmer/tracking';
-import { action } from '@ember/object';
 
 export default class PixTable extends Component {
-  headerContext = new HeaderContext({
-    type: 'header',
-    toggleSort: this.toggleSort,
-  });
-
-  @tracked
-  computedData = [...this.args.data];
-
   get variant() {
     const value = this.args.variant ?? 'primary';
     warn(
@@ -35,41 +25,4 @@ export default class PixTable extends Component {
   get headerClass() {
     return `pix-table-header--${this.variant}`;
   }
-
-  get cellContext() {
-    return {
-      type: 'cell',
-    };
-  }
-
-  @action
-  toggleSort(id, sortFun) {
-    this.headerContext.refresh(id);
-    if (this.headerContext.sortOrder === 'asc') {
-      this.computedData = this.computedData.sort(sortFun);
-      return;
-    }
-    this.computedData = this.computedData.reverse();
-  }
-}
-
-class HeaderContext {
-  @tracked sortOrder;
-  @tracked currentSortedColumnId;
-
-  constructor({ type, toggleSort, sortOrder, currentSortedColumnId }) {
-    this.type = type;
-    this.toggleSort = toggleSort;
-    this.sortOrder = sortOrder;
-    this.currentSortedColumnId = currentSortedColumnId;
-  }
-
-  refresh(id) {
-    if (id !== this.currentSortedColumnId) {
-      this.sortOrder = 'asc';
-      this.currentSortedColumnId = id;
-    } else {
-      this.sortOrder = this.sortOrder === 'asc' ? 'desc' : 'asc';
-    }
-  }
 }
diff --git a/addon/styles/_pix-table-basic-column.scss b/addon/styles/_pix-table-basic-column.scss
@@ -1,5 +1,5 @@
 td.pix-table-basic-column {
-  &--number{
+  &--number {
     text-align: right;
   }
 }
diff --git a/tests/integration/components/pix-table-test.js b/tests/integration/components/pix-table-test.js
@@ -126,28 +126,26 @@ module('Integration | Component | table', function (hooks) {
   });
 
   module('#sort', function () {
-    test('it should sort properly when `@sort` is a custom sort function', async function (assert) {
-      const arialLabelDefaultSort =
-        "La table n'est pas triée par nom. Cliquer pour trier par ordre alphabétique";
+    test('it should call sort function on click', async function (assert) {
+      // given
+      const sortStub = sinon.stub();
+      this.onSort = sortStub;
+
+      const arialLabelDefaultSort = 'default label sort';
       this.arialLabelDefaultSort = arialLabelDefaultSort;
-      const otherArialLabelDefaultSort = 'click sur un autre';
-      this.otherArialLabelDefaultSort = otherArialLabelDefaultSort;
-      const ariaLabelSortAsc = 'Cliquer pour trier en ordre ascendant';
-      this.ariaLabelSortAsc = ariaLabelSortAsc;
-      const ariaLabelSortDesc = 'Cliquer pour trier en ordre descendant';
-      this.ariaLabelSortDesc = ariaLabelSortDesc;
-      this.sort = (a, b) => a.name.localeCompare(b.name);
 
       // when
+
       const screen = await render(
         hbs`<PixTable @caption='Ceci est le caption de notre table' @data={{this.data}}>
   <:columns as |row context|>
     <PixTableColumn
       @context={{context}}
-      @sort={{this.sort}}
+      @onSort={{this.onSort}}
+      @sortOrder={{null}}
       @ariaLabelDefaultSort={{this.arialLabelDefaultSort}}
-      @ariaLabelSortAsc={{this.ariaLabelSortAsc}}
-      @ariaLabelSortDesc={{this.ariaLabelSortDesc}}
+      @ariaLabelSortAsc="asc label sort"
+      @ariaLabelSortDesc="desc label sort"
     >
       <:header>
         Nom
@@ -156,46 +154,117 @@ module('Integration | Component | table', function (hooks) {
         {{row.name}}
       </:cell>
     </PixTableColumn>
+  </:columns>
+</PixTable>`,
+      );
+
+      // then
+      await click(await screen.getByRole('button', { name: arialLabelDefaultSort }));
+      assert.ok(sortStub.calledOnce);
+    });
+
+    test('it should display `ariaLabelSortAsc` when sortOrder is `desc`', async function (assert) {
+      // given
+      const sortStub = sinon.stub();
+      this.onSort = sortStub;
+
+      this.sortOrder = 'desc';
+
+      const ariaLabelSortAsc = "clicker pour trié dans l'ordre desc";
+      this.ariaLabelSortAsc = ariaLabelSortAsc;
+
+      // when
+
+      const screen = await render(
+        hbs`<PixTable @caption='Ceci est le caption de notre table' @data={{this.data}}>
+  <:columns as |row context|>
     <PixTableColumn
       @context={{context}}
-      @sort={{this.sort}}
-      @ariaLabelDefaultSort={{this.otherArialLabelDefaultSort}}
+      @onSort={{this.onSort}}
+      @sortOrder={{this.sortOrder}}
+      @ariaLabelDefaultSort="default label sort"
       @ariaLabelSortAsc={{this.ariaLabelSortAsc}}
+      @ariaLabelSortDesc="desc label sort"
+    >
+      <:header>
+        Nom
+      </:header>
+      <:cell>
+        {{row.name}}
+      </:cell>
+    </PixTableColumn>
+  </:columns>
+</PixTable>`,
+      );
+
+      // then
+      assert.ok(await screen.getByRole('button', { name: ariaLabelSortAsc }));
+    });
+
+    test('it should display `ariaLabelSortDesc` when sortOrder is `asc`', async function (assert) {
+      // given
+      const sortStub = sinon.stub();
+      this.onSort = sortStub;
+
+      this.sortOrder = 'asc';
+
+      const ariaLabelSortDesc = "clicker pour trié dans l'ordre asc";
+      this.ariaLabelSortDesc = ariaLabelSortDesc;
+
+      // when
+
+      const screen = await render(
+        hbs`<PixTable @caption='Ceci est le caption de notre table' @data={{this.data}}>
+  <:columns as |row context|>
+    <PixTableColumn
+      @context={{context}}
+      @onSort={{this.onSort}}
+      @sortOrder={{this.sortOrder}}
+      @ariaLabelDefaultSort="default label sort"
       @ariaLabelSortDesc={{this.ariaLabelSortDesc}}
+      @ariaLabelSortAsc="asc label sort"
     >
       <:header>
-        Age
+        Nom
       </:header>
       <:cell>
-        {{row.age}}
+        {{row.name}}
       </:cell>
     </PixTableColumn>
   </:columns>
 </PixTable>`,
       );
 
       // then
-      const names = screen.queryAllByText(/jean|brian|zoé/);
-      assert.strictEqual(names.length, 3);
-      assert.strictEqual(names[0].textContent.trim(), 'jean');
-      assert.strictEqual(names[1].textContent.trim(), 'brian');
-      assert.strictEqual(names[2].textContent.trim(), 'zoé');
+      assert.ok(await screen.getByRole('button', { name: ariaLabelSortDesc }));
+    });
 
-      await click(await screen.getByRole('button', { name: arialLabelDefaultSort }));
-      const namesAsc = screen.queryAllByText(/jean|brian|zoé/);
-      assert.strictEqual(namesAsc[0].textContent.trim(), 'brian');
-      assert.strictEqual(namesAsc[1].textContent.trim(), 'jean');
-      assert.strictEqual(namesAsc[2].textContent.trim(), 'zoé');
+    test('it should not display sortlabel when `@onSort` is not provided', async function (assert) {
+      // given
+      const arialLabelDefaultSort = 'default label sort';
+      this.arialLabelDefaultSort = arialLabelDefaultSort;
 
-      await click(await screen.getByRole('button', { name: ariaLabelSortDesc }));
-      const namesDesc = screen.queryAllByText(/jean|brian|zoé/);
-      assert.strictEqual(namesDesc[0].textContent.trim(), 'zoé');
-      assert.strictEqual(namesDesc[1].textContent.trim(), 'jean');
-      assert.strictEqual(namesDesc[2].textContent.trim(), 'brian');
-      assert.ok(await screen.getByRole('button', { name: ariaLabelSortAsc }));
+      // when
+      const screen = await render(
+        hbs`<PixTable @caption='Ceci est le caption de notre table' @data={{this.data}}>
+  <:columns as |row context|>
+    <PixTableColumn
+      @context={{context}}
+      @ariaLabelDefaultSort={{this.arialLabelDefaultSort}}
+    >
+      <:header>
+        Nom
+      </:header>
+      <:cell>
+        {{row.name}}
+      </:cell>
+    </PixTableColumn>
+  </:columns>
+</PixTable>`,
+      );
 
-      await click(await screen.getByRole('button', { name: otherArialLabelDefaultSort }));
-      assert.ok(await screen.getByRole('button', { name: arialLabelDefaultSort }));
+      // then
+      assert.notOk(await screen.queryByRole('button', { name: arialLabelDefaultSort }));
     });
   });