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

🐛 (stacked discrete bar) wrap long labels #3341

Merged
merged 1 commit into from
Mar 14, 2024

Conversation

sophiamersmann
Copy link
Member

@sophiamersmann sophiamersmann commented Mar 13, 2024

Related: #2629

We should also do this for DiscreteBar charts, but I don't have time now. I added a note to the issue above 👆🏻

Before After
Screenshot 2024-03-13 at 20 42 53 Screenshot 2024-03-13 at 20 42 41
Screenshot 2024-03-13 at 20 42 07 Screenshot 2024-03-13 at 20 42 22

Copy link
Member Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @sophiamersmann and the rest of your teammates on Graphite Graphite

@sophiamersmann sophiamersmann force-pushed the stacked-discrete-bar-wrap-labels branch 2 times, most recently from 2e18aa1 to 8071ecc Compare March 13, 2024 20:15
@sophiamersmann sophiamersmann changed the title 🐛 (stacked discrete bar) wrap long labels 🐛 (discrete bar) wrap long labels Mar 13, 2024
@sophiamersmann sophiamersmann changed the title 🐛 (discrete bar) wrap long labels 🐛 (stacked discrete bar) wrap long labels Mar 13, 2024
@sophiamersmann sophiamersmann marked this pull request as ready for review March 13, 2024 21:17
@sophiamersmann sophiamersmann force-pushed the stacked-discrete-bar-wrap-labels branch from 8071ecc to 00e7dad Compare March 13, 2024 21:17
Comment on lines +393 to +397
// useful if `this.barHeight` can't be used due to a cyclic dependency
// keep in mind though that this is not exactly the same as `this.barHeight`
@computed private get approximateBarHeight(): number {
return (0.8 * this.boundsWithoutLegend.height) / this.barCount
}
Copy link
Member Author

Choose a reason for hiding this comment

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

There is a circular dependency here that I can't get rid of easily. If I remember correctly, it's something like: labelFontSize -> barHeight -> innerBounds -> labelWidth -> labelStyle -> labelFontSize.

Breaking the cycle by using boundsWithoutLegend instead of innerBounds is not new, but I added a computed property that makes it explicit.

Copy link
Member

@marcelgerber marcelgerber left a comment

Choose a reason for hiding this comment

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

Nice, this looks great!

It would be awesome if we had some way to have more balanced lines (i.e. that each line is roughly the same length), but I realize that that's not easy to do. And this is already soo much better than what it was before.

@sophiamersmann sophiamersmann force-pushed the stacked-discrete-bar-wrap-labels branch from 00e7dad to 128013c Compare March 14, 2024 15:59
@sophiamersmann
Copy link
Member Author

Maybe write an issue for it? It could be a cool cooldown project :)

@sophiamersmann sophiamersmann merged commit 3901d26 into master Mar 14, 2024
24 of 30 checks passed
@sophiamersmann sophiamersmann deleted the stacked-discrete-bar-wrap-labels branch March 14, 2024 17:07
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