headlamp
headlamp copied to clipboard
frontend: Add new Table component and use it in ResourceTable
We want more features in our tables. Stuff like sorting, filtering, selection, actions, etc. This is where the new Table
component comes in and SimpleTable
will remain simple.
material-react-table (MRT) was chosen because it uses the same ui framework, looks good, has all the feature we will need, MIT licensed, based on headless tanstack table and very customizable.
Table
component is mostly a wrapper around MRT with some sensible defaults and additional app specific behaviour (like storing page state in url). I kept most of the props as aliases to the MRT props so it can be extensible without introducing any plumbing.
In the scope of this PR I also updated ResourceTable component to use the new table. The only change from the 'outside' is that each column needs to provide getter and renderer functions, first is needed for filtering and sorting, second one is for displaying. Getter needs to return a string and since most of the existing column getters were simple functions there wasn't many changes there. In those cases where existing getter returned JSX node it was renamed to render and a simple getter was added, representing plain value.
While not in the scope of this PR there are features that we can implement now using the new Table: row selection, actions column, multi-select for the filtering, selection actions (like deleting all selected Pods).
Fixes #1646 #1641 and unblocks #1006 #1640
I'm still checking if I didn't break anything and ported the existing functionality properly but if you want you can leave any comments or suggestions
This is how it looks like
Column filtering is enabled by default for all columns, can be disabled
Sorting by multiple column works out of the box
Drop down with values in a filter can be enabled by setting filterVariant: 'multi-select'
It's looking good. I left a few comments. The ResourceTable is something we export to plugins, so we need to make sure it's backwards compatible (including the exported
useThrottle
hook that you moved). I know this is complicated to do with so many fields, so if there's another suggestion (new component name that fits right), I'm open to that. Otherwise, we do try to keep commits atomic, so if there are changes that should be in other isolated commits, let's move them there.
I've reverted some name changes to keep ResourceTable backwards compatible. I missed that it was a common component. The only partly incompatible thing is the getter
change in the column definition. It is going to work without an issue, even if the JSX is returned. But it will display a type error during the build/development of the plugin in cases where getter
returns JSX. This will help plugin authors update it to take advantage of the filtering/sorting functionality.
But it will display a type error during the build/development of the plugin in cases where
getter
returns JSX. This will help plugin authors update it to take advantage of the filtering/sorting functionality.
This will be annoying to developers who need to rebuild but cannot yet update that part of the table (maybe they can be time-pressed).
So I would recommend either doing a type composition where the version with the JSX getter is deprecated. Or (probably easier and less confusing), just deprecating that property altogether and add a new getValue
for getting the value instead.
This will be annoying to developers who need to rebuild but cannot yet update that part of the table (maybe they can be time-pressed). So I would recommend either doing a type composition where the version with the JSX getter is deprecated. Or (probably easier and less confusing), just deprecating that property altogether and add a new
getValue
for getting the value instead.
Good point, I went with the latter option. Catching places where JSX getter is used is unreliable, even in our codebase jsx is sometimes typed as any, so I went with introducing a new property getValue.
I have noticed a few issues with the table replacement: Column chooser considers a column that has just a context menu:
Fixed, columns without a label will not be shown in the column picker.
Long cell values are not ellipsizing or wrapping, just expanding the table:
Fixed, wrapping now behaves like it used to
The age column has the column controls on the left (probably because we align this columns name to the right but what we really need is for the column to occupy the minimum width it needs)
Age column is now narrow, I've kept the right alignment, I think it looks better that way
The filters/search in the table shouldn't conflict with the ones in the section header: If not super complicated I think we should for now keep the section filter but make the search there be used for the table search (which is much better than what we have). The namespace filter should just show the section namespace filtering (we need to study how to move the namespace to a more global way).
Implemented the changes we discussed. The section header will now only have namespace switcher that is displayed without a toggle, search is now handled only inside the table.
translations' *_old.json files are not to be committed.
done
Age column:
- Even though it's nice for the values to be aligned to the right, the column shows the controls to the left of it:
now only body cell values aligned to the right, column itself now has min-content width
Nodes:
- Some columns look incorrectly spaces:
implemented grid layout for the table, it now behaves the same way it used to
Namespace Header filter:
- Looks unaligned with the table, or maybe it's the table that is missing some padding...
I didn't really touch paddings there, it was like this:
There are some negative margins and paddings that can be cleaned up, I could look into that but in a different pr probably
Column chooser popover:
- Has a "Hide All" option. Not sure if there's a quick way to remove this option, but the option doesn't make a lot of sense IMO.
I didn't find a quick way to customize it, but I can see a usecase for it. For example if you want only 2 columns out of 10, you hide all and then click only 2 times to show the ones you need
Could you please let me know what testing you have done?
Just manual, looked through all of the tables side by side with current headlamp version. Checked if all the example plugins are building without an error
One thing I just remembered is that this needs to be overridden by plugins (look for registerResourceTableColumnsProcessor
in example plugins).
If you implement this @sniok , @yolossn has an interesting corner case he's facing with the table column processor + sorting.
@sniok I have pushed this style change just for tracking it. Today I will bring this to the design team and check whether we keep this design change or do something else. Let's wait for that before we merge the commit + update snaptshots.
I have found the following problems when looking further: Node details:
- Hidden cell contents that have hover info icons
Maybe if we enable the column resizing feature, it will prevent these issues?
- Hidden cell contents that have hover info icons
I noticed now that this table may not be the new one.
namespace filter was reverted. went through each commit and regenerated snapshots
Can you please rebase with main when you have a chance?
The gh interface is pretty bad with these big PRs. It's super slow, and I don't get notified when a convo is resolved. To help with this, can we please leave convos open until they are resolved by note opener. This way we don't have to search through 90 of them whilst my CPU fans are on 100%.
Made Table exported as default and rebased with main branch