Fix uniformtext and enable coloraxis for sunburst and treemap as well as pathbar.textfont by archmoj · Pull Request #4444 · plotly/plotly.js

archmoj · 2019-12-26T03:12:26Z

Resolves#4443 by adding coloraxis to sunburst and treemap
and fixes uniformtext scale issues with react (follow up of #4420):

clear previous uniformtext scale before starting each plot type.
fix uniformtext after switching levels when there is no transition for sunburst and treemap
fix tests and add new tests for uniformtext as well as new coloraxis feature.

Also:

fixespathbar.textfont has no effect in treemap #4451pathbar.textfont bug in 6a4b451
fixesProblem fitting text inside bar-like traces when textangle is set #4456textangle bug in 2a7721c

demo treemap
demo sunburst
demo bar

@plotly/plotly_js

- fix clear previous uniformtext scale before each plot - fix uniformtext after switch level when has no transition for sunburst and treemap - fix tests and add new tests

etpinard · 2019-12-27T19:38:48Z

 };
+
+ // don't clear bar when this is called from waterfall or funnel
+ clearMinTextSize('bar', fullLayout);


Wait. Here you're clearing the min-text-size stash during the plot step. So what's happens when a style-only edit (e.g. Plotly.restyle(gd, 'marker.color', 'red')) gets called?
To me, this logic should probably be somewhere in the calc step.

Restyle works. Considering interactions (e.g. zoom and selection) this should be cleared after calc and at the stat of plot.

Considering interactions (e.g. zoom and selection) this should be cleared after calc and at the stat of plot.
Can you explain a bit more here? To me, sounds like we should not clear minTextSize during zoom interactions.

Or let me rephrase my question: if you do move clearMinTextSize to the calc step, do any of the tests fail?

Yes. In that case the react tests would fail.

Can you share the branch you used to test that?

Summing up a private convo:
I was under the impression that the uniformtext attributes were editType: 'calc', so that's why I thought it would be best to clear the min-text-size during the calc step, but they're not. The uniformtext attributes are editType: 'plot'.
@archmoj says that making the uniformtext attributes editType: 'calc' would have an impact on the way graph with set uniformtext would behave on zoom
Since we'll need to refactor the trace text pipeline at some point (more info in Consistent text mode for bar-like & pie-like traces and feature to control text orientation inside pie/sunburst slices #4420 (comment)), let's keep the clear-min-text logic in the plot step for now.
The most important of this PR are the newly added tests.

etpinard · 2019-12-27T19:39:45Z

-function transition(selection, opts, makeOnCompleteCallback) {
- if(hasTransition(opts)) {
+function transition(selection, fullLayout, opts, makeOnCompleteCallback) {
+ if(!fullLayout.uniformtext.mode && hasTransition(opts)) {


Why? I thought you said this transitions looked fine

They did not apply a uniform text size during and after smooth transition.

…burst and treemap

…textfont - fixing issue 4451

etpinard · 2019-12-30T15:53:40Z


- var font = Lib.ensureUniformFontSize(gd, helpers.determineTextFont(trace, pt, fullLayout.font, trace.pathdir));
+ var font = Lib.ensureUniformFontSize(gd, helpers.determineTextFont(trace, pt, fullLayout.font, {
+ onPathbar: true


Can you explain why this commit made three existing mocks change (treemap_level-depth, treemap_packings and treemap_textposition) ?

Good question.
And a bad mistake here.
There is no such a thing as pathdir!
It should have been written pathbar.

…r positions - also revise code related to autorotate

archmoj · 2020-01-05T23:08:39Z

After a9c9589 this PR is now ready to go.

archmoj · 2020-01-06T17:12:38Z

Here is info related to new/modified baselines:
textangle bug fix (#4456):

(mod) funnel_axis_textangle
(mod) funnel_axis_textangle_start-end
(new) bar-like_textangle45
(new) bar-like_textangle60

treemap trace custom fonts bug fix (#4451)

(new) treemap_fonts_nocolor
(new) treemap_fonts_withcolor
(mod) treemap_packings
(mod) treemap_textposition
(mod) uniformtext_sunburst_treemap

treemap coloraxis with values as well as custom fonts and uniformtext (#4443):

(new) uniformtext_treemap

uniform start/end anchor positions in respect to (#4247):

(mod) niformtext_bar_edgecase2 - text positioned with an identical pad from the top of the bar
(mod) uniformtext_bar_edgecase3 - text positioned with an identical pad from the top of the bar
(new) uniformtext_bar_edgecase4 - text positioned with an identical pad from the bottom of the bar
(new) uniformtext_bar_edgecase5 - text positioned with an identical pad from the bottom of the bar - case of tilted text
(new) uniformtext_bar_edgecase5 - text positioned with an identical pad from the bottom of the bar - case of vertical text on vertical bars
(new) uniformtext_bar_edgecase6 - text positioned with an identical pad from the bottom of the bar - case of horizontal text on horizontal bars
(new) uniformtext_bar_edgecase6 - text positioned with an identical pad from the bottom of the bar - case of tilted text on horizontal bars
(new) uniformtext_bar_axis_textangle_inside - inside text positioned with an identical pad in different axis orientations
(new) uniformtext_bar_axis_textangle_outside - inside text positioned with an identical pad in different axis orientations

modified mock: middle anchors is not used now that the text positions properly by default

(mod) uniformtext_bar-like_10_auto
(mod) uniformtext_bar-like_8_horizontal
(mod) uniformtext_bar-like_8_textangle
(mod) uniformtext_bar-like_8_textangle45

etpinard · 2020-01-06T18:14:02Z

💃 💃

Fixes of uniformtext and enable coloraxis for sunburst and treemap
869b97b
- fix clear previous uniformtext scale before each plot - fix uniformtext after switch level when has no transition for sunburst and treemap - fix tests and add new tests

archmoj added bug something broken feature something new status: reviewable labels Dec 26, 2019

archmoj added this to the v1.52.0 milestone Dec 26, 2019

archmoj added 4 commits December 25, 2019 22:34

no transition with uniformtext for now
b129c5d

only call resizeText not the whole style
c630060

add jasmine tests for sunburst and treemap transition with uniformtext
3737ca9

tweak new test to pass on CI
f6b91e8

etpinard reviewed Dec 27, 2019
View reviewed changes

Comment threadtest/jasmine/tests/sunburst_test.js

etpinard reviewed Dec 27, 2019
View reviewed changes

Comment threadtest/jasmine/tests/treemap_test.js

etpinard reviewed Dec 27, 2019
View reviewed changes

Comment threadsrc/traces/sunburst/defaults.js Outdated

archmoj added 3 commits December 27, 2019 17:05

add comment about special logic to consider values colorscales in sun…
ef6a952
…burst and treemap

make new transition tests more robust
2cf7def

treemap pathbar.textfont should have an effect similar to insidetext.…
6a4b451
…textfont - fixing issue 4451

archmoj changed the title ~~Fix uniformtext and enable coloraxis for sunburst and treemap~~Fix uniformtext and enable coloraxis for sunburst and treemap as well as pathbar.textfont Dec 30, 2019

improve treemap image test with unifromtext and pathbar custom font
16848b3

etpinard reviewed Dec 30, 2019
View reviewed changes

archmoj added 7 commits December 31, 2019 10:39

uniform treemap pathbar text
79ea1cc

textangle bug fix issue 4456
2a7721c

move all uniformtext functions to bar/uniform_text.js
ba1ae6f

declare uniform functions on top of bar plot
599de22

simplify (Inside | Outside) transform logic in bar
0e3e6e2

Merge branch 'master' into more-unifotrm-text
a827bff

keep text pad consistent when uniformtext rescales at start/end ancho…
a9c9589
…r positions - also revise code related to autorotate

etpinard reviewed Jan 6, 2020
View reviewed changes

Comment threadtest/jasmine/tests/bar_test.js

etpinard mentioned this pull request Jan 6, 2020
coloraxes with values for sunburst and treemap #4443
Closed

archmoj merged commit 86af82a into masterJan 6, 2020

archmoj deleted the more-unifotrm-text branch January 6, 2020 18:15

jdamiba mentioned this pull request Jan 9, 2020
1.52 Dash QA #4476
Closed

snyk-bot mentioned this pull request Feb 4, 2020
[Snyk] Upgrade plotly.js from 1.45.3 to 1.52.0 beauremus/ftp-viewer#1
Merged

snyk-bot mentioned this pull request Feb 22, 2020
[Snyk] Upgrade plotly.js from 1.35.2 to 1.52.1 raindigi/eShopOnContainersAI#1
Open

snyk-bot mentioned this pull request Mar 18, 2020
[Snyk] Upgrade plotly.js from 1.52.0 to 1.52.2 beauremus/ftp-viewer#19
Open

snyk-bot mentioned this pull request Apr 17, 2020
[Snyk] Upgrade plotly.js from 1.41.3 to 1.52.3 dunzoit/redash#4
Open

Uh oh!

Uh oh!

Conversation

archmoj commented Dec 26, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

archmoj commented Jan 5, 2020

Uh oh!

Uh oh!

archmoj commented Jan 6, 2020

Uh oh!

etpinard commented Jan 6, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

archmoj commented Dec 26, 2019•
edited
Loading