ag-grid PoC #574

DTCurrie · 2024-09-05T16:45:12Z

This is a PoC to show a relatively quick-and-dirty implementation of ag-grid in Svelte. This gives us column sorting and drag-and-drop right out of the box, and one of the existing themes is already pretty close to what we probably want. I tried to make this a pretty straightforward passthrough of the ag-grid API, but the current props are large, deeply nesting objects, so there may be some performance concerns.

The following steps will be to add styles to overwrite the theme and match ours.

The large diff is due to updates in the pnpm lockfile.

Note to reviewers: I added you because the topic of using an ag-grid for tables has been raised a few times now, and I wanted to start the conversation about what it would look like. Feel free to remove yourself from the PR if this isn't relevant to you.

tahiyasalam · 2024-09-09T17:46:20Z

Super cool! This would be a great improvement.

ehhong · 2024-09-09T17:48:57Z

can we add the deploy tag to play with this grid? also, are we worried about increase in bundle size from adding this dependency?

DTCurrie · 2024-09-09T18:52:47Z

can we add the deploy tag to play with this grid? also, are we worried about increase in bundle size from adding this dependency?

Do we have a deploy tag in this repo? I didn't see a label for it.

ehhong · 2024-09-09T18:55:10Z

Do we have a deploy tag in this repo? I didn't see a label for it.

oh my bad, didn't realize this was in prime 🤦

DTCurrie · 2024-09-10T18:34:49Z

I updated the PoC to tweak some styles and show how to sort and filter. I also broke out the columns and rows into their props so they could be changed without forcing an entire re-render. You can test it out with the "Remove row" button.

CC: @micheal-parks

DTCurrie · 2024-09-10T18:48:35Z

also, are we worried about increase in bundle size from adding this dependency?

I don't think so, but I will do a build to compare. I updated the PoC to use modules provided by ag-grid which should reduce the bundle size.

DTCurrie · 2024-09-10T19:13:44Z

Added row variants. Only outstanding feature is the TableVariant which switches between table-auto and table-fixed classes, but I think we will want to ignore those anyway and instead rely on ag-grid rendering.

DTCurrie · 2024-09-10T19:49:24Z

Updated styles to match more recent table designs.

DTCurrie · 2024-09-10T19:55:26Z

Please let me know if there is a use case you would like to see me include in the example.

ehhong · 2024-09-10T20:26:50Z

packages/core/src/lib/table/ag-grid-table.svelte

+    data?.variant === 'error',
+  '!border-green-100 !bg-green-50 !text-green-700': ({ data }) =>
+    data?.variant === 'success',
+  '!bg-gray-50 !text-gray-500': ({ data }) => data?.variant === 'disabled',


[minor] can we update these to our theme colors?

I think so, these were taken straight from the table component but we are tweaking the designs anyway so we should just keep to the theme.

ehhong · 2024-09-10T20:29:29Z

packages/core/src/routes/+page.svelte

+        flex: 1,
+      },
+    }}
+  />


is there a way to add table data with custom elements? ie. in the fleet dashboard project, some of the values should be in pills

There is, but we will have to figure out how to make that work with svelte. I can make that the next thing to try in the example.

mcous

Very cool PoC! Left some initial thoughts that are primarily concerns about productionization and integrating this into the existing app architecture. So far, I still need convincing that ag-grid is a good choice for our app - I'm a little worried that, for our needs, it makes the wrong tradeoffs. That being said, my current focus is on the needs of the fleet dashboard specifically, and there my be other functionality needs elsewhere in the app that I'm not familiar with

mcous · 2024-09-11T13:40:15Z

pnpm-lock.yaml

I think this lockfile change contains unnecessary changes, it can be regenerated from main to remove unrelated bumps

git checkout main -- pnpm-lock.yaml pnpm install pnpm dedupe git add pnpm-lock.yaml

I'm showing a much smaller diff after this:

❯ git diff --stat main -- pnpm-lock.yaml pnpm-lock.yaml | 925 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------------------------------------------------------------------- 1 file changed, 579 insertions(+), 346 deletions(-)

mcous · 2024-09-11T13:44:08Z

packages/core/src/routes/+page.svelte

+  {
+    id: 1,
+    first_name: 'Bobette',
+    last_name: 'Dankov',
+    email: '[email protected]',
+    ip_address: '212.10.172.113',
+  },


I'm struggling to see how this proposed API of the component would be used by the machines dashboard table, where cells are are frequently made up of interactable elements and/or other Prime components.

I can see that ag-grid has an API for defining cell components, which we could use to live-render Svelte components, but managing that sort of compatibility layer makes me uneasy.

I think doing this work in the prime repository might be too much of a bottom-up approach for this kind of UI component. Would it be possible to take this work and drop it into app instead, so we can iterate on a PoC with the actual use case of the all machines table? This decision happened while my review was in progress

That is the row data API, I think we should look at defining custom components to be used in the columnDefs. The components would need to accept the expected data object and understand how to render (like the status badge) based on that.

There is a few different ways they manage creating cell renderers. I think the only layer we would need is something to make that accept Svelte components, which I saw a svelte 3 example of here. I think a similar wrapper that allows us to pass svelte components as a part of the object should work, that is next on my plan to experiment with.

mcous · 2024-09-11T13:58:04Z

packages/core/src/lib/table/ag-grid-table.svelte

+onMount(() => {
+  grid = createGrid(
+    eGui,
+    { ...options, columnDefs, rowData, rowClassRules },
+    params
+  );
+  return () => grid.destroy();
+});


I'm a bit concerned about taking a table approach that is entirely client-side rendered. We have "tables" that are currently capable of being rendered at build time, but switching to this would lock us out of that possibility - e.g. /registry

I have no experience with the library, but I wonder if a headless solution like tanstack table (20.3 kB (gzip)), where we define our own markup directly, would have an acceptable feature set / performance for our needs

I didn't dig too deep, but server-side rendering for rows is an enterprise feature.

All for considering tanstack table if it's a better solution since we haven't started investing in ag-grid yet

mcous · 2024-09-11T14:08:48Z

packages/core/package.json

+    "@ag-grid-community/client-side-row-model": "^32.1.0",
+    "@ag-grid-community/core": "^32.1.0",
+    "@ag-grid-community/styles": "^32.1.0",


Did a quick check on bundle.js, and it looks like these packages will add 221 kB (gzip) to our page bundles where they're used. If we go this route, I think we'll need try to make it easy to code split

DTCurrie · 2024-09-11T19:33:34Z

Closing this, @mcous is opening another PR on the app side so we can start using and iterating on this. Keep an eye out for that PR, and thanks for the input!

ag-grid PoC

a80b387

DTCurrie requested review from katiepeters, mcous, ethanlookpotts, tahiyasalam, sbal13 and micheal-parks September 5, 2024 16:45

DTCurrie self-assigned this Sep 5, 2024

simplify example

98c3e34

DTCurrie requested review from ehhong and anjinai September 5, 2024 17:38

Tweak styles and add sorting/filtering examples

c526337

DTCurrie added 2 commits September 10, 2024 14:35

cleanup

025e53f

Move to ag-grid modules

068db6c

Support table row variants

bcc8379

Update styles to match newer designs

9797aa9

ehhong reviewed Sep 10, 2024

View reviewed changes

mcous reviewed Sep 11, 2024

View reviewed changes

DTCurrie closed this Sep 11, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ag-grid PoC #574

ag-grid PoC #574

DTCurrie commented Sep 5, 2024 •

edited

Loading

tahiyasalam commented Sep 9, 2024

ehhong commented Sep 9, 2024 •

edited

Loading

DTCurrie commented Sep 9, 2024

ehhong commented Sep 9, 2024

DTCurrie commented Sep 10, 2024

DTCurrie commented Sep 10, 2024

DTCurrie commented Sep 10, 2024

DTCurrie commented Sep 10, 2024

DTCurrie commented Sep 10, 2024

ehhong Sep 10, 2024

DTCurrie Sep 11, 2024

ehhong Sep 10, 2024

DTCurrie Sep 11, 2024

mcous left a comment •

edited

Loading

mcous Sep 11, 2024 •

edited

Loading

mcous Sep 11, 2024

DTCurrie Sep 11, 2024

mcous Sep 11, 2024

DTCurrie Sep 11, 2024

micheal-parks Sep 11, 2024

mcous Sep 11, 2024

DTCurrie commented Sep 11, 2024

ag-grid PoC #574

ag-grid PoC #574

Conversation

DTCurrie commented Sep 5, 2024 • edited Loading

tahiyasalam commented Sep 9, 2024

ehhong commented Sep 9, 2024 • edited Loading

DTCurrie commented Sep 9, 2024

ehhong commented Sep 9, 2024

DTCurrie commented Sep 10, 2024

DTCurrie commented Sep 10, 2024

DTCurrie commented Sep 10, 2024

DTCurrie commented Sep 10, 2024

DTCurrie commented Sep 10, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcous left a comment • edited Loading

Choose a reason for hiding this comment

mcous Sep 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DTCurrie commented Sep 11, 2024

DTCurrie commented Sep 5, 2024 •

edited

Loading

ehhong commented Sep 9, 2024 •

edited

Loading

mcous left a comment •

edited

Loading

mcous Sep 11, 2024 •

edited

Loading