New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Series fade on legend tap with styled chart on mobile (probably extra mouseover event) #7418

Closed
RainerSchwarze opened this Issue Nov 17, 2017 · 4 comments

Comments

Projects
None yet
3 participants
@RainerSchwarze

RainerSchwarze commented Nov 17, 2017

Summary

When using the styled Highcharts version on a mobile platform, tapping on the legend items sometimes leads to all series faded out. It looks like an extra mouseover occurs, which triggers the hover behaviour for fading out the chart.

Expected behaviour

  • Tap on legend items does not fade series

Actual behaviour

  • Tap on legend items sometimes fades series

Live demo with steps to reproduce

When adding a log call to this snippet belonging to Interaction.js, one can see mouseover output in the console:

		// Set the events on the item group, or in case of useHTML, the item itself (#1249)
		(useHTML ? legendItem : item.legendGroup).on('mouseover', function () {
			console.log("mouseover");
			item.setState('hover');

It looks like mouseover events are sometimes generated on mobile platforms in addition to the touch events. (As a quick test, I inserted if (hasTouch) return; and this "solved" the problem. Not sure, if this is appropriate.) A quick search lead me to this: https://stackoverflow.com/q/41625763/1396265, indicating it is a browser bug? (I did not dig any deeper so far)

Affected browser(s)

  • tested in Chrome 62 responsive mode
  • tested in iOS11 Safari (iPhone 6 plus)
@pawelfus

This comment has been minimized.

Show comment
Hide comment
@pawelfus

pawelfus Nov 20, 2017

Contributor

Hi @RainerSchwarze

Thank you for reporting this issue. I can see how series fade out/in on Chrome+responsivne, tested also:

  • Android 6 + Chrome - works fine
  • iOS v11.1.2 + Safari - almost every tap on "Manufacturing" tex causes the issue
Contributor

pawelfus commented Nov 20, 2017

Hi @RainerSchwarze

Thank you for reporting this issue. I can see how series fade out/in on Chrome+responsivne, tested also:

  • Android 6 + Chrome - works fine
  • iOS v11.1.2 + Safari - almost every tap on "Manufacturing" tex causes the issue

@pawelfus pawelfus added the Bug label Nov 20, 2017

@RainerSchwarze

This comment has been minimized.

Show comment
Hide comment
@RainerSchwarze

RainerSchwarze Nov 29, 2017

Hi @pawelfus,

I was looking around on how to "fix" this. I noticed, that in the Pointer module in setDOMEvents one could cancel the mouseover if it seems to be related to a touchstart. I did not yet test this in more detail. Something like this:

...
if (H.hasTouch) {
    // inserted code:
    container.addEventListener('touchstart', function(e) {
        this.xTouchEvent = {
            scrX: e.changedTouches[0].screenX,
            scrY: e.changedTouches[0].screenY,
            scrTS: e.timeStamp,
        };
    });
    container.addEventListener('mouseover', function(e) {
        var t = this.xTouchEvent;
        if (t) {
            if (
                Math.abs(e.screenX - t.scrX) <= 2 &&
                Math.abs(e.screenY - t.scrY) <= 2 &&
                Math.abs(e.timeStamp - t.scrTS) <= 200
            ) {
                console.log("stopped"); // test for me
                e.stopPropagation();
            }
        }
    }, true);
    ...

What do you think about this? Would you already know of any problems this would cause?

I'm not yet sure about the thresholds (need more testing) and I did not yet test it in iOS Safari.
It seems to fix the problem in Chrome+responsive.

(While testing I noticed, that a very reliable way to reproduce the issue, is to tap right between two legend items.)

RainerSchwarze commented Nov 29, 2017

Hi @pawelfus,

I was looking around on how to "fix" this. I noticed, that in the Pointer module in setDOMEvents one could cancel the mouseover if it seems to be related to a touchstart. I did not yet test this in more detail. Something like this:

...
if (H.hasTouch) {
    // inserted code:
    container.addEventListener('touchstart', function(e) {
        this.xTouchEvent = {
            scrX: e.changedTouches[0].screenX,
            scrY: e.changedTouches[0].screenY,
            scrTS: e.timeStamp,
        };
    });
    container.addEventListener('mouseover', function(e) {
        var t = this.xTouchEvent;
        if (t) {
            if (
                Math.abs(e.screenX - t.scrX) <= 2 &&
                Math.abs(e.screenY - t.scrY) <= 2 &&
                Math.abs(e.timeStamp - t.scrTS) <= 200
            ) {
                console.log("stopped"); // test for me
                e.stopPropagation();
            }
        }
    }, true);
    ...

What do you think about this? Would you already know of any problems this would cause?

I'm not yet sure about the thresholds (need more testing) and I did not yet test it in iOS Safari.
It seems to fix the problem in Chrome+responsive.

(While testing I noticed, that a very reliable way to reproduce the issue, is to tap right between two legend items.)

@TorsteinHonsi

This comment has been minimized.

Show comment
Hide comment
@TorsteinHonsi

TorsteinHonsi Dec 8, 2017

Collaborator

issue7418

Collaborator

TorsteinHonsi commented Dec 8, 2017

issue7418

@TorsteinHonsi

This comment has been minimized.

Show comment
Hide comment
@TorsteinHonsi

TorsteinHonsi Dec 8, 2017

Collaborator

I fixed this simply by removing the active class name in the click event. Your fix looks valid too @RainerSchwarze, but I'm afraid it may break other functionality in unforeseen places.

Collaborator

TorsteinHonsi commented Dec 8, 2017

I fixed this simply by removing the active class name in the click event. Your fix looks valid too @RainerSchwarze, but I'm afraid it may break other functionality in unforeseen places.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment