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

FunnelChartNext follow ups #1788

Merged
merged 1 commit into from
Jan 8, 2025
Merged

Conversation

michaelnesen
Copy link
Collaborator

@michaelnesen michaelnesen commented Jan 7, 2025

What does this implement/fix?

This PR removes the main percentage label in FunnelChartNext, and addresses a few follow up issues:

Does this close any currently open issues?

What do the changes look like?

  • Main percentage is no longer shown at the top of the viz
Screenshot 2025-01-07 at 5 46 44 PM
  • In the event that negative values are passed into the chart, we now handle them more gracefully:
Screenshot 2025-01-07 at 5 49 27 PM

Storybook link

https://6062ad4a2d14cd0021539c1b-yjjtqaptpo.chromatic.com/

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

@michaelnesen michaelnesen force-pushed the funnel-chart-next-follow-ups branch from bae23b5 to 97c5af0 Compare January 7, 2025 22:48
Copy link

github-actions bot commented Jan 7, 2025

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
polaris-viz-core-cjs 61.73 KB (0%) 1.3 s (0%) 833 ms (+2.66% 🔺) 2.1 s
polaris-viz-cjs 228.78 KB (-0.07% 🔽) 4.6 s (-0.07% 🔽) 1.8 s (-14.35% 🔽) 6.4 s
polaris-viz-esm 185.24 KB (-0.04% 🔽) 3.8 s (-0.04% 🔽) 1.4 s (+3.9% 🔺) 5.1 s
polaris-viz-css 5.72 KB (0%) 115 ms (0%) 299 ms (-4.03% 🔽) 413 ms
polaris-viz-esnext 192.29 KB (-0.04% 🔽) 3.9 s (-0.04% 🔽) 1.5 s (-2.06% 🔽) 5.3 s

Copy link
Collaborator

@mollerjorge mollerjorge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested edge cases values in story book (negative, cero, big and small numbers), they look good!

Thanks for adding the tests as well 🙏

@michaelnesen michaelnesen merged commit f3dd276 into main Jan 8, 2025
6 checks passed
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.

2 participants