evidence-dev / evidence

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

Base map legends #2647

Closed kwongz closed 1 month ago

kwongz commented 1 month ago

Description

Map Legends

(Video Recording not properly catching show legend transition)

https://github.com/user-attachments/assets/4540ce85-6d06-4893-bf5e-bb149f5b9e5a

https://github.com/user-attachments/assets/0a1a6e30-0132-4f27-8923-650830caa347

  1. Takes in data column from Value prop to create legend
  2. Categorical Legends output distinct strings and map them to colorPallete (handles null values)
  3. Scalar Legends create a color gradient scale based on numerical max and min values
  4. Default color handling + max/min fmt
  5. Legend positioning (bottomLeft, bottomRight, topLeft, topRight)
  6. New single legend design
  7. Moved Legend component into Basemaps
  8. Refactored Legend component
  9. Dynamic Categorical and Scalar Legends based on legenType Prop
  10. Multi-Legend added for BaseMap
  11. Smoother animations and transitions

Checklist

changeset-bot[bot] commented 1 month ago

πŸ¦‹ Changeset detected

Latest commit: bec59740b1517d0f013cc66ef732cc9142474cc0

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

This PR includes changesets to release 7 packages | Name | Type | | ----------------------------- | ----- | | @evidence-dev/core-components | Patch | | my-evidence-project | Patch | | e2e-prerender | Patch | | e2e-spa | Patch | | e2e-themes | Patch | | @evidence-dev/components | 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[bot] commented 1 month ago

The latest updates on your projects. Learn more about Vercel for Git β†—οΈŽ

Name Status Preview Comments Updated (UTC)
docs βœ… Ready (Inspect) Visit Preview πŸ’¬ Add feedback Oct 24, 2024 2:06pm
next-docs βœ… Ready (Inspect) Visit Preview πŸ’¬ Add feedback Oct 24, 2024 2:06pm
netlify[bot] commented 1 month ago

Deploy Preview for evidence-test-env ready!

Name Link
Latest commit bec59740b1517d0f013cc66ef732cc9142474cc0
Latest deploy log https://app.netlify.com/sites/evidence-test-env/deploys/671a53674dc6a7000888f930
Deploy Preview https://deploy-preview-2647--evidence-test-env.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify[bot] commented 1 month ago

Deploy Preview for next-docs-evidence ready!

Name Link
Latest commit bec59740b1517d0f013cc66ef732cc9142474cc0
Latest deploy log https://app.netlify.com/sites/next-docs-evidence/deploys/671a5367acd4d500082c2500
Deploy Preview https://deploy-preview-2647--next-docs-evidence.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify[bot] commented 1 month ago

Deploy Preview for evidence-development-workspace ready!

Name Link
Latest commit bec59740b1517d0f013cc66ef732cc9142474cc0
Latest deploy log https://app.netlify.com/sites/evidence-development-workspace/deploys/671a5367d3bd8f000813a810
Deploy Preview https://deploy-preview-2647--evidence-development-workspace.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

kwongz commented 1 month ago

found a weird bug occurring on local docs and not on storybook, duplicate legend data causes double legends

image

kwongz commented 1 month ago

i noticed the init function was running multiple times, (3 -2)

Current solution:

Move init() to onMount, seems to also speed up map rendering

just uncertain about other consequences, but looks good now

Update: Solved with 2nd await init() then block, as well as legend data duplicate update handling for dev mode

kwongz commented 1 month ago

Looking into default legend rendering for maps

kwongz commented 1 month ago

@hughess

Issue

Looking into legends as default, but legends depend on Value prop which is not required for maps.

image

I think a nice solution is the legend comes with the Value prop, and users can control showing the legend with a noLegend prop

Solution

Legend w/ Default

image

image

No legend

image

image

kwongz commented 1 month ago

@zachstence

Just spoke with @hughess, would we be able to get rid of the text, and add a icon to represent the map with a label for screen readers?

kwongz commented 1 month ago

@hughess

Update

  1. Legends only require value prop, to hide legend legend=false
  2. After talking to @zachstence we would like to pitch the chevron with screen readers for accessibility
  3. overflow fix for category legends

https://github.com/user-attachments/assets/8f331c07-b1f0-46c3-9253-3a755feec995

Summary Other fixes:

  1. No more duplicate legend rendering in docs
  2. Aligned multi legend scalar and category padding
kwongz commented 1 month ago

image

Currently, Legends appear when legends=true && value=!undefined.

kwongz commented 1 month ago

image

custom color example, color prop makes all areas the same color.

Should we handle this? or should we and users use the legend=false prop for this case

hughess commented 1 month ago

Hmm. What happens if you also supply a colorPalette prop? Does the palette override?

my initial take is that we should set legend to false automatically when color is used in this way

hughess commented 1 month ago

I think default logic is perfect

Default behaviour should be: legend appears when variable Colors exist in the map (which is driven by value prop in this case). Legend prop is then available as a fallback to turn it off in the 5% of cases where people want to do that

We may need to handle the situation for this:

<BaseMap>
    <Areas data={south_region_postcodes} color=red/>
    <Areas data={north_region_postcodes} color=green/>
</BaseMap>
kwongz commented 1 month ago

@hughess

Problem: currently color overrides colorPalette prop. Looking at the points/areas component, it seems color is only used create 1 overall color scheme.

image

image

Solution:

agreed, added a conditional to init legends was colors is undefined

image

Colors provided, resulting in no legend image

Will leave legend logic as is, legend default as true, requires Value prop

kwongz commented 1 month ago

"We may need to handle the situation for this:" @hughess, which situation are you referring to?

hughess commented 1 month ago

@kwongz sorry i put the code outside a code block and it didn't render in the comment - just updated it!

kwongz commented 1 month ago

would it be confusing to move or duplicate the value prop into the legends section with a requirement? Alternatively we can use the docs example i post above.

add a required to value and move/duplicate it inside the legend options section image

image

The only issue i can see is user may interpret they can actually access the legend prop <Legend value={data.column}, which is not how it is built.

hughess commented 1 month ago

I don't think you need to put value in the legend section.

The ideal situation is that the user doesn't need to think about the legend in 95% of scenarios - it just appears when it makes sense, and doesn't when there isn't relevant data. Then in the 5% of other cases where they need more control, they can easily find the props to turn it off or change it

kwongz commented 1 month ago

image

hmmm what would users want from this scenario from a legend?

My first thoughts on colors vs colorPallete Legend handling:

  1. Instead, users could create two categorical legends, one with red and one with green.

  2. What is the value category in this case?

  3. Why might a legend with color with 2 areas be necessary instead of using a color palette like [red, green] with a single area or areamap? (reason may depend on the value column)

  4. I would only use the color prop if my dataset has a wide range of values and I want to display just one color on the map. In that case, a legend might not be necessary or be helpful to me.

kwongz commented 1 month ago

Awesome, Thanks @hughess! I'll leave the docs as is

image

kwongz commented 1 month ago

Updates

Added truncation + on hover title for category text Added semiBold font to Title Fixed Chev directions

image

pulled css scrollbar styles from dataTables and Queries into legend scroll bar (may need to put in global css styles) added min-width for categories to add space between scroll bar at text

image

kwongz commented 1 month ago

@hughess

Chevron Arrows now respect legend positions

image image

image image

kwongz commented 1 month ago

@hughess did a last min QA on docs, updated and fix some minor things. Refined Map legend design w/ @mcrascal

If all looks good we can merge!

hughess commented 1 month ago

In the docs preview the open close function doesn’t work

kwongz commented 1 month ago

Sorry about that, its a older preview with a slower transition delay for testing. A deploy completed soon after your comment. Open and close is good