Issue 3801 - legend scrolling on mobile

[andrewbaldock]

Potential fix for issue #3801

• Updates scrollbar drag listener so it scrolls when dragged via mouse or touch
• Adds scrollbox touch listener so the box natural-scrolls when legend is dragged via touch

Not working example, from issue 3801:
https://codepen.io/anon/pen/MRqOgE

Working example:
https://codepen.io/anon/pen/yWggJM

[etpinard]

Thanks very much for this PR!

Legend scroll on touch works fine in https://codepen.io/anon/pen/yWggJM for me (tested on mobile FF 66)

Now, it seems that we can no longer scroll the page on touch, like if the legend touch handlers took over the whole page. I'm sure why that is, can anyone else check by trying to scroll by touching in the graph's left margin?

[andrewbaldock]

Hi, no problem, we love Plotly!

I should have mentioned that in the fixed Codepen, in the CSS section I set html { overflow: hidden } to better demonstrate the scrolling in the legend -- remove that CSS in the Codepen and you should see it work as you expect.

Another possible issue - a coworker pointed out that touch-dragging the legend: going in for a second touch to continue scrolling can cause the legend to jump... I was going to look at that today.

[andrewbaldock]

My coworker was right (going in for a second touch to continue scrolling can cause the legend to jump), I was mistakenly inverting the initial touch when there was no need.

I have force-pushed this change to fix it:
https://github.com/plotly/plotly.js/compare/e155a8b658d3f17b0f87e91ea3cf25b039b7cf2b..26ea3bc454b4f4d9060d028983a9f3956041695e

The 'working' codepen has been updated: https://codepen.io/anon/pen/yWggJM

Now I think this PR is ready for review, thanks very much.

[oshikryu]

👍 This works great!

[andrewbaldock]

Thanks @oshikryu!

However I was still a bit optimistic, I could still see a little jump sometimes.

With the last commit above, the math is finally correct for the natural scrolling, and I can no longer reproduce the jump. Working codepen also updated for this change.

Thanks @etpinard for your patience, I’m done now. I didn’t squash this last commit so you could see the evolution more easily, let me know if you’d like me to.

[etpinard]

Thanks @andrewbaldock @oshikryu

The behavior off 53fa7f3 feels good to me.

Now, would you be interested in writing a test similar to

plotly.js/test/jasmine/tests/legend_scroll_test.js

Lines 152 to 160 in 53fa7f3

	it('should scroll on dragging the scrollbar', function() {
	var finalDataScroll = dragScroll(getScrollBar());
	var scrollBox = getScrollBox();
	var dataScroll = getScroll(gd);
	expect(dataScroll).toBeCloseTo(finalDataScroll, 3);
	expect(scrollBox.getAttribute('transform')).toBe(
	'translate(0, ' + -dataScroll + ')');
	});

but for touch drag? Using our touchEvent wrapper and looking at how the select suite emulates touch should help you get started.

[andrewbaldock]

Will do! Just got buried at work, I will try to get it to you by early next week.

[dan8f]

https://codepen.io/anon/pen/yWggJM

Seems to cause double scrolling (legend and page), when touch scrolling the legend?

[andrewbaldock]

@dan8f
I think the code for the legend should probably not have any power of the overflow/scroll settings of the containing elements for the plot... overflow at the outer levels should be managed by setting overflow:hidden on document.body ...or wherever best suited.

[andrewbaldock]

Update on unit test -

Sorry this took so long to get back to. I just spent over a day looking at this, but frustrating to report, I could never get a new user test using 'touchEvent' to work correctly on the legend. Here's what I experienced.

By console logging out scrollBoxTouchDrag in components/legend/draw.js, I see the following behavior:

---

GOOD: d3.behavior.drag() is successfully responding to touchstart AND touchmove when they are generated by an actual user... say on a phone, or using Chrome iPad simulator.

---

BAD: d3.behavior.drag() handles ONLY 'touchstart' when the events are generated by a test runner using dispatchEvent on the scrollBox. The 'touchmove' and 'touchend' sent by dispatchEvent onto the scrollBox are never seen by d3.behavior.drag when testing.

Ideas I tried -

• I looked at the d3 docs and experimented with d3.event.sourceEvent.stopPropagation
• I tried using touchDrag found in another test, and tried using jasmine/assets/drag with opts.touch===true (of course these both rely on jasmine/assets/touchEvent, so it's the same issue).
• I tried creating touchEvent handler that's not d3.
• I thought about upgrading d3, but too many breaking changes to support

Sorry to report a swing and a miss. If you see something that eluded me, please let me know.

[etpinard]

@andrewbaldock thanks for your reply!

We should able to take this from here. We're planning on making a few legend improvements before releasing v1.50.0; we'll make sure to incorporate your work and find a way to test it.

Thanks again!

[etpinard]

Looks like I can't push commits to this branch. So I'll merge this PR now and attempt to add a test on a branch up-to-date with the latest legend work.