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

[Bug Fix]: Adjust tooltip position on StackedAreaChart when page is scrolled #1595

Merged
merged 1 commit into from
Oct 17, 2023

Conversation

euripidean
Copy link
Contributor

@euripidean euripidean commented Sep 29, 2023

What does this implement/fix?

Fixes: https://github.com/Shopify/core-issues/issues/60331

Issue
Received a possible bug that the ReturningCustomerRate card was not rendering a tooltip.

  • Investigated and found that the tooltip was being rendered, but that when the window was scrolled, the tooltip was rendering outside of the bounds of the chart and was no longer visible. It was also being partially cut off when the scroll value was lower.
Screen.Recording.2023-09-22.at.2.32.57.PM.mov
  • To replicate the issue, staff in to any store and navigate to the Dashboard. Ensure the ReturningCustomerRate card is displayed (currently the only StackedArea chart available).
  • If you move this card to a place on the dashboard and have to scroll to access it, hover over and you'll notice the tooltip does not appear in the expected position.
  • Move the card to the top of the dashboard and hover over it. Without scroll, the tooltip will appear as expected.
  • You can experiment with different scroll amounts to see the full behaviour.

Does this close any currently open issues?

Fixes https://github.com/Shopify/core-issues/issues/60331

What do the changes look like?

Note: Tested in Storybook - need to confirm this fix works once updated and available in web.

Screen.Recording.2023-09-29.at.12.43.20.PM.mov

Extra Thoughts

There are a few similar functions across the charts now to adjust the position of the tooltip in various scenarios. I was thinking it might be worth combining these into one function to adjust the positioning that (ideally) works for all of the charts.

Happy to look into this further if there's consensus this needs to happen.

Storybook link

Before merging

  • Check your changes on a variety of browsers and devices.

  • Update the Changelog's Unreleased section with your changes.

  • Update relevant documentation, tests, and Storybook.

  • Make sure you're exporting any new shared Components, Types and Utilities from the top level index file of the package

@euripidean euripidean self-assigned this Sep 29, 2023
@github-actions
Copy link

github-actions bot commented Sep 29, 2023

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
polaris-viz-core-cjs 61.3 KB (0%) 1.3 s (0%) 1.5 s (-24.96% 🔽) 2.7 s
polaris-viz-cjs 211.12 KB (+0.03% 🔺) 4.3 s (+0.03% 🔺) 2.6 s (+5.77% 🔺) 6.8 s
polaris-viz-esm 173.51 KB (+0.04% 🔺) 3.5 s (+0.04% 🔺) 3 s (+11.53% 🔺) 6.5 s
polaris-viz-css 4.57 KB (0%) 92 ms (0%) 534 ms (-19.16% 🔽) 625 ms
polaris-viz-esnext 178.6 KB (+0.04% 🔺) 3.6 s (+0.04% 🔺) 2.4 s (-13.98% 🔽) 5.9 s

@envex
Copy link
Collaborator

envex commented Oct 4, 2023

@euripidean You can setup a story in storybook to test the scrolling without having to put it in web: https://github.com/Shopify/polaris-viz/blob/main/packages/polaris-viz/src/components/BarChart/stories/playground/ExternalTooltip.stories.tsx

We also have the getAlteredLineChartPosition function you may be able to lean into.

@euripidean euripidean force-pushed the stacked-area-tooltip branch from d7bad2e to 71e9908 Compare October 6, 2023 19:38
@euripidean
Copy link
Contributor Author

@euripidean You can setup a story in storybook to test the scrolling without having to put it in web: https://github.com/Shopify/polaris-viz/blob/main/packages/polaris-viz/src/components/BarChart/stories/playground/ExternalTooltip.stories.tsx

We also have the getAlteredLineChartPosition function you may be able to lean into.

Added the story. I did use the getAlteredLineChartPosition function as the basis for the getAlteredStackedAreaChartPosition function, but let me know if there's anything you think I missed.

@euripidean euripidean force-pushed the stacked-area-tooltip branch from 71e9908 to d4f4c2b Compare October 17, 2023 17:32
@euripidean euripidean merged commit 7f712b9 into main Oct 17, 2023
4 checks passed
@euripidean euripidean deleted the stacked-area-tooltip branch October 17, 2023 17:39
@shopify-shipit shopify-shipit bot temporarily deployed to production October 17, 2023 18:04 Inactive
@ShopKlaw ShopKlaw added the #gsd:31444 Analytics Overview Dashboard Customization label Oct 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
#gsd:31444 Analytics Overview Dashboard Customization
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants