Treemap text positions for out of range cases when transitioning with maxdepth #4227
Conversation
test/jasmine/tests/treemap_test.js
Outdated
| @@ -1225,7 +1225,7 @@ describe('Test treemap tweening:', function() { | |||
| 'M284.375,188.5L548.375,188.5L548.375,308.5L284.375,308.5Z' | |||
| ); | |||
| _assert('move B text to new position', 'transform', 'B', [220.25126, 0]); | |||
| _assert('enter b text to new position', 'transform', 'b', [287.375195, 5]); | |||
| _assert('enter b text to new position', 'transform', 'b', [284.66071, 35714285714286]); | |||
There was a problem hiding this comment.
That 35714285714286 y-translate coordinate looks odd. Is this node support to go way off the screen?
There was a problem hiding this comment.
@etpinard No that should stay on the screen.
I have no idea why we need that only for the test to pass on the CI.
There was a problem hiding this comment.
I have no idea why we need that only for the test to pass on the CI.
Can you try to find out why?
There was a problem hiding this comment.
... e.g. by first comparing this 35714285714286 value with the before and after transition values
There was a problem hiding this comment.
OK. Let me find out.
There was a problem hiding this comment.
That is related to using one space character instead of a blank string.
With that the test is OK.
There was a problem hiding this comment.
So, what y-translate value do we get if we use a blank string?
There was a problem hiding this comment.
[287.375195, 0] if we use blank. But then the transition doesn't look good.
There was a problem hiding this comment.
Ok - well this is hardly a satisfactory answer, but hey the transition looks better now, so I'll stop complaining.
|
https://codepen.io/MojtabaSamimi/pen/GRKLNLe?editors=0010 looks amazing @archmoj - thanks very much! |
- cast _text to pt for easier debugging - pass pt object to the function instead of separated arguments - do not return in the middle of draw functions - expand void area for blank text
| @@ -1127,7 +1127,7 @@ describe('Test treemap tweening:', function() { | |||
| if(attrName === 'transform') { | |||
| var fake = {attr: function() { return actual; }}; | |||
| var xy = Drawing.getTranslate(fake); | |||
| expect([xy.x, xy.y]).toBeWithinArray(exp, 1, msg2); | |||
| expect([xy.x, xy.y]).toBeWithinArray(exp, 2, msg2); | |||
There was a problem hiding this comment.
Increase range so that the test pass on my machine.
|
The image test would pass if we cherry pick 422d5d3. |
…ring treemap transition
…void header jumps
|
After spending good amount of time on testing with different devices, the transition appears to work smoothly after 6d6f946. |
Setting undefined positions to zeros (made in a1a2a2b in this PR) was not a good idea. That could result in all the text for new points enter from the top left corner of the viewport which was not desirable. Because of that the proper condition checks are added back in 6d6f946. |
Got it. Thanks!! This PR is good to merge 💃 Feel free to cherry-pick 422d5d3 onto this branch and merge it - or wait until #4219 is merged to merge master into this branch. |
|
@etpinard |
|
Now that #4219 is merged, please merge the current master into this branch |
Fixes the jump happening at the end of entering new text elements of
treemapgraph whenmaxdepthis set.In the example below the inner text grows too big during the transition and then returns to its normal on transition end.
Codepen before
Codepen after
@etpinard