feat(Table): Implement sortable columns

[Mintoo200]

Implements sortable columns as described in the DSFR documentation for the Table component.
The technical documentation for this feature is quite sparse, but an example of its implementation is nonetheless available (if a bit hidden in the "Miscellaneous Table Story").

[Mintoo200]

I believe this impacts #268 but does not close it

[Mintoo200]

We decided for this implementation to always render buttons for the sort action even though sorting is often handled with a URL change and full page (or at least table) reload (which usually requires anchors). This has been done because the sorting action is not a navigation to a new resource per se, but rather a change in the way the same data is displayed.
This design decision is not clearly documented in the storybook because it does not pertain to the usage of the table itself but rather the way it is implemented.
I anticipate you might have questions about this decision in the future so, if you think it pertinent, we can add a small paragraph about it in the description of the story.

[kevbarns]

@Mintoo200 Thanks for this. The three-state cycle and the controlled/uncontrolled split are the right shape, and the story documenting the sparse-array behaviour is welcome.

I checked out 50ea333 and ran the CI steps locally. Note that GitHub Actions never ran on this PR: the run sits at action_required (fork PR awaiting maintainer approval), so nothing below was caught upstream.

Blocking

yarn install fails. @gouvfr/dsfr@1.15.x added a preinstall license gate that exits 1 unless a .dsfr.yml consent file sits at the project root or DSFR_ACCEPT_LICENSE=1 is set. Reproduced on a bare project with only @gouvfr/dsfr@^1.15.2 as a dependency: [NO_YML] Le DSFR n'est pas installé dans ce projet. CI uses bahmutov/npm-install, so test_format and test both die before running.

yarn test fails. With 1.15.2 installed, dsfrComponentsCascadeOrder.test.ts fails: the upstream sourcemap now exposes a display section that DSFR_COMPONENTS_CASCADE_ORDER does not list (43 vs 44 entries). src/bin/only-include-used-components.ts needs display added in the right cascade position, plus the new fr-table--sm / --lg / --multiline handling.

Controlled mode ignores the sort prop. cycleSortingOrder closes over useSort's own state; sort is never passed to useSort. Simulating the hook's state machine with sort={{column:1,order:"ascending"}} and no defaultSort, four clicks emit ascending, descending, none, ascending where the DSFR cycle expects descending, none, ascending, descending — the first click never advances.

Sort button has no accessible name and no type="button". Detail inline.

To discuss

The DSFR bump does not belong in this PR. It changes ColorVariant, adds three new table variants and needs the license-gate handling above. Splitting it out would let the sorting feature land on its own. Also, ^1.15.2 is a range while the repo pins @gouvfr/dsfr exactly — patches/@gouvfr+dsfr+1.15.2.patch is keyed on the exact version, so a 1.15.3 release would silently drift.

Half-applied 1.15 structure. DSFR 1.15 wants fr-table > fr-table__wrapper > fr-table__container > fr-table__content (a <div>) > <table>. This PR puts fr-table__content as a class on the <table> itself. The descendant selectors happen to still match, but it is not the documented structure.

Positional sortableColumns. A sparse (boolean | undefined)[] desynchronises silently from headers when a column is inserted. A per-header shape would be sturdier.

Optional

Raw class strings ("fr-table__content", `fr-btn--sort`) bypass fr.cx and lose the FrClassName check the rest of the file relies on. import SortingOrder = TableProps.SortingOrder is unusual for this codebase; a plain type alias reads closer to the surrounding code.

Heads up: #340 rewrites the same component (size, header column, cell alignment) and #268 tracks the whole update. Worth agreeing on a single track.

Nothing to flag on security.

[kevbarns]

Three divergences from the DSFR reference markup (example/component/table/index.html), which emits <button type="button" class="fr-btn--sort fr-btn fr-btn--sm">Trier</button>:

1. No type. A <button> inside a <form> defaults to type="submit", so clicking a sort control submits the form.
2. No fr-btn fr-btn--sm. .fr-btn carries display: inline-flex and align-items: center; without it the ::before arrow icon falls back to default button rendering.
3. fr-btn--sort and fr-btn--sort-asc are emitted together. The reference uses one or the other. .fr-btn--sort::before (arrow-up-down) and .fr-btn--sort-asc::before (arrow-up) have equal specificity, so the right icon only wins by source order in dsfr.css.

Suggested change

	<button
	className={cx(
	`fr-btn--sort`,
	order === "ascending" && "fr-btn--sort-asc",
	order === "descending" && "fr-btn--sort-desc"
	)}
	onClick={() => {
	onSort();
	}}
	/>
	<button
	type="button"
	className={cx(
	fr.cx("fr-btn", "fr-btn--sm"),
	order === "ascending"
	? "fr-btn--sort-asc"
	: order === "descending"
	? "fr-btn--sort-desc"
	: "fr-btn--sort"
	)}
	onClick={onSort}
	/>

What the suggestion cannot carry: the button still has no accessible name. The reference puts visible text inside it (clipped by max-width: 2rem; overflow: hidden), which is what screen readers and voice control announce. Table has no i18n yet — Pagination shows the createComponentI18nApi pattern to follow for a translatable "Trier".

[Mintoo200]

Applied in #a45d5280 and #cb036c42. I did not use the ternary as I find simple chained && easier to read, but I can change that if you prefer the ternary form

[Mintoo200]

Hey @kevbarns , thanks for the quick review.

The DSFR bump does not belong in this PR.

I openned a new PR (#517) with only this bump. To note, this PR (sortable columns) depends on that bump, as 1.14 has a bug where several adjacent sortable columns would display on top of each other instead of next to each other.

@gouvfr/dsfr@1.15.x added a preinstall license gate that exits 1 unless a .dsfr.yml consent file sits at the project root or DSFR_ACCEPT_LICENSE=1 is set.

In this new PR, I added DSFR_ACCEPT_LICENSE=1 to any yarn install in the CI. I believe that for local development, the developer should still have to accept the license explicitly so I added it to the .gitignore, but if you think that committing .dsfr.yml is the way to go, I'll let you commit it since I do not have the authority on this repository.

yarn test fails. With 1.15.2 installed, dsfrComponentsCascadeOrder.test.ts fails: the upstream sourcemap now exposes a display section that DSFR_COMPONENTS_CASCADE_ORDER does not list (43 vs 44 entries).

Added to #517

plus the new fr-table--sm / --lg / --multiline handling.

Do you want me to add that to the dependency update PR or to a separate "feature" PR ?

Controlled mode ignores the sort prop. cycleSortingOrder closes over useSort's own state; sort is never passed to useSort.

✅ Fixed in #12c90eb9

Sort button has no accessible name and no type="button".

✅ Fixed in #a45d5280
Kinda ashamed I missed that 😬

To discuss

The DSFR bump does not belong in this PR. It changes ColorVariant, adds three new table variants and needs the license-gate handling above. Splitting it out would let the sorting feature land on its own. Also, ^1.15.2 is a range while the repo pins @gouvfr/dsfr exactly — patches/@gouvfr+dsfr+1.15.2.patch is keyed on the exact version, so a 1.15.3 release would silently drift.

The update has been moved to #517 with all related issues (except the new variants as discussed above). As stated above, this PR needs at least v1.14.3 which fixes a bug on the sort button. I rebased this pull request on #517 for it to work, but it will need to cascade back on main once #517 is merged and it will still display changes from #517 until then.

Half-applied 1.15 structure. DSFR 1.15 wants fr-table > fr-table__wrapper > fr-table__container > fr-table__content (a <div>) > <table>. This PR puts fr-table__content as a class on the <table> itself. The descendant selectors happen to still match, but it is not the documented structure.

✅ applied in #fa18b6bc

Positional sortableColumns. A sparse (boolean | undefined)[] desynchronises silently from headers when a column is inserted. A per-header shape would be sturdier.

#85358518 changes the way sortable columns are declared. headers now accepts (as well as a regular ReactNode as before) an object as { label: ReactNode, sortable: boolean }

Optional

Raw class strings ("fr-table__content", `fr-btn--sort`) bypass fr.cx and lose the FrClassName check the rest of the file relies on.

✅ fixed in #fa18b6bc

import SortingOrder = TableProps.SortingOrder is unusual for this codebase; a plain type alias reads closer to the surrounding code.

✅ fixed in #85358518

Heads up: #340 rewrites the same component (size, header column, cell alignment) and #268 tracks the whole update. Worth agreeing on a single track.

The changes added by #340 do add some elements provided in this pull request, such as the structure of the component, but not the sortable columns feature. We are currently developing a table-heavy set of applications and we expect to open several update on the table component in the coming months. Since #340 has not had any update in a couple of years, and given the quantity and impact of its changes, could we consider merging these requests as they come and update that pull request to remove features as they are integrated ?

[kevbarns]

Second pass, @Mintoo200. Verified on cb036c42 with yarn install, yarn build, tsc -p src, tsc -p src/bin, yarn test, yarn format:check, plus renderToStaticMarkup and npx react-dsfr optimize-css on throwaway projects.

Settled, checked against the branch code and not the commit names:

• Controlled mode. useSort(defaultSort, sort) now resolves sort ?? currentSortState inside the hook. Simulated: four clicks emit descending, none, ascending, descending, the DSFR cycle.
• Sort button. type="button", {t("sort")}, fr-btn fr-btn--sm, and one sort class at a time.
• Wrapper structure. Rendered markup matches the 1.15.2 reference: fr-table > fr-table__wrapper > fr-table__container > fr-table__content > <table>.
• fr.cx everywhere, import X = TableProps.X gone, headers accepts { label, sortable }.
• @gouvfr/dsfr pinned exactly again.

Blocking

yarn test still fails, on a different test now. dsfrComponentDetectionClassPrefixes rejects "display": ["fr-display"]: "fr-display never opens a selector in display.css". It is right, the one rule in that file is .fr-display .fr-fieldset__element:last-child, where the class is only an ancestor. Use "display": [], the record is total so the key has to stay.

display is in the cascade order but no module maps to it, so the Display CSS is now dropped. src/Display/Display.tsx:45 renders className={"fr-display"}, and Header mounts <Display /> unless disableDisplay is set. Ran optimize-css on a project importing only Display: 5 components of 46, and .fr-display .fr-fieldset__element appears 0 times in the output. Same with Header. Adding "display" to the Display and Header entries brings it to 6 and 10 components with the rule present, and the whole suite green.

To discuss

1.14.3 may be all this PR needs, which would unblock it from #517. The only sort change between 1.14.2 and 1.14.3 in table.css is the button selector widening from .fr-cell--sort .fr-btn--sort to .fr-cell--sort [class^=fr-btn--sort]. That matters for your new exclusive classes, since fr-btn--sort-asc alone no longer matches the old selector, but the effect is a missing margin-left: 1rem on a sorted column, not columns stacking. The stacking you saw is better explained by the old markup having no fr-table__content, which .fr-cell--sort { display: flex } requires and which this PR now emits.

1.14.3 already ships fr-table__wrapper, __container, __content, fr-cell--sort and fr-cell__title, has no preinstall license gate (added in 1.15.0) and no component/display section. Bumping to 1.14.3 instead would drop the CI env var, the .gitignore entry, the cascade order change and both blockers above. The patch target line is still present in 1.14.3, so only the patch filename needs regenerating.

ColorVariant is not part of the bump. I got this wrong in my first pass, sorry: fr-table--sm, --lg and --multiline already exist in 1.14.2, the version on main. be70225b fixes a bug that is live today, independent of any version change, and could land on main on its own.

fr-table--sm / --lg / --multiline props: a separate feature PR. They are new API surface, and per the above they need no bump at all.

Optional

.gitignore is normalised to CRLF end to end, so a 3 line change reads as 9 added and 6 removed.

The reference wraps the header label in <span class="fr-cell__title">, the PR renders it bare. No visual difference, its only rule is font-weight: 700 and <th> is already bold.

On #340: merging as features come and trimming #340 in step seems right to me, but that call is @garronej's and @revolunet's, not mine. I asked for the arbitration there.

Nothing on security.

[Mintoo200]

Blocking

yarn test still fails, on a different test now. dsfrComponentDetectionClassPrefixes rejects "display": ["fr-display"]: "fr-display never opens a selector in display.css". It is right, the one rule in that file is .fr-display .fr-fieldset__element:last-child, where the class is only an ancestor. Use "display": [], the record is total so the key has to stay.

✅ fixed in #517 and rebased

display is in the cascade order but no module maps to it, so the Display CSS is now dropped. src/Display/Display.tsx:45 renders className={"fr-display"}, and Header mounts <Display /> unless disableDisplay is set. Ran optimize-css on a project importing only Display: 5 components of 46, and .fr-display .fr-fieldset__element appears 0 times in the output. Same with Header. Adding "display" to the Display and Header entries brings it to 6 and 10 components with the rule present, and the whole suite green.

✅ fixed (?) in #517 and rebased

To discuss

1.14.3 may be all this PR needs, which would unblock it from #517. The only sort change between 1.14.2 and 1.14.3 in table.css is the button selector widening from .fr-cell--sort .fr-btn--sort to .fr-cell--sort [class^=fr-btn--sort]. That matters for your new exclusive classes, since fr-btn--sort-asc alone no longer matches the old selector, but the effect is a missing margin-left: 1rem on a sorted column, not columns stacking. The stacking you saw is better explained by the old markup having no fr-table__content, which .fr-cell--sort { display: flex } requires and which this PR now emits.

1.14.3 already ships fr-table__wrapper, __container, __content, fr-cell--sort and fr-cell__title, has no preinstall license gate (added in 1.15.0) and no component/display section. Bumping to 1.14.3 instead would drop the CI env var, the .gitignore entry, the cascade order change and both blockers above. The patch target line is still present in 1.14.3, so only the patch filename needs regenerating.

I rebased onto main and updated @gouvfr/dsfr to 1.14.3 in this pull request. #518 does not seem to conflict with this pull request so I left them independent from each other.

ColorVariant is not part of the bump. I got this wrong in my first pass, sorry: fr-table--sm, --lg and --multiline already exist in 1.14.2, the version on main. be70225b fixes a bug that is live today, independent of any version change, and could land on main on its own.

✅ Moved to #518

fr-table--sm / --lg / --multiline props: a separate feature PR. They are new API surface, and per the above they need no bump at all.

👌

Optional

.gitignore is normalised to CRLF end to end, so a 3 line change reads as 9 added and 6 removed.

cf #517

The reference wraps the header label in <span class="fr-cell__title">, the PR renders it bare. No visual difference, its only rule is font-weight: 700 and <th> is already bold.

This is not documented explicitly in the code documentation for the Table component. Should I add them anyway ?

On #340: merging as features come and trimming #340 in step seems right to me, but that call is @garronej's and @revolunet's, not mine. I asked for the arbitration there.

👌