web: refactor the table renderer for legibility

[kensternberg-authentik]

Details

Refactor the Table component for legiibility.

This commit does not change the functionality of the Table, nor does it require any changes to existing uses of the Table.

It will probably be easier to review this by looking at the View Code in the upper-right-hand corner of GitHub’s reviewer; that or side-by-side, if your monitor is wide-enough.

Background

The existing Table component is used 49 times (at last count) in authentik, and those uses are wide-ranging and complex, but they all come down to a couple of entries:

• Displaying a row of summary information
• Permitting the display of more complex (“expanded”) information
• Displaying a collection of rows
• Displaying a collection of rows grouped by some header
• Pagination of many rows
• Permitting an action on the visible rows
• Not blocking events that may happen on a cell or expansion
• Providing a toolbar
• Providing a display of “selected items” when using the table as a multi-select with many pages of items (chips display)
• Providing sort functionality on columns
• Providing the ability to filter the table from the back-end

This commit changes none of that.

Actions taken

What this commit does is re-arrange the innards of Table.ts into smaller units:

• The RowGroup’s “checkbox” and “expansion” segments are pulled out into their own functions, which makes the RowGroup’s actual functionality much easier to see and understand. The same is true of the rowGroup’s selection and expansion handlers.
• Almost all in-line decisions and event handlers have been extracted and named, to make it easier to see and understand what’s happening inside what is otherwise a jumble of HTML.
• The TablePagination code was duplicated– and one of the duplicates was wrong! So I’ve deduplicated it and fixed the bug.
• In many cases, the conditional code grew organically, resulting in some pretty hard-to-understand conditions.
  • A really good example is the itemSelectHandler; there are two possible events that result in a change, and the consequences of that change may be that all checkboxes are unchecked. In all cases where there’s an add/remove option, I’ve opted to remove the specific object always (even if it’s not present!), and then add it if it’s actually an add. Logically coherent as long as the accessors are not also mutators.

Conflicts discovered

It was not possible to redefine the columns() function to take anything other than a TableColumn object; I wanted to be able to replace all of the new TableColumn("Foo") with just "Foo", building the TableColumn dynamically at construction time. Unfortunately, some of our most complex tables dynamically re-arrange the columns (RBAC, for example, draws an empty table, fetches the content, then redraws with the columns based on what was retrieved), and detecting that change and rebuilding those columns proved more difficult than anticipated. I may contemplate an alternative column specification if I find myself building a lot of tables.

Likewise, it was not possible to replace all of our uses of the empty html declaration with the Lit-preferred nothing sigil; hard-coded TemplateResult entries scattered throughout the code caused massive type inconsistencies, since a type of TemplateResult | nothing is unique thanks to nothing’s underlying Symbol. It is for this issue that Typescript itself recommends you “prefer allowing Typescript infer the return type.” I may revisit this issue later.

Misc

I’ve added a prequick command to package.json; this one runs only the Typescript type checker, lit-analyse, and eslint:precommit, the last of which lints only the files touched since the last commit. This is fast, intended to support quick checks of code quality not normally displayed in the IDE.

Checklist

• Local tests pass (ak test authentik/)
• The code has been formatted (make lint-fix)

If an API change has been made

• The API schema has been updated (make gen-build)

If changes to the frontend have been made

• The code has been formatted (make web)
• The translation files have been updated (make i18n-extract)

If applicable

• The documentation has been updated
• The documentation has been formatted (make website)

[netlify]

✅ Deploy Preview for authentik-storybook ready!

Name	Link
🔨 Latest commit	f8566c0
🔍 Latest deploy log	https://app.netlify.com/sites/authentik-storybook/deploys/657738ae58f38a0008dbfa7c
😎 Deploy Preview	https://deploy-preview-7433--authentik-storybook.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site configuration.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (1fccbaa) 43.71% compared to head (f8566c0) 92.62%.

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #7433       +/-   ##
===========================================
+ Coverage   43.71%   92.62%   +48.91%     
===========================================
  Files         588      588               
  Lines       29200    29200               
===========================================
+ Hits        12764    27047    +14283     
+ Misses      16436     2153    -14283     

Flag	Coverage Δ
e2e	50.65% <ø> (+6.93%)	⬆️
integration	25.91% <ø> (?)
unit	89.70% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[BeryJu]

couple of questions

[BeryJu]

I think I left the variable name as item here as that's the default when inheriting from the class

[kensternberg-authentik]

Good point; I've put it back the way it was.

I was trying to use the "unused variable in abstract code" idiom here, and it shouldn't matter what the name is in the child code, only the position. But leaving the name as-is and commenting out the check is probably better for readability in the long-run.

[netlify]

✅ Deploy Preview for authentik ready!

Name	Link
🔨 Latest commit	f8566c0
🔍 Latest deploy log	https://app.netlify.com/sites/authentik/deploys/657738aebcfbc80008c2d308
😎 Deploy Preview	https://deploy-preview-7433--authentik.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
Lighthouse	1 paths audited Performance: 96 (🔴 down 1 from production) Accessibility: 90 (no change from production) Best Practices: 100 (no change from production) SEO: 80 (no change from production) PWA: - View the detailed breakdown and full score reports

---

To edit notification comments on pull requests, go to your Netlify site configuration.