evidence icon indicating copy to clipboard operation
evidence copied to clipboard

[WIP] Add `ReferencePoint` and `Callout` components

Open zachstence opened this issue 1 year ago • 8 comments

Description

Checklist

  • [x] For UI or styling changes, I have added a screenshot or gif showing before & after
  • [x] I have added a changeset
  • [ ] I have added to the docs where applicable
  • [x] I have added to the VS Code extension where applicable

zachstence avatar Jun 26 '24 21:06 zachstence

🦋 Changeset detected

Latest commit: 92faafd5afbd32ea7c2fce4ffea747ac321250b8

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/core-components Minor
@evidence-dev/evidence Major
my-evidence-project 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

changeset-bot[bot] avatar Jun 26 '24 21:06 changeset-bot[bot]

Deploy Preview for evidence-development-workspace ready!

Name Link
Latest commit 92faafd5afbd32ea7c2fce4ffea747ac321250b8
Latest deploy log https://app.netlify.com/sites/evidence-development-workspace/deploys/668573ebe4651b0009db5795
Deploy Preview https://deploy-preview-2164--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.

netlify[bot] avatar Jun 26 '24 21:06 netlify[bot]

Deploy Preview for next-docs-evidence ready!

Name Link
Latest commit 92faafd5afbd32ea7c2fce4ffea747ac321250b8
Latest deploy log https://app.netlify.com/sites/next-docs-evidence/deploys/668573ebac241f000896867a
Deploy Preview https://deploy-preview-2164--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] avatar Jun 26 '24 21:06 netlify[bot]

Deploy Preview for evidence-test-env ready!

Name Link
Latest commit 92faafd5afbd32ea7c2fce4ffea747ac321250b8
Latest deploy log https://app.netlify.com/sites/evidence-test-env/deploys/668573eb4f0fa5000883da95
Deploy Preview https://deploy-preview-2164--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] avatar Jun 26 '24 21:06 netlify[bot]

ReferencePoint image

Callout image

zachstence avatar Jun 28 '24 18:06 zachstence

lovely!

so the solution if you have a short string and you want it to not have extra space in the callout is to do width=0?

archiewood avatar Jun 28 '24 19:06 archiewood

lovely!

so the solution if you have a short string and you want it to not have extra space in the callout is to do width=0?

@archiewood Hmm actually it would be width={undefined}, which isn't great... Maybe we should default to no width for the Callout too?

zachstence avatar Jun 28 '24 20:06 zachstence

@archiewood Or it could be width=fit

zachstence avatar Jun 28 '24 20:06 zachstence

This is really nice!

It looks like there might be an issue with the default yellow color - getting some flickering on hover. In the past, this has been caused by echarts not knowing how to adjust the color for the hover state (can't remember exactly what triggered it, but something to do with the color code):

CleanShot 2024-07-02 at 12 36 45

hughess avatar Jul 02 '24 16:07 hughess

Only other point of feedback I have on the styling is: I think the grey of the label border is too different vs the grey of the text.

Might be best to line up with the grey we use for the tooltip border, or maybe a shade darker (+ maybe match the same border radius, slightly less than the current callout radius). That way the greys are on the same scale

hughess avatar Jul 02 '24 19:07 hughess

Overall like. Good design

QA

  • Still getting flicker on the yellow point in the docs

    • Weird hover stuff? CleanShot 2024-07-02 at 19 35 52
  • maybe it's just a browser thing but I'm seeing almost a drop shadow top-left behind the callout (macos, firefox) CleanShot 2024-07-02 at 19 32 54@2x

  • Do i need to be able to pass an array to labelposition to fix stuff like this for just one of my labels? CleanShot 2024-07-02 at 19 36 29@2x

  • Cursor should probably not be pointer as no action on click CleanShot 2024-07-02 at 19 43 52

archiewood avatar Jul 02 '24 23:07 archiewood

@archiewood Thank you for the feedback!

Weird hover stuff? Cursor should probably not be pointer as no action on click

Fixed by making reference points not hoverable (this is in line with reference area/line functionality)

maybe it's just a browser thing but I'm seeing almost a drop shadow top-left behind the callout (macos, firefox)

It is likely just something with how ECharts is rendering the one pixel border, it may have been on a pixel border on your monitor resulting in it looking a little thicker. I don't think there's anything I can do to control that.

Do i need to be able to pass an array to labelposition to fix stuff like this for just one of my labels?

I don't know... I feel like if you're using data you would have a dynamic number of items and therefore configuring via a prop wouldn't make much sense. Options I see here:

  • Have labelPosition accept a column from data to use for positioning each point
  • The user just has to use width/labelPosition/add line breaks to try to get the label to not overflow in any spot

I wish ECharts had some way to detect/prevent overflow, but I don't think they do

zachstence avatar Jul 03 '24 14:07 zachstence

Fixed by making reference points not hoverable (this is in line with reference area/line functionality)

it still greys out the Callouts and ReferencePoints when you hover over data series, which is different to the ReferenceLine & Area behaviour

CleanShot 2024-07-03 at 11 28 34

archiewood avatar Jul 03 '24 15:07 archiewood