feat(partition): linked text overflow avoidance by monfera · Pull Request #670 · elastic/elastic-charts

monfera · 2020-05-07T18:03:00Z

Summary

Truncates linked label text to fit in the chart box (WIP)

Example with off-center pie chart to avoid centered special case:

It culls a link label altogether if

its text has no horizontal room left
the text would overflow the rectangular area vertically; eg. no linked label for the green Food... slice:

Closes #633

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

…rder

monfera · 2020-05-11T08:29:30Z

Removed the wip label as it has the vertical space feature as well.
The image tests capture the desired effect to a greater extent than unit tests could. I also have plans to revisit it for improvements, eg. more accurate maximalization of horizontal space.

The unit testing plan for a subsequent PR is the following:

extract out and maybe rename the maximizer function which is basically a greedy search, gradient descent function
switch to this function for the determination of font sizes for fill text, which currently iterates through the font sizes linearly, and not yet modular anyway
the optimizer thus becomes exported, add unit tests for it

codecov-io · 2020-05-11T08:45:19Z

Codecov Report

Merging #670 into master will decrease coverage by 0.37%.
The diff coverage is 6.89%.

@@            Coverage Diff             @@
##           master     #670      +/-   ##
==========================================
- Coverage   72.97%   72.60%   -0.38%     
==========================================
  Files         266      266              
  Lines        8634     8678      +44     
  Branches     1698     1710      +12     
==========================================
  Hits         6301     6301              
- Misses       2294     2338      +44     
  Partials       39       39

Impacted Files	Coverage Δ
...hart_types/partition_chart/layout/config/config.ts	`68.75% <ø> (ø)`
...types/partition_chart/layout/types/config_types.ts	`100.00% <ø> (ø)`
...es/partition_chart/layout/types/viewmodel_types.ts	`80.00% <ø> (ø)`
...rtition_chart/layout/viewmodel/link_text_layout.ts	`10.12% <0.00%> (-8.93%)`	⬇️
src/utils/data_generators/simple_noise.ts	`100.00% <ø> (ø)`
src/utils/data_generators/data_generator.ts	`44.44% <22.22%> (-10.73%)`	⬇️
...ypes/partition_chart/layout/viewmodel/viewmodel.ts	`89.23% <100.00%> (ø)`
src/mocks/utils.ts	`94.11% <100.00%> (ø)`
... and 2 more

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 a79e899...2f21442. Read the comment docs.

markov00

LGTM, I've added a comment to include the TSDoc for the monotonicMaximizer function as it's a little bit difficult to understand looking just at the function signature.

markov00 · 2020-05-19T09:41:18Z

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

+    .filter((l: LinkLabelVM) => l.text !== ''); // cull linked labels whose text was truncated to nothing
+}
+
+function monotonicMaximizer(


nit: Could you please add a jsdoc describing what this function does?
It's a little cryptic and I've difficulties understanding what is going on there

Please, also add the return type

Thanks Marco, will do, the upcoming PR deals with and extracts out this function

nickofthyme

Code and functionality all LGTM

codecov-commenter · 2020-05-19T19:47:55Z

Codecov Report

Merging #670 into master will decrease coverage by 0.37%.
The diff coverage is 6.89%.

@@            Coverage Diff             @@
##           master     #670      +/-   ##
==========================================
- Coverage   72.97%   72.60%   -0.38%     
==========================================
  Files         266      266              
  Lines        8634     8678      +44     
  Branches     1698     1710      +12     
==========================================
  Hits         6301     6301              
- Misses       2294     2338      +44     
  Partials       39       39

Impacted Files	Coverage Δ
...hart_types/partition_chart/layout/config/config.ts	`68.75% <ø> (ø)`
...types/partition_chart/layout/types/config_types.ts	`100.00% <ø> (ø)`
...es/partition_chart/layout/types/viewmodel_types.ts	`80.00% <ø> (ø)`
...rtition_chart/layout/viewmodel/link_text_layout.ts	`10.12% <0.00%> (-8.93%)`	⬇️
src/utils/data_generators/simple_noise.ts	`100.00% <ø> (ø)`
src/utils/data_generators/data_generator.ts	`44.44% <22.22%> (-10.73%)`	⬇️
...ypes/partition_chart/layout/viewmodel/viewmodel.ts	`89.23% <100.00%> (ø)`
src/mocks/utils.ts	`94.11% <100.00%> (ø)`
... and 2 more

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 a79e899...2f21442. Read the comment docs.

# [19.4.0](v19.3.0...v19.4.0) (2020-05-28) ### Bug Fixes * **partition:** consider legendMaxDepth on legend size ([#654](#654)) ([9429dcf](9429dcf)), closes [#639](#639) ### Features * **partition:** enable grooves in all group layers ([#666](#666)) ([f5b4767](f5b4767)) * **partition:** linked text overflow avoidance ([#670](#670)) ([b6e5911](b6e5911)), closes [#633](#633) * **partition:** monotonic font size scaling ([#681](#681)) ([ea2489b](ea2489b)), closes [#661](#661) * **tooltip:** improve positioning with popperjs ([#651](#651)) ([6512950](6512950)), closes [#596](#596)

markov00 · 2020-05-28T14:32:29Z

🎉 This PR is included in version 19.4.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

monfera added 2 commits May 7, 2020 19:51

feat: avoid linked label text overflow

b3a7fbd

test: avoid linked label text overflow

1501d92

monfera added enhancement New feature or request wip work in progress :partition Partition/PieChart/Donut/Sunburst/Treemap chart related labels May 7, 2020

monfera self-assigned this May 7, 2020

monfera added 6 commits May 8, 2020 12:09

chore: adding a 1em padding so text is aesthetically away from the bo…

d4b3c2c

…rder

chore: type simplification

e888541

refactor: type extraction

9f0d476

chore: geometry types internal

135d064

feat: cull link labels with fully truncated text

ac3aa46

feat: cull link label if there's no vertical space

aecc2a4

monfera requested review from markov00, rshen91 and wylieconlon May 11, 2020 08:22

Merge branch 'master' into partition-linked-text-overflow-squashed

2f21442

monfera removed the wip work in progress label May 11, 2020

markov00 reviewed May 19, 2020

View reviewed changes

markov00 approved these changes May 19, 2020

View reviewed changes

nickofthyme approved these changes May 19, 2020

View reviewed changes

monfera merged commit b6e5911 into elastic:master May 19, 2020

monfera deleted the partition-linked-text-overflow-squashed branch May 19, 2020 19:50

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

markov00 mentioned this pull request Jun 15, 2020

Update dependency @elastic/charts to v19.5.2 elastic/kibana#69126

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(partition): linked text overflow avoidance#670

feat(partition): linked text overflow avoidance#670
monfera merged 9 commits intoelastic:masterfrom
monfera:partition-linked-text-overflow-squashed

monfera commented May 7, 2020 •

edited

Loading

Uh oh!

monfera commented May 11, 2020

Uh oh!

codecov-io commented May 11, 2020

Uh oh!

markov00 left a comment

Uh oh!

markov00 May 19, 2020

Uh oh!

markov00 May 19, 2020

Uh oh!

monfera May 19, 2020

Uh oh!

nickofthyme left a comment

Uh oh!

codecov-commenter commented May 19, 2020

Uh oh!

markov00 commented May 28, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

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

Uh oh!

Summary

Checklist

Uh oh!

monfera commented May 11, 2020

Uh oh!

codecov-io commented May 11, 2020

Codecov Report

Uh oh!

markov00 left a comment

Choose a reason for hiding this comment

Uh oh!

markov00 May 19, 2020

Choose a reason for hiding this comment

Uh oh!

markov00 May 19, 2020

Choose a reason for hiding this comment

Uh oh!

monfera May 19, 2020

Choose a reason for hiding this comment

Uh oh!

nickofthyme left a comment

Choose a reason for hiding this comment

Uh oh!

codecov-commenter commented May 19, 2020

Codecov Report

Uh oh!

markov00 commented May 28, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

monfera commented May 7, 2020 •

edited

Loading