evidence-dev / evidence

Business intelligence as code: build fast, interactive data visualizations in pure SQL and markdown
https://evidence.dev
MIT License
3.44k stars 167 forks source link

Improved DataTable #544

Closed hughess closed 1 year ago

hughess commented 1 year ago

Description

This PR is the first step in implementing improved tables in Evidence (see #468). It covers the following improvements to our DataTable component:

Examples

Sample Table

datatable-all

Interactive Demonstration

datatable-interactive

Development Demonstration

datatable-development

Checklist

Issues with this PR

changeset-bot[bot] commented 1 year ago

🦋 Changeset detected

Latest commit: d1176cff8b86ad7ff723b89cb75d14970141edd8

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages | Name | Type | | ------------------------- | ----- | | @evidence-dev/preprocess | Minor | | @evidence-dev/components | Minor | | @evidence-dev/evidence | Major | | evidence-test-environment | Patch |

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

vercel[bot] commented 1 year ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
evidence-development-workspace ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Jan 6, 2023 at 9:39PM (UTC)
evidence-docs ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Jan 6, 2023 at 9:39PM (UTC)
archiewood commented 1 year ago

This is a huge improvement on what we have.

Some thoughts for your consideration:

hughess commented 1 year ago

Thanks @archiewood

I'm going to take a look at the mobile search bar again, and I like your approach to abstracting the sort icons into a component - I'll add that into the new table component.

I'm not able to reproduce the header truncation issue - it should automatically add horizontal scroll even for headers only. Which browser/page width are you using?

I agree on pagination, but I think it's probably a longer-term fix because of scrolling considerations for long tables within an article and vertical scroll on mobile. I think in the future we might want to allow multiple options - e.g., pagination, vertical scroll, scrubber, etc.

archiewood commented 1 year ago

Hmmm I can't repro the header issue either... Maybe I had changed something else in my branch

Re pagination I was just hoping I might be able to click on where it says page 1 and type in 500, which would then jump me to page 500 or similar, vs clicking 500 times

image

Also, now I notice it, we might want to style 10000 as 10,000 in the pages

hughess commented 1 year ago

Ahh, good catch - I hadn't considered such high page numbers.

I've fixed the number formatting, but I'll need to think about the page editing idea.

If the pagination section didn't have the editable page number, would your preference be to use pagination or the data scrubber like we have in the query viewer? And does your answer change for either of these situations:

archiewood commented 1 year ago

Probably depends on the size of the dataset more than these different situations. For small datasets either seems okay. For larger datasets:

I think the scrubber is probably better overall. Though I think would probably prefer the scrubber to sit vertically up the right hand size rather than the bottom

mcrascal commented 1 year ago

@hughess @archiewood this is really sick.

Pagination styling: alignment of elements needs a bit of work here.

Print formatting: print formatting can be a lot better (e.g., pagination buttons remain in print version) A stopgap solution here could be to display a nicer looking “X of Y records” message rather than the pagination buttons

I took a stab at the alignment and printing issues identified in the original PR, and adjusted the colouring. I'll open a PR into this branch w/ the proposed changes for your consideration. I've included some screenshots and discussion below.

In addition to the already identified issues in the PR, I would add two more. Neither of these are blocking in my opinion.

Pagination Styling

Table hovered state

Before

CleanShot 2023-01-04 at 21 15 06@2x

After

CleanShot 2023-01-04 at 21 13 52@2x

Un-hovered state

CleanShot 2023-01-04 at 21 16 17@2x

Printing

CleanShot 2023-01-04 at 21 19 35@2x
hughess commented 1 year ago

@mcrascal looks really nice - I like it!

hughess commented 1 year ago

Just played around with the hovering - looks much cleaner. Liking the fade in on the download button too

hughess commented 1 year ago

I played around with the idea Archie had to allow an input for the page number (and using the hover feature Adam added).

@archiewood @mcrascal what do you guys think of this?

The input width is tricky to set - would need to try it with larger page numbers and see what our options are.

CleanShot 2023-01-05 at 15 53 27

mcrascal commented 1 year ago

I dig it. Seems discoverable without being intrusive when you're just reading.

One other idea for a future release: I think it would be nice to advance the pages continuously when you hold down the next/previous page button. I often use the scrubber to visually scan data that way, and I find it demos really well for teams who are moving off of a slower tool.

archiewood commented 1 year ago

Yeah I really like that.

Could have a tiny bit more spacing after the "/" before the "total pages" number to even it out? Not a biggie though.

hughess commented 1 year ago

Ok I made some modifications and got it working more consistently. I've set it up to show 3 digits, which I hope will cover most scenarios.

This isn't great for very large page numbers, but I think we'll need a better solution for those anyways, so this can be a good option until we have that.

CleanShot 2023-01-06 at 16 03 22