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

Dynamically set y-axis label width for credits chart #12625

Merged
merged 2 commits into from
May 6, 2022

Conversation

krishnaglick
Copy link
Contributor

@krishnaglick krishnaglick commented May 5, 2022

Fixes #11586

This is honestly a little hackier than I wanted, but we don't have a way to dynamically set the width

@krishnaglick krishnaglick requested a review from a team as a code owner May 5, 2022 19:58
@CLAassistant
Copy link

CLAassistant commented May 5, 2022

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot added area/frontend area/platform issues related to the platform labels May 5, 2022
@@ -16,6 +16,11 @@ const BarChart: React.FC<BarChartProps> = ({ data, legendLabels, xLabel, yLabel
const chartLinesColor = theme.greyColor20;
const chartTicksColor = theme.lightTextColor;

const width = Math.min(
Copy link
Contributor

Choose a reason for hiding this comment

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

There's some sorting happening. I think it deserves a memo

@@ -16,6 +16,11 @@ const BarChart: React.FC<BarChartProps> = ({ data, legendLabels, xLabel, yLabel
const chartLinesColor = theme.greyColor20;
const chartTicksColor = theme.lightTextColor;

const width = Math.min(
Math.max([...data].sort((a, b) => b.value - a.value)[0].value.toFixed(0).length * 10, 80),
Copy link
Contributor

Choose a reason for hiding this comment

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

Assumes that there's always at least 1 item in the array?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The default data is an array of dates + values at 0.

@krishnaglick krishnaglick merged commit ce1774b into master May 6, 2022
@krishnaglick krishnaglick deleted the kc/dynamic-credits-chart-width branch May 6, 2022 13:29
suhomud pushed a commit that referenced this pull request May 23, 2022
* This feels a little hacky, but we don't have a way to dynamically set width otherwise.

* memo width
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/platform issues related to the platform
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Credits chart axis should calculate dynamic width
3 participants