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

Clearing visual regressions #761

Closed nidhi-kala closed 1 year ago

nidhi-kala commented 1 year ago

Description

Addressing #736

Before After
image Screenshot 2023-04-13 at 15 27 57
Before After
image Screenshot 2023-04-13 at 16 42 06
Before After
image Screenshot 2023-04-13 at 16 46 35
-All input field heights Before After
image Screenshot 2023-04-13 at 16 50 20
-Settings Before After
image Screenshot 2023-04-13 at 16 53 09

Checklist

changeset-bot[bot] commented 1 year ago

🦋 Changeset detected

Latest commit: 06d392513da6b05ad5a22ac2e45a9e8d98e973a8

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

This PR includes changesets to release 3 packages | Name | Type | | ------------------------- | ----- | | @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 (UTC)
evidence-development-workspace ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 19, 2023 7:02pm
evidence-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 19, 2023 7:02pm
vercel[bot] commented 1 year ago

@nidhi-kala is attempting to deploy a commit to the Evidence Dev Team on Vercel.

A member of the Team first needs to authorize it.

nidhi-kala commented 1 year ago

@ItsMeBrianD Hi! I resolved the merge conflicts. Let me know if there's anything else that needs to be done. :)

archiewood commented 1 year ago

could you run the code though our (shiny new) formatter and commit the result? pnpm run format

nidhi-kala commented 1 year ago

could you run the code though our (shiny new) formatter and commit the result? pnpm run format

Ofcourse! just did that.

archiewood commented 1 year ago

Hi @nidhi-kala,

Mostly looks great.

A couple of things I noticed:

nidhi-kala commented 1 year ago

@archiewood I fixed the dropdown menu height. Thanks for pointing it out :)

The settings icon were the same dimensions as 15.0.1. these are the dimensions of the settings icon in my branch:

Screenshot 2023-04-17 at 14 57 34

And these are in 15.0.1:

Screenshot 2023-04-17 at 14 51 37

which looks like:

Screenshot 2023-04-17 at 14 36 17

I think 24*24 looks better, let me know if this works:

Screenshot 2023-04-17 at 15 00 41

I will commit the changes shortly!

archiewood commented 1 year ago

Settings menu

While that span may still be at 22px, the icon is definitely smaller visually than it used to be.

15.0.1

image image

main

image image

I think the culprit is the tailwind upgrade, which has overriden the defaults tags using border-box rather than content-box CleanShot 2023-04-17 at 22 59 03

My suspicion is that we will need to override this for this element

ItsMeBrianD commented 1 year ago

I would attempt to style the svg, rather than .settings-icon using the selector .settings-icon :global(svg), that should let us keep border-box and also style the icon to the precisely correct size.

nidhi-kala commented 1 year ago

I would attempt to style the svg, rather than .settings-icon using the selector .settings-icon :global(svg), that should let us keep border-box and also style the icon to the precisely correct size.

Sounds great. This gives us more control over the icon's size and styling. I'll proceed with this approach and adjust the dimensions. Thanks for the suggestion.

ItsMeBrianD commented 1 year ago

I went ahead and applied this fix to unblock us for the day. I believe that is the last issue we have here

image

archiewood commented 1 year ago

Thanks @nidhi-kala & @ItsMeBrianD !