From 59372f0e900e54adc42391ef24699c1975e8042a Mon Sep 17 00:00:00 2001 From: Fael BASSETTI Date: Fri, 6 Dec 2024 13:39:42 +0100 Subject: [PATCH] refacto: remove sort logic from pix-table --- addon/components/pix-table-basic-column.hbs | 3 +- addon/components/pix-table-column.hbs | 2 +- addon/components/pix-table-column.js | 40 +++-- addon/components/pix-table.hbs | 7 +- addon/components/pix-table.js | 47 ------ addon/styles/_pix-table-basic-column.scss | 2 +- .../integration/components/pix-table-test.js | 139 +++++++++++++----- 7 files changed, 135 insertions(+), 105 deletions(-) diff --git a/addon/components/pix-table-basic-column.hbs b/addon/components/pix-table-basic-column.hbs index f0042c533..497cfa926 100644 --- a/addon/components/pix-table-basic-column.hbs +++ b/addon/components/pix-table-basic-column.hbs @@ -1,6 +1,7 @@ {{/if}} diff --git a/addon/components/pix-table-column.js b/addon/components/pix-table-column.js index e6911fa89..62ad9b0c0 100644 --- 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 index 2d9b83fbd..938e5dc12 100644 --- a/addon/components/pix-table.hbs +++ b/addon/components/pix-table.hbs @@ -3,14 +3,13 @@ {{this.caption}} - {{yield null this.headerContext to="columns"}} + {{yield null "header" to="columns"}} - {{this.headerContext.date}} - {{#each this.computedData as |row|}} + {{#each @data as |row|}} - {{yield row this.cellContext to="columns"}} + {{yield row "cell" to="columns"}} {{/each}} diff --git a/addon/components/pix-table.js b/addon/components/pix-table.js index c5a0bbc02..1ba350b89 100644 --- 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 index e72359777..2cb1aaebb 100644 --- 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 index 773dd1037..a000840c9 100644 --- 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` <:columns as |row context|> <:header> Nom @@ -156,18 +154,81 @@ module('Integration | Component | table', function (hooks) { {{row.name}} + +`, + ); + + // 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` + <:columns as |row context|> + <:header> + Nom + + <:cell> + {{row.name}} + + + +`, + ); + + // 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` + <:columns as |row context|> + <:header> - Age + Nom <:cell> - {{row.age}} + {{row.name}} @@ -175,27 +236,35 @@ module('Integration | Component | table', function (hooks) { ); // 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` + <:columns as |row context|> + + <:header> + Nom + + <:cell> + {{row.name}} + + + +`, + ); - 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 })); }); });