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

Treemap should also observe the fillLabel.padding setting #659

Closed
wylieconlon opened this issue Apr 30, 2020 · 3 comments · Fixed by #660
Closed

Treemap should also observe the fillLabel.padding setting #659

wylieconlon opened this issue Apr 30, 2020 · 3 comments · Fixed by #660
Labels
enhancement New feature or request :partition Partition/PieChart/Donut/Sunburst/Treemap chart related released Issue released publicly :styling Styling related issue

Comments

@wylieconlon
Copy link

Thanks @monfera for merging the update to add grooved text! I'm trying it out, and found what looks like a bug. I see in the "groove label" storybook example the commented out settings for fillLabel.padding, but when I uncomment the settings nothing changes. This is also happening in my Lens configuration:

Screenshot 2020-04-30 18 16 53

@wylieconlon wylieconlon added bug Something isn't working :styling Styling related issue :partition Partition/PieChart/Donut/Sunburst/Treemap chart related labels Apr 30, 2020
@monfera
Copy link
Contributor

monfera commented May 1, 2020

@wylieconlon correct, see description. As it's a different layouting it hasn't yet been added back, the plan is to reinstate it in a next PR now that #658 is merged - it's very motivating to see it with this small typeface where it's most felt, thanks!

Btw. it looks good with these EUI colors! Maybe it'd be better for group separation and group label readability to not color the groups themselves (it's unrelated to your note so I don't view it as a design decision). The grooves are also height constrained, sandwiching the group label text, which therefore have inherent visual noise above and below, so using color there has a worse effect than coloring the leaf nodes themselves. The group delineation also varies with lightness, Men's Shoes is better separated than Women's Accessories

@monfera
Copy link
Contributor

monfera commented May 1, 2020

Minor note, the padding was never in effect for the treemap but it wasn't obvious as text was centered verticlaly and horizontally, so I'm removing the bug label. Also, the padding needs to be more sophisticated - per direction, eg. paddingLeft can be important for groove labels but uniform all around padding would remove too much from the vertical space

@monfera monfera added enhancement New feature or request and removed bug Something isn't working labels May 1, 2020
@monfera monfera changed the title Treemap labels aren't using fillLabel.padding setting Treemap should also observe the fillLabel.padding setting May 1, 2020
monfera added a commit that referenced this issue May 5, 2020
This adds per-layer, per-side configurability for padding between a piece of treemap text and its rectangular container.

close #659
@markov00
Copy link
Member

markov00 commented May 5, 2020

🎉 This issue has been resolved in version 19.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@markov00 markov00 added the released Issue released publicly label May 5, 2020
AMoo-Miki pushed a commit to AMoo-Miki/OpenSearch-Dashboards that referenced this issue Feb 10, 2022
This adds per-layer, per-side configurability for padding between a piece of treemap text and its rectangular container.

close elastic/elastic-charts#659
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request :partition Partition/PieChart/Donut/Sunburst/Treemap chart related released Issue released publicly :styling Styling related issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants