search

tremorlabs / tremor

React components to build charts and dashboards
https://tremor.so
Apache License 2.0
15.66k stars 452 forks source link

feat: add maxValue and minValue prop to AreaChart, BarChart, and LineChart #217

Closed sam-b-rose closed 1 year ago

sam-b-rose commented 1 year ago

Adds an optional maxValue and minValue prop to the following components: AreaChart, BarChart, and LineChart.

This allows for more flexibility setting the domain of the YAxis component from Recharts.

This is needed for a use-case were the domain might be a percentage (0-100) and the data is only a small percentage.

Cheers!

storybook-example
sam-b-rose commented 1 year ago

Hi @mitrotasios or @christopherkindl! 👋 sorry to bother you directly, but any thoughts on this enhancement? Something we could consider adding?

mitrotasios commented 1 year ago

Hey @samrose3 sorry for the delayed response. Yeah I think that definitely makes sense, especially when you have percentage values as you mentioned. To keep it consistent I would suggest something like minValue and maxValue. Wdyt? We could include this in our next minor release potentially.

sam-b-rose commented 1 year ago

@mitrotasios perfect, I like minValue and maxValue 👍 I've updated the PR and added some stories to showcase/test the props.

mitrotasios commented 1 year ago

awesome thanks! How urgent is this for you?

sam-b-rose commented 1 year ago

Of course, happy to contribute! Awesome library.

How urgent is this for you?

Moderate urgency. We'd like to have the feature sooner than later to help complete a product feature, but not critical 👍

mitrotasios commented 1 year ago

cool, we'll try to merge this as soon as we can. Would you maybe like to hop on a call at any time? We would love to hear feedback and more about your use case in general. Feel free to email me: achi@tremor.so

sam-b-rose commented 1 year ago

Hi @mitrotasios just checking in on this. What is the Tremor release cadence? And anything I can help with that may be blocking this from merging into main?

mitrotasios commented 1 year ago

Hey @samrose3. There is no regular release cadence unfortunately because we work full time and our schedule changes here and there. We will release this most likely on Monday the latest though.

mitrotasios commented 1 year ago

hey @samrose3, I made a few changes to allow only numeric values as inputs for the minValue and maxValue props. We'd like to keep the API framework agnostic and avoid using recharts-specific inputs as we are considering migrating to another chart library eventually. I hope that's ok. Will merge this to beta now. Thanks a lot for the work! 🤗

github-actions[bot] commented 1 year ago

:tada: This PR is included in version 1.3.0-beta-feat-1.3.0.1 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

mitrotasios commented 1 year ago

@samrose3 JFYI, we will merge this to main tomorrow probably, but you can already use the beta version by npm installing @tremor/react@beta-feat-1.3.0 if you like :)

sam-b-rose commented 1 year ago

Woohoo!! Happy to help and thank you for accepting the contribution 🚀