Feature/sc-48047/table-sdk-sticky-columns

[etienneburdet]

Summary

The goal for this PR is to add sticky columns to the table. I tried to support a "pin" column scenario, where users can add/remove multiple columns to sticky side.

(Internal for Opendatasoft only) Associated Shortcut ticket: sc-48047.

Changes

Sticky style
I chose position: sticky with computed offsets.
Other possibilities: back to full div, second non scrolling table.
Pros: a11y because it's a single table.
Cons: shadow is a bit harder to do, requires js to compute offsets.

Sticky state API
I chose to declare the sticky in the column declaration.
Other possibilities: declaring a stickyColumns: … in the config, exposing an headless state manager (column.pin()) à la/with Tanstack table.
Pros: can only pin defined colmun no id mismatch.
Cons: can be a it of a burden to update since it's an object, start mixing definition and state management.

Sticky state internal
I went with initilazing a Map and reseting both the map and scroll on each change of columns (even non-sticky) and sorting columns based on sticky and their initial order.
Other possibilities: using array/indexes, pushing in order of selection, not messing with order (sticky only when "met"), using column.sticky in palce.
Pros: no need for index matching in 3 different places, everything derived from base store, seperation between user config and internal state.
Cons: May reset on colmuns update that didn't require it.

I made a specific Th component first because it was getting really big, but mostly to avoid an array of clientWidth since array reactivity in Svelte can be annoying.

The shadow is made with a linear gradient, because box-shadow cannot be applied to only one side and thus required some keyed z-index manipulations for each column to hide the non-wanted parts.

Breaking changes

Since I needed border: separate, we end-up with a 1px diffs an all table stories, one way or another. Here I removed the extra 1px td padding, the other way around added it. I thought it wasn't worth the 0.5px padding just to minimize the diff.

Changelog

"It's now possible to add sticky columns to the table, that remains visible while scrolling".

Review checklist

• Description is complete
• Commits respect the Conventional Commits Specification
• 2 reviewers (1 if trivial)
• Tests coverage has improved
• Code is ready for a release on NPM

[KevinFabre-ods]

That's a nice feature GG! 👏

The offset computation implementation is not so big and is reactive to column widths !(column definition doesn't need a size)

Sticky style
I chose position: sticky with computed offsets.
Other possibilities: back to full div, second non scrolling table.
Pros: a11y because it's a single table.
Cons: shadow is a bit harder to do, requires js to compute offsets.

I think it's looking nice! The offset computation is quite lite to be fair.

Sticky state API
I chose to declare the sticky in the column declaration.
Other possibilities: declaring a stickyColumns: … in the config, exposing an headless state manager (column.pin()) à la/with Tanstack table.
Pros: can only pin defined colmun no id mismatch.
Cons: can be a it of a burden to update since it's an object, start mixing definition and state management.

It's a good start to stay consistence with our other declarations (sort system for example).
You're right to point out that it can be heavy to declare if we have UIs where sticky columns could be change frequently by end users.

Sticky state internal
I went with initilazing a Map and reseting both the map and scroll on each change of columns (even non-sticky) and sorting columns based on sticky and their initial order.
Other possibilities: using array/indexes, pushing in order of selection, not messing with order (sticky only when "met"), using column.sticky in palce.
Pros: no need for index matching in 3 different places, everything derived from base store, seperation between user config and internal state.
Cons: May reset on colmuns update that didn't require it.

For a start, IMO it's acceptable that the scroll resets when columns option change. There is room for optimization

Other than that, i left some comment to fix minors issues

[KevinFabre-ods]

We might want to not declare this variable when stickyColumnsOffset[key] is undefined

[KevinFabre-ods]

We can set a generic type <key, number> for the Map objects

[KevinFabre-ods]

Suggested change

	<>
	<React.Fragment key={col.key}>

Just because we have an error in the console

[KevinFabre-ods]

We lost the ability to apply a background color on tbody, tr

Maybe we could move this style to the tr tag?

[etienneburdet]

It has to be on the td because we will have overlapping tds with no background elese. I changed the customization stories, it seems acceptable.

[KevinFabre-ods]

Do you think we could put L14-L24 into the Th component ?

[KevinFabre-ods]

These three classes are shared between Th and Cell
What do you think to create a hook to build these sticky classes ?

[KevinFabre-ods]

Suggested change

	$: if (columns && scrollBox) {
	scrollBox.scrollLeft = 0;
	sortedStickyColumns = [...columns].sort((colA, colB) => {
	if (Boolean(colA?.sticky) === Boolean(colB?.sticky)) {
	return 0;
	}
	return colA?.sticky ? -1 : 1;
	});
	stickyColumnsWidth.reset();
	sortedStickyColumns
	.filter((col) => col?.sticky)
	.forEach((col) => {
	stickyColumnsWidth.updateColumn(col.key, 0);
	});
	}
	$: if (columns && scrollBox) {
	scrollBox.scrollLeft = 0;
	stickyColumnsWidth.reset();
	columns.forEach((col) => {
	col?.sticky && stickyColumnsWidth.updateColumn(col.key, 0);
	});
	}

Did I miss something?
Do we want to get update the store state with only columns keys that are sticky?
(While keeping the original order of the column)

[etienneburdet]

I opted to sort the sticky column first so that they are already in their sticky position. In the version you shared colmun will stick "as they come". I thought it was pretty common for "pinned" columns to be first. This could also be done by putting sticky columns in a different array (in which case we don't need to sort).

[KevinFabre-ods]

[...columns].sort((colA, colB) => {
            if (Boolean(colA?.sticky) === Boolean(colB?.sticky)) {
                return 0;
            }
            return colA?.sticky ? -1 : 1;
        });

or

columns.filter((col) => col?.sticky)

seems to produce the same result.
I don't understand why we need this sort function