feat(partition): treemap padding by monfera · Pull Request #660 · elastic/elastic-charts

monfera · 2020-05-01T13:36:48Z

Summary

Adds treemap text padding configurability by specifying

fillLabel: { padding: 3 } for the perimeter in pixels, or
fillLabel: {padding: { top: 0, bottom: 0, left: 2, right: 2 } } to allow per-side configuration

Closes #659

The default value is minimal, but non-zero (2px), to avoid minor text bleeding. This is in effect if padding is omitted altogether, or for unspecified sides if padding is an object.

It's a per-layer configuration, because constraints, font sizes etc. may vary per layer.

In particular, it's good practice to have very small (0..2px) top and bottom padding for in-groove group texts, while there's more freedom to adjust eg. left (and less importantly, right) to horizontally align the left edge of the groove text with

the logical are of the group
the left edge of leaf node ink
the left edge of the (potentially left-padded) node leaf text

The story 8_groove_text.tsx shows an example for horizontal alignment with the ink.

Reviewer notes:

the last commit is pure renaming
besides the padding example 8_groove_text.tsx the other treemap mock changes are due to the use of a 2px padding rather than reusing the overhang padding, the change is small though

Checklist

Delete any items that are not applicable to this PR.

Any consumer-facing exports were added to src/index.ts (and stories only import from ../src except for test data & storybook)
This was checked for cross-browser compatibility, including a check against IE11
Proper documentation or storybook story was added for features that require explanation or tutorials
Unit tests were updated or added to match the most common scenarios

codecov-io · 2020-05-01T13:42:10Z

Codecov Report

Merging #660 into master will increase coverage by 0.03%.
The diff coverage is 93.75%.

@@            Coverage Diff             @@
##           master     #660      +/-   ##
==========================================
+ Coverage   72.94%   72.97%   +0.03%     
==========================================
  Files         266      266              
  Lines        8618     8630      +12     
  Branches     1695     1696       +1     
==========================================
+ Hits         6286     6298      +12     
  Misses       2293     2293              
  Partials       39       39

Impacted Files	Coverage Δ
...types/partition_chart/layout/types/config_types.ts	`100.00% <ø> (ø)`
...es/partition_chart/layout/types/viewmodel_types.ts	`80.00% <ø> (ø)`
...artition_chart/renderer/canvas/canvas_renderers.ts	`6.20% <0.00%> (ø)`
...rtition_chart/layout/viewmodel/fill_text_layout.ts	`95.26% <100.00%> (+0.31%)`	⬆️
...tion_chart/layout/viewmodel/hierarchy_of_arrays.ts	`88.88% <100.00%> (ø)`
...ypes/partition_chart/layout/viewmodel/viewmodel.ts	`89.14% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 16d9974...536e000. Read the comment docs.

monfera · 2020-05-01T15:47:22Z

src/chart_types/partition_chart/layout/viewmodel/fill_text_layout.ts

 }

+type GetShapeRowGeometry = (
+  container: any, // fixme RectangleConstruction | RingSector,


This type centralizes a number of old references and replaces the former ...args: any[] blanket so it's an improvement; I'm planning to revisit this sole remaining parameter, likely in a next treemap PR, to make this config available asap

This could be also something like

type GetShapeRowGeometry<C> = ( container: C, ...

markov00

Code LGTM,
nit: if you can add a set of knobs to the groove text story it will give the reader a good idea on how the padding play a role in the rendering

markov00 · 2020-05-04T12:42:04Z

src/chart_types/partition_chart/layout/viewmodel/fill_text_layout.ts

 }

+type GetShapeRowGeometry = (
+  container: any, // fixme RectangleConstruction | RingSector,


This could be also something like

type GetShapeRowGeometry<C> = ( container: C, ...

src/chart_types/partition_chart/layout/types/config_types.ts

src/chart_types/partition_chart/layout/types/viewmodel_types.ts

markov00

Code LGTM

…charts into treemap-padding

# [19.2.0](v19.1.2...v19.2.0) (2020-05-05) ### Features * **partition:** treemap padding ([#660](#660)) ([ed1e8be](ed1e8be))

markov00 · 2020-05-05T14:32:56Z

🎉 This PR is included in version 19.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

# [19.2.0](elastic/elastic-charts@v19.1.2...v19.2.0) (2020-05-05) ### Features * **partition:** treemap padding ([opensearch-project#660](elastic/elastic-charts#660)) ([b462a38](elastic/elastic-charts@b462a38))

codecov-commenter · 2025-07-25T04:53:08Z

Codecov Report

Attention: Patch coverage is 94.28571% with 2 lines in your changes missing coverage. Please review.

Project coverage is 72.98%. Comparing base (019323a) to head (a2f64db).

Files with missing lines	Patch %	Lines
...artition_chart/renderer/canvas/canvas_renderers.ts	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #660      +/-   ##
==========================================
+ Coverage   72.94%   72.98%   +0.04%     
==========================================
  Files         266      266              
  Lines        8618     8632      +14     
  Branches     1695     1697       +2     
==========================================
+ Hits         6286     6300      +14     
  Misses       2293     2293              
  Partials       39       39

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

monfera added 3 commits May 1, 2020 11:45

refactor: name is groove, not padding

403d107

feat: detailed padding config options

a0bd23e

test: subtle image updates

5bbf750

monfera self-assigned this May 1, 2020

monfera added the wip work in progress label May 1, 2020

monfera added 2 commits May 1, 2020 17:14

test: unit tests

6456e54

refactor: pure rename rowCentroid to rowAnchor

a448bb0

monfera commented May 1, 2020

View reviewed changes

monfera requested review from markov00, nickofthyme, rshen91 and wylieconlon May 1, 2020 15:52

monfera added :chart Chart element related issue :partition Partition/PieChart/Donut/Sunburst/Treemap chart related enhancement New feature or request and removed wip work in progress labels May 1, 2020

markov00 approved these changes May 4, 2020

View reviewed changes

monfera added 6 commits May 4, 2020 15:39

Merge remote-tracking branch 'origin/master' into treemap-padding

9ef3f42

chore: turn certain exports internal

bd8b5d3

chore: turn certain exports internal

99406ef

refactor: common type for top-right-bottom-left

53511e5

chore: more things are internal

e8af1be

refactor: more specific type ht Marco

f28a44a

monfera requested a review from markov00 May 4, 2020 17:34

monfera added 3 commits May 4, 2020 20:14

test: adding padding knobs

91f491f

test: large story font for knobs

578b0c5

fix: horizontal center

22baa3d

markov00 approved these changes May 5, 2020

View reviewed changes

Merge branch 'master' into treemap-padding

edbf8b9

monfera added 6 commits May 5, 2020 12:14

fix: vertical padding on bottom align

7d79257

test: view model test update

ca10f5e

test: bottom alignment test added

7d8dc44

test: image updates

b2c7f3f

Merge branch 'treemap-padding' of https://github.com/monfera/elastic-…

536e000

…charts into treemap-padding

fix: empty row avoided

a2f64db

monfera merged commit ed1e8be into elastic:master May 5, 2020

monfera deleted the treemap-padding branch May 5, 2020 14:06

markov00 pushed a commit that referenced this pull request May 5, 2020

chore(release): 19.2.0 [skip ci]

4421748

# [19.2.0](v19.1.2...v19.2.0) (2020-05-05) ### Features * **partition:** treemap padding ([#660](#660)) ([ed1e8be](ed1e8be))

markov00 added the released Issue released publicly label May 5, 2020

markov00 mentioned this pull request May 5, 2020

Update dependency @elastic/charts to v19.2.0 elastic/kibana#65279

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(partition): treemap padding#660

feat(partition): treemap padding#660
monfera merged 21 commits intoelastic:masterfrom
monfera:treemap-padding

monfera commented May 1, 2020 •

edited

Loading

Uh oh!

codecov-io commented May 1, 2020 •

edited

Loading

Uh oh!

monfera May 1, 2020 •

edited

Loading

Uh oh!

markov00 May 4, 2020

Uh oh!

markov00 left a comment

Uh oh!

markov00 May 4, 2020

Uh oh!

Uh oh!

Uh oh!

markov00 left a comment

Uh oh!

markov00 commented May 5, 2020

Uh oh!

codecov-commenter commented Jul 25, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

monfera commented May 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Uh oh!

codecov-io commented May 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

monfera May 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

markov00 May 4, 2020

Choose a reason for hiding this comment

Uh oh!

markov00 left a comment

Choose a reason for hiding this comment

Uh oh!

markov00 May 4, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

markov00 left a comment

Choose a reason for hiding this comment

Uh oh!

markov00 commented May 5, 2020

Uh oh!

codecov-commenter commented Jul 25, 2025

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

monfera commented May 1, 2020 •

edited

Loading

codecov-io commented May 1, 2020 •

edited

Loading

monfera May 1, 2020 •

edited

Loading