fix(partition): rtl text label support #1433

nickofthyme · 2021-10-18T21:08:49Z

Summary

Better support RTL text value in partition and xy charts. Current support is automatically applied given the label text characters.

Screen.Recording.2021-10-21.at.04.10.52.PM.mp4

Details

Partition charts parse the words individually by performing .split(' ') on the string to obtain the words. Thus the global canvas dir attribute alone will not fix the issue.

Add support for horizontal alignment of fill labels in partition charts, namely sunburst and treemap. Currently hardcoded as treemap && RTL->'right', treemap && not RTL->'left', sunburst->'center'. We could expose this to the user later.
Add RTL support for link labels in partition charts, namely sunburst and treemap.
Provide better styling support for RTL, specifically related to legend and tooltip.

Styles for DOM elements, not canvas, are mostly handled by default by just setting the dir or direction on an element, with a few exceptions. These exceptions are handled with css overrides for [dir=rtl].

The only visual change in this PR is that the formatted value string in fill labels used to be split to allow for better fitting. This caused issues related to RTL strings so I removed this and now the value strings act as a single string.

This PR adds pretty good support for rtl text in partition and xy charts and applies the global dir setting on the canvas element for both based on .

Missing support

RTL support for other charts (i.e. heatmap, goal, etc.)
RTL support for annotation tooltips
Consider using DOM elements to better render directional text labels.
Missing support for rotations of fill and link labels. I don't see any usage of this in kibana, or out own stories just yet.
Tick values should force direction when formatted. Now if using rtl strings formatted values will show in reverse.
Consensus on how to support value interpolation. Now just forcing all value string labels to ltr.

Issues

fix #1432

Checklist

The proper chart type label has been added (e.g. :xy, :partition)
All related issues have been linked (i.e. closes #123, fixes #123)
Unit tests have been added or updated to match the most common scenarios
The proper documentation and/or storybook story has been added or updated
The code has been checked for cross-browser compatibility (Chrome, Firefox, Safari, Edge)

markov00

It looks good, but I can't understand the language used here and I can't say if that is correct or not.
@ghudgins do you know someone that knows an RTL language to confirm our tests are valid?
The only think I see is that the RTL text in the pie starts at the bottom, I'm not sure but RTL text is not automatically a bottom to top language.

packages/charts/src/utils/common.ts

nickofthyme · 2021-10-19T15:12:40Z

@markov00 what are your thought about adding a way to set the dir attribute on the canvas? Should we attempt to set this from the data using isRTL?

UPDATE

Per zoom chat, we assign global canvas dir based on majority of strings.

- Add better alignment support for partition fill and link labels - Better support for mixed text with force ctx.direction - Improve legend and tooltip styles for rtl strings and mixed

nickofthyme

I still need to update the unit and VRT's with latest changes.

packages/charts/src/components/legend/legend_item.tsx

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

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

packages/charts/src/chart_types/partition_chart/renderer/canvas/canvas_renderers.ts

packages/charts/src/components/legend/_legend_item.scss

packages/charts/src/components/legend/label.tsx

packages/charts/src/components/tooltip/_tooltip.scss

nickofthyme · 2021-11-03T18:40:19Z

@elasticmachine merge upstream

packages/charts/src/chart_types/xy_chart/renderer/canvas/utils/has_mostly_rtl.ts

packages/charts/src/common/text_utils.ts

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

markov00

Changes looks good to me, tested locally and works great!

packages/charts/src/chart_types/partition_chart/renderer/canvas/canvas_renderers.ts

monfera

LGTM, great addition! Thanks for adopting the small code suggestions and the addition of more RTL languages 👍

nickofthyme · 2021-11-09T15:40:30Z

Added logic to xy_chart tick labels to treat all non-ordinal labels as ltr text and ordinal tick depending on the label characters. Now formatted numerical values with pre/postfixes are correctly rendered.

## [39.0.1](v39.0.0...v39.0.1) (2021-11-15) ### Bug Fixes * **partition:** rtl text label support ([#1433](#1433)) ([01bbe3a](01bbe3a))

support rtl text labels in xy and partition charts, including mixed ltr/rtl. # Conflicts: # packages/charts/src/chart_types/heatmap/layout/viewmodel/viewmodel.ts # packages/charts/src/chart_types/xy_chart/renderer/canvas/axes/tick_label.ts # packages/charts/src/chart_types/xy_chart/state/selectors/visible_ticks.ts # packages/charts/src/chart_types/xy_chart/utils/axis_utils.test.ts # packages/charts/src/chart_types/xy_chart/utils/axis_utils.ts

## [38.0.4](v38.0.3...v38.0.4) (2021-11-15) ### Bug Fixes * **partition:** rtl text label support ([#1433](#1433)) ([#1478](#1478)) ([5d5c186](5d5c186))

feat(partition): rtl text labels

c58b4af

nickofthyme added bug Something isn't working :partition Partition/PieChart/Donut/Sunburst/Treemap chart related labels Oct 18, 2021

nickofthyme requested review from markov00 and monfera October 18, 2021 21:08

nickofthyme added 2 commits October 18, 2021 16:53

fix: add better styles and fix ellipsis position

f4cfa22

test: update vrt screenshot cases

bd024e4

nickofthyme marked this pull request as ready for review October 18, 2021 22:05

markov00 reviewed Oct 19, 2021

View reviewed changes

packages/charts/src/utils/common.ts Outdated Show resolved Hide resolved

nickofthyme added 2 commits October 21, 2021 15:52

refactor: improve rtl logic and styles

3ce674f

- Add better alignment support for partition fill and link labels - Better support for mixed text with force ctx.direction - Improve legend and tooltip styles for rtl strings and mixed

fix: support for nested legends

78b5a21

nickofthyme force-pushed the rtl-text branch from 5d6ec15 to 78b5a21 Compare October 21, 2021 21:29

nickofthyme commented Oct 21, 2021

View reviewed changes

test: update vrt cases and screenshots

e19c25d

nickofthyme requested review from markov00 and ghudgins October 21, 2021 22:38

nickofthyme added 6 commits October 21, 2021 17:39

chore: fix prettier errors

b5ea4f1

test: update changes jest snapshots

f849736

chore: update unexpected vrt diffs

d4c0099

fix: add rtl logic to xy chart renderer

8e42fac

chore: update brocken vrt screenshots

0d421a7

test: update jest snapshot

27888cb

nickofthyme added the :xy Bar/Line/Area chart related label Oct 25, 2021

markov00 mentioned this pull request Nov 1, 2021

RTL support improvements #1450

Open

5 tasks

markov00 requested a review from rshen91 November 1, 2021 15:59

Merge branch 'master' into rtl-text

e29b61c

monfera reviewed Nov 3, 2021

View reviewed changes

packages/charts/src/chart_types/xy_chart/renderer/canvas/utils/has_mostly_rtl.ts Show resolved Hide resolved

monfera reviewed Nov 3, 2021

View reviewed changes

packages/charts/src/common/text_utils.ts Outdated Show resolved Hide resolved

monfera reviewed Nov 3, 2021

View reviewed changes

packages/charts/src/chart_types/partition_chart/layout/types/viewmodel_types.ts Outdated Show resolved Hide resolved

markov00 approved these changes Nov 5, 2021

View reviewed changes

packages/charts/src/chart_types/partition_chart/renderer/canvas/canvas_renderers.ts Outdated Show resolved Hide resolved

nickofthyme added 2 commits November 5, 2021 15:17

Merge branch 'master' into rtl-text

7f26022

refactor: address PR comments

974db6b

nickofthyme requested a review from monfera November 5, 2021 21:25

nickofthyme added 2 commits November 5, 2021 16:43

chore: udpate vrt and story with multiple rtl languages

27fca75

refactor: DRY up crx logic

34e9bae

nickofthyme requested review from monfera and removed request for monfera and rshen91 November 6, 2021 17:49

monfera approved these changes Nov 8, 2021

View reviewed changes

nickofthyme added 3 commits November 8, 2021 17:02

fix: formatted non-ordinal tick label direction

d0404c7

Merge branch 'master' into rtl-text

a576b46

test: fix tests

577cd64

elastic deleted a comment from elasticmachine Nov 9, 2021

nickofthyme added 2 commits November 9, 2021 08:48

Merge branch 'master' into rtl-text

1aee260

chore: update vrt screenshots

f453e7b

nickofthyme merged commit 01bbe3a into elastic:master Nov 15, 2021

nickofthyme deleted the rtl-text branch November 15, 2021 15:29

nickofthyme changed the title ~~fix(partition): rtl text labels~~ fix(partition): rtl text label support Nov 15, 2021

nickofthyme pushed a commit that referenced this pull request Nov 15, 2021

chore(release): 39.0.1 [skip ci]

ea73780

## [39.0.1](v39.0.0...v39.0.1) (2021-11-15) ### Bug Fixes * **partition:** rtl text label support ([#1433](#1433)) ([01bbe3a](01bbe3a))

nickofthyme mentioned this pull request Nov 15, 2021

fix(partition): rtl text label support (#1433) [38.0.x] #1478

Merged

nickofthyme added a commit that referenced this pull request Nov 15, 2021

fix(partition): rtl text label support (#1433) (#1478)

5d5c186

nickofthyme pushed a commit that referenced this pull request Nov 15, 2021

chore(release): 38.0.4 [skip ci]

ea3515f

## [38.0.4](v38.0.3...v38.0.4) (2021-11-15) ### Bug Fixes * **partition:** rtl text label support ([#1433](#1433)) ([#1478](#1478)) ([5d5c186](5d5c186))

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(partition): rtl text label support #1433

fix(partition): rtl text label support #1433

nickofthyme commented Oct 18, 2021 •

edited

Loading

markov00 left a comment •

edited

Loading

nickofthyme commented Oct 19, 2021 •

edited

Loading

nickofthyme left a comment

nickofthyme commented Nov 3, 2021

markov00 left a comment

monfera left a comment

nickofthyme commented Nov 9, 2021

fix(partition): rtl text label support #1433

fix(partition): rtl text label support #1433

Conversation

nickofthyme commented Oct 18, 2021 • edited Loading

Summary

Details

Missing support

Issues

Checklist

markov00 left a comment • edited Loading

Choose a reason for hiding this comment

nickofthyme commented Oct 19, 2021 • edited Loading

UPDATE

nickofthyme left a comment

Choose a reason for hiding this comment

nickofthyme commented Nov 3, 2021

markov00 left a comment

Choose a reason for hiding this comment

monfera left a comment

Choose a reason for hiding this comment

nickofthyme commented Nov 9, 2021

nickofthyme commented Oct 18, 2021 •

edited

Loading

markov00 left a comment •

edited

Loading

nickofthyme commented Oct 19, 2021 •

edited

Loading