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

Layout rev3 #1079

Closed mcrascal closed 9 months ago

mcrascal commented 9 months ago

This is ready for feedback:

Goals

  1. Modernize the overall layout and make the project feel more cohesive
  2. Make it easier to keep things isolated by targetting user markdown with the .markdown class, instead of global styles
  3. Give us a good starting point for tackling some more visual improvements as smaller PRs

Not goals

  1. Restyle every component. I've tried to make minimal changes to existing components just to keep things working. Where I did, this was usually a tailwind conversion inside existing classes.
  2. Fix every existing visual issue or bug. That said, this closes a few.

Closes #573, closes #712, closes #739, closes #777, closes #501

Notable changes

  1. Default font -> Inter
  2. Sidebar, header, table of contents
  3. Adds https://svelte-headlessui.goss.io/docs/2.0/dialog to make an accessible drop down menu, which I wouldn't mind using in a few other places.
  4. Type: headers, and some other type details
  5. Table of contents excludes the page title when set through frontmatter @archiewood
  6. Colours: back to tailwind defaults. Despite spending a fair amount of time obsessing over gray-200, our existing palette was quite close to tailwind's defaults anyways.

Feedback

For feedback, it'd be helpful to get it broken into two categories:

  1. Major issues: things that should prevent us from shipping this.
  2. Follow ons: small nits and things you don't like, but that we can tackle in smaller chunks.

Next Steps

changeset-bot[bot] commented 9 months ago

🦋 Changeset detected

Latest commit: 57fb18e45af40bc5df3807063060ba929ece42c0

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

This PR includes changesets to release 5 packages | Name | Type | | ------------------------------ | ----- | | @evidence-dev/evidence | Minor | | @evidence-dev/universal-sql | Patch | | evidence-test-environment | Patch | | @evidence-dev/plugin-connector | Patch | | @evidence-dev/components | 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 9 months ago

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

Name Status Preview Comments Updated (UTC)
evidence-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 7, 2023 6:03pm
netlify[bot] commented 9 months ago

Deploy Preview for evidence-development-workspace failed.

Name Link
Latest commit 57fb18e45af40bc5df3807063060ba929ece42c0
Latest deploy log https://app.netlify.com/sites/evidence-development-workspace/deploys/64d15b477e538000087661e3
archiewood commented 9 months ago

Dig the skeleton a lot

ItsMeBrianD commented 9 months ago

@csjh can you take a look at making the tailwind palette import-able into component utilities. We have a colors file that I'd like to link back to the tailwind config. I don't view this as blocking.

the colors const is exported from the tailwind configuration package; so we might just need to update the colors to be hex instead of hsla, and that should be enough.

csjh commented 9 months ago

the colors const is exported from the tailwind configuration package; so we might just need to update the colors to be hex instead of hsla, and that should be enough.

Does tailwind not recognize HSLA? Or am I misunderstanding

csjh commented 9 months ago

@csjh can you help me move the fonts out of example project to be packaged with the tailwind package?

As in, put the .woff files in the tailwind package and use them as static files from there?

mcrascal commented 9 months ago

@csjh

Re colors -- I want this block to depend on the colours from whatever tailwind theme is present in the project (similar to how the css vars are created in app.css). Just trying to remove hardcodes, and centralize this type of base config in tailwind.

Re fonts -- I am not sure if that's the best approach, but I have a hunch they should not live in the template's static directory. I'd like them to be accessible to core-components, (or anything else that's using our tailwind preset), and I'd like us to be able to manage them in one place.

ItsMeBrianD commented 9 months ago

Does tailwind not recognize HSLA? Or am I misunderstanding

It does, but the values have an <alpha-value> in them which would break when trying to use them in JavaScript, hex wouldn't have this issue

mcrascal commented 9 months ago

If I open on mobile, I can "scroll" the navbar which feels weird

I am not totally sure what the screen cap is showing there. I can't get the header container to actually scroll, but I think you're looking at the overscroll behaviour, which was a bit weird in chrome. I've updated to give the header the same overscroll behaviour as what we have now.

I managed to get caught in a scroll issue on mobile - it was scrolling the page behind rather than the sidebar

Resolved, we're now scroll locking the body when the mobile/tablet sidebar is open.

The error boundaries are inconsistent between Value and Charts

Toned down the border on the value errors

error boundary alignment seems off, remove top margin / padding?

Only the fonts have been changed. See existing:

CleanShot 2023-08-07 at 10 17 55@2x

Same with the label. See existing:

CleanShot 2023-08-07 at 10 18 54@2x

maybe we should make it clearer when breadcrumbs are vs are not clickable? eg you cannot click on Ui components here, but the mouse still turns into a pointer on hover

Done. Clickable gets an underline on hover, pointer cursor. Un-clickable gets neither.

mcrascal commented 9 months ago

Closing in favour of #1086