Closed hughess closed 1 month ago
Latest commit: 993c60a448bf64488a0b7201661a5ede485ed09f
The changes in this PR will be included in the next version bump.
Not sure what this means? Click here to learn what changesets are.
Click here if you're a maintainer who wants to add another changeset to this PR
The latest updates on your projects. Learn more about Vercel for Git ↗︎
Name | Status | Preview | Comments | Updated (UTC) |
---|---|---|---|---|
docs | ✅ Ready (Inspect) | Visit Preview | 💬 Add feedback | Mar 28, 2024 2:54am |
Name | Link |
---|---|
Latest commit | 993c60a448bf64488a0b7201661a5ede485ed09f |
Latest deploy log | https://app.netlify.com/sites/evidence-development-workspace/deploys/6604dbb38661df0008db42ae |
Deploy Preview | https://deploy-preview-1761--evidence-development-workspace.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.
@ItsMeBrianD @csjh I've refactored quite a lot of this into smaller components and ripped out as much CSS as I could. There's still duplication in a few places, but I think when we USQL-ify this component we'll have a good opportunity to decide what the right long-term structure should be.
Some challenges I ran into:
columnSummary
. I'm not sure the best way to get this logic into tailwind - seemed like I would need to add more conditionals (e.g., class:align-left={column.type !== 'number'}
const finalColumnOrder
to $: finalColumnOrder
, it broke the page for me$props.columns
and columnSummary
in the script tag to make it easier to handle in the markup, but that caused a flash to happen in the table, since apparently the content in the markdown was rendered before the $props.columns
was ready for use
SubtotalRow
component, and once in the DataTable
component. The SubtotalRow
logic gets executed first, so the correct numbers appear, then the DataTable
version runs behind the scenes. Without the SubtotalRow
calcs, there is a split second flash of incorrect numbers to correct numbers. Name | Link |
---|---|
Latest commit | 4b12eef2ca28dca3052e299703820b9746e307d9 |
Latest deploy log | https://app.netlify.com/sites/evidence-docs/deploys/65fb5eb6fa95430008dfe255 |
Paused this due to some performance issues showing up when putting a large query (>100k rows) into DataTable.
Initially thought this was due to the new component structure (having a TableCell
for every td
). I think that does have a negative impact, but the bulk of the issue is caused by this block for the Fullscreen feature.
When this is included, in combo with the TableCell component, it generates a TableCell for every cell in the query (which in my test query was 1.5 million cells). When this code is removed, TableCells are only generated for the visible cells in the table (150 in my test query):
{#if !isFullPage}
<Fullscreen bind:open={fullscreen}>
<!-- header and last row are 22.5+22.5 = 45px, middle rows are 23 -->
{@const ROW_HEIGHT = 23}
{@const Y_AXIS_PADDING = 45 + 234}
<div class="pl-8 pt-4">
<svelte:self
{...$$props}
rows={1 + Math.round((innerHeight - Y_AXIS_PADDING) / ROW_HEIGHT)}
isFullPage
>
{#each $props.columns as column}
<Column {...column} />
{/each}
</svelte:self>
</div>
</Fullscreen>
{/if}
@csjh in the code snippet above, what's the purpose of the isFullPage
variable? Should that be replaced with fullscreen
?
No, isFullPage
indicates whether or not the current DataTable
is a full page DataTable
(i.e. an instance of the svelte:self
one)
It shouldn't work any different than
<DataTable rows={some_number}>
<Column {...some_column_props} />
</DataTable>
which is why I'm confused how it would suddenly generate a million TableCell
s (though some_number
is higher so some_number
would be higher than usual)
Ah got it, so maybe it should be {#if !isFullPage && fullscreen}
?
I don't think so, that would slow down the modal opening quite a bit
How were you testing how many times it rendered? A global variable?
I just put a console log in the TableCell component.
I got 1.5M logs on page load, then a pause, then 150 logs. Then when clicking full screen I got 450 logs
after commenting out that section I got 150 logs on page load
Hmm OK, it's probably the DataTable accidentally being run recursively somehow, thanks
Ahh this was funny, it was because during SSR and initial render the innerHeight
in rows={1 + Math.round((innerHeight - Y_AXIS_PADDING) / ROW_HEIGHT)}
was undefined
, which caused it to be rows={NaN}
, which caused every row to be rendered very unnecessarily
Oh wow, nice find!
Name | Link |
---|---|
Latest commit | cd4d3b68002ed5f807ff9998a4b1f5055281a4be |
Latest deploy log | https://app.netlify.com/sites/evidence-test-env/deploys/6604b6f3633f9700087ec397 |
Table Groups
Adds groups to DataTable. Groups can either be
accordion
orsection
. For now, this is limited to one level of groups, but will be expanded to allow nested groups in future versions.subtotals
are generated using the supplied groups from thegroupBy
prop and can be configured in the same way as the total row - using thetotalAgg
prop within eachColumn
Accordion
Section
Standalone Delta Component
This PR also adds a standalone Delta component which can be used in the same way the Value component is used.
Symbol Position
symbolPosition
(right
is default, seeleft
below):Chip
chip=true
formats the Delta with a background and border:Neutral Range
Inspired by this Twitter thread: https://twitter.com/archieemwood/status/1762286734845653103 Allows you to set a range for neutral values using
neutralMin
andneutralMax
. Values appear in grey with a dash instead of an arrow:Checklist