Table fix

[ciaranschutte]

• custom TD and TH
• sorting
• sticky cell
  related uikit PR Custom table elemets uikit#141

[justincorrigible]

what's FO in this context? may be useful to abstract this into a declaratively named constant, to facilitate reading.

[ciaranschutte]

There's logic like this all over this file.

[justincorrigible]

While I know it's a contentious point in the community, often labelled as "nit picking", I truly believe alphabetising properties does help preventing duplicates... which in CSS may introduce bugs that can be rather hard to find at times, because of how the order matters (keyword "cascading").

In this tiny list it may seem superfluous, but we should do it everywhere to elicit consistency, right?
broken windows, etc.

[justincorrigible]

cool! hadn't seen this property in the react-table docs. or is it a uikit thing perhaps?

[ciaranschutte]

more a uikit thing. otherwise the cell and header render functions get wrapped in specific components like <TableCell> and <TableHeaderWrapper>. This becomes problematic when you can't override styling they provide. For example padding on parent. You can hack around it but I opened a PR with that way of doing things and we decided as team to go the bigger more correct fix.

[justincorrigible]

in contrast, as follow up to my earlier "alphabetise all the things" comment, TS does a much better job at identifying and pointing out duplicate React props, so I don't care as much here.

...I'd only advocate for it here for consistency and "coding discipline", and I realise other devs don't feel as strongly about that as I do.

[justincorrigible]

val -> value, no need to skimp on letters; unless value is already in scope, in which case aValue vs bValue would be better than variants of the same single word.

[justincorrigible]

not a request for change, because I know this is an established pattern and not something just from this PR

just a thought I had, on how we could do these non-dev business-domain things better for new hires who won't have the background or context (and me, inexcusably).
🤔 this dictionary right here is what I wish we do with VS' intellisense, that if you hover over a constant (like the one I suggested here), it also gives you a description of what it means, apart from its string value.
wonder if there's a plugin to do that sort of thing.

[justincorrigible]

update:
ahh JSDoc to the rescue! no plugin needed 👍

[ciaranschutte]

nice!

[joneubank]

I assume this one is thick border cause its the last element? Should the logic be based on it being the last element not a specific element?

[ciaranschutte]

It's somewhat unclear in regards to the groupings in the table.
Last element requires us to target the last element of the row.

We can't access the row element.
So we'd have to do some logic with the indices from the internal react table data which we shouldn't be touching at this level. That is way more flaky than just hard coding a value.

DX wise val === "FO is very clear. It works and is very much in line with the code quality in the rest of this file

[joneubank]

Sounds fine, it is clear whats happening. Just to complete my though there though, do we not have access the index form the .map? partial code example:

const coreCompletionColumnsCount = 7;
columns: columns.slice(0, coreCompletionColumnsCount).map((column, index) => ({
	...column,
	sortingFn: sortEntityData,
	header: (props) => {
		const isLastElement = index === coreCompletionColumnsCount -1;