Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BigValue Component #379

Merged
merged 10 commits into from Aug 16, 2022
Merged

BigValue Component #379

merged 10 commits into from Aug 16, 2022

Conversation

mcrascal
Copy link
Member

@mcrascal mcrascal commented Aug 12, 2022

Adds a beloved BI feature: the Big Number Viz™ to Evidence.

Includes:

  • Number
  • Sparkline
  • Comparison
  • Comparison is configurable for "down is good"
  • Multiple Big Numbers stack into a row
  • Smart enough defaults that only the data prop is required

BigValue Example

@changeset-bot
Copy link

changeset-bot bot commented Aug 12, 2022

🦋 Changeset detected

Latest commit: 50e713e

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 Patch
@evidence-dev/evidence Patch
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
Copy link

vercel bot commented Aug 12, 2022

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

Name Status Preview Updated
evidence-docs ✅ Ready (Inspect) Visit Preview Aug 16, 2022 at 3:06PM (UTC)

@archiewood
Copy link
Member

archiewood commented Aug 12, 2022

Love this feature!
Some implementation details

Defaults

  1. Not sure the default behaviour is working perfectly here. Or maybe it is but the results are a bit puzzling.
    • I think I'm using it wrong, but feel like the acid test here is what if I do the naive-est thing. Don't make me read the docs to work out how to use it

Eg1: Grabs a string as a "comparison"

```orders
select * from orders limit 10
```

<BigValue data={orders}/>

image

Eg2: Fails when two columns are passed. I think this should succeed, but without a comparison
image

Eg3: an unexpected comparison (and cropped sparkline, see more below)
image

I know I'm tuning 180, but maybe we should try to infer less...

  • Perhaps the value prop is mandatory, and the others are off by default?
  • (Aside from title, for which inferring the column name seems sensible)

Visual Nits (for considerations)

  1. It looks a bit like the sparkline is cropped at the bottom (but actually I think it is to do with the svg "brushstroke", rather than margins etc?)

image

  1. For a BigValue, the Value could be a bit Bigger, perhaps closer to H1 than H2 size - cant remember exactly how em vs rem work but 1.5em looks like this:

image

  1. Not sure this is an issue but the error message bleeds out rather than truncates. Also other components label what they are in this error message (eg LineChart)

image

File structure

  1. Any reason this is not in the Viz folder with other chart / value components?

image

@mcrascal
Copy link
Member Author

mcrascal commented Aug 12, 2022

Thanks @archiewood, excellent feedback as always. Just consolidating here:

  • Drop the default system
  • Tidy up error messages
  • Move the component to viz, and add it to the preprocessor default imports
  • Take one more look at sizing and consider nudging up

@mcrascal
Copy link
Member Author

@archiewood updates in the branch, except for the size changes. Take a look, and feel free to merge if you're comfortable.

Once you get the actual value close to H1, two things happen:

  1. They compete with the H1 Headers, so it's harder to understand where they necessarily fit.
  2. Just a lot less space for these.

I'd propose shipping at the current size and seeing how that feels in practice.

Current Size:
CleanShot 2022-08-15 at 14 52 16@2x

H1 Sized:
CleanShot 2022-08-15 at 14 51 40@2x

@archiewood
Copy link
Member

archiewood commented Aug 15, 2022

Will have a play around with the new lack of defaults

Re sizing: happy to ship as is:

  • I was probably thinking of making just the number sized H1, not the title of the <BigValue/>. Agree above definitely competing with actual H1s

@mcrascal
Copy link
Member Author

Ah got it

Yeah looks pretty good IMO

CleanShot 2022-08-15 at 15 58 28@2x

@archiewood
Copy link
Member

archiewood commented Aug 16, 2022

EDIT: TableOfContents not Breadcrumbs

I noticed that if you are in a wide enough viewport to see it, the <TableofContents/> shows "Big Value" every time there is one, as the logic in this displays all the H1 elements.

image

Solved this by using the <ErrorChart/> component for Big Value's error message, which styles the Error differently.

image

@mcrascal
Copy link
Member Author

Thank you @archiewood 🚢

@mcrascal mcrascal merged commit 13d1acd into main Aug 16, 2022
@mcrascal mcrascal deleted the metric-cards branch August 16, 2022 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants