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

Add TailwindCSS #702

Closed ItsMeBrianD closed 1 year ago

ItsMeBrianD commented 1 year ago

Description

This builds on the work started in #623, adding TailwindCSS and styling the BigValue and ErrorChart components.

Resolves #611

Additions from the previous PR:

Checklist

Noteworthy Items for Review:

Before

image

After

image

changeset-bot[bot] commented 1 year ago

🦋 Changeset detected

Latest commit: f9007071dc61a75b45d18b42b88265cd07a33c3d

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/components | Minor | | @evidence-dev/preprocess | Patch | | @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 Mar 28, 2023 at 7:12PM (UTC)
evidence-docs ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Mar 28, 2023 at 7:12PM (UTC)
mcrascal commented 1 year ago

@ItsMeBrianD I am not fussed about the awkward whitespace on the error state for now.

It looks like the sparkline here isn't rendering quite right. [Edit, nevermind it looks like this is actually regression in main -- bug here: #703 ]

Winterhart commented 1 year ago

Can we merge on another branch then main until the stabilization work is done? 🤔

ItsMeBrianD commented 1 year ago

Can we merge on another branch then main until the stabilization work is done? 🤔

If we don't have a dev branch or similar, we can create a new one or I can continue to work off this one until the stabilization. Maybe a next branch would make sense?

mcrascal commented 1 year ago

Fooling around with this locally.

Alerts:

Nits:

Sticky mode:

I can get two behaviours with the sticky prop:

  1. sticky = {true} and the alert will follow the scroll past it's position on the page
  2. {sticky} in reference to the bool on the example page, and I get a full-width
CleanShot 2023-03-22 at 18 02 02@2x

Tabs:

These are sweet -- I'd go sans serif on the labels, and switch to the blue we use for active pages in the sidebar as the "active" colour over the green

hughess commented 1 year ago

These are great!

One thing worth considering is the syntax for true/false props - currently, the rest of the component library accepts those props as strings and converts them to boolean (idea being that it keeps the prop syntax cleaner w/ less curly braces)

ItsMeBrianD commented 1 year ago

When it's in "sticky mode", and is scrolling with the header, I would remove the rounding on the corners.

I poked around with this; and this seems to be somewhat complex to implement; in theory it is possible with IntersectionObserver, but it was hard to make that play nice with the header involved. I can continue to try to get this implemented, but I wasn't sure if it was more effort than it was worth

mcrascal commented 1 year ago

Looking great to me. I'm OK to skip the "unrounding". Not sure what's going on with the MacOS 16 test, but once passing I think you're good to go.

ItsMeBrianD commented 1 year ago

I'm not entirely sure where to start with the MacOS / 16 test, it looks like an error getting credentials for the BigQuery tests, this has come up frequently on PRs I've worked on

Winterhart commented 1 year ago

I think we can merge on main?