Skip to content
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

Tooltips don't work on Windows #967

Closed
maxwondercorn opened this issue Jul 5, 2019 · 12 comments
Closed

Tooltips don't work on Windows #967

maxwondercorn opened this issue Jul 5, 2019 · 12 comments
Labels

Comments

@maxwondercorn
Copy link

maxwondercorn commented Jul 5, 2019

Description

Tooltips are not working on Windows - Chrome, Edge, and IE11. Can not test on Firefox. I develop on OSX where there isn't an issue.

All other features seem to be working correctly, including a data.onclick event handler in the application.

Steps to check or reproduce

I discovered the issue when deploying a new application but the issue also presents in the billboard examples and playground. I haven't found any yet that work correctly.

OS is Win10 on company computers. While they are pretty locked down, I can't image any security settings that would break this and on three different browsers.

Edited - I have confirmed tool tips aren't working on a non-work computer, so this is a general issues with Billboard on Win10.

@netil netil added the question label Jul 8, 2019
@netil
Copy link
Member

netil commented Jul 8, 2019

Hi @maxwondercorn, thanks for the report.

My primary work environment is Win10, so basically if any issue was noticed, I will be the first one knowing that.

As you can see from the screenshot(Left - IE11, Right - IE Edge(Not the chromium based)), I did test on example page, but tooltip works as expected.
tooltip-win10

If somebody experiencing same isssue, plz leave comments.

@maxwondercorn
Copy link
Author

maxwondercorn commented Jul 8, 2019

I can not generate gifs but screenshots below show chrome, firefox and edge (not chromium) on Win10. IE also doesn't show the tooltips.

The mouseover events are not being triggered. If I try the data.onover function in the playground it will only work with Firefox. No issues on MacOS.

screenshots

Edit: Mousing over the legend does work correctly for all browsers

@netil
Copy link
Member

netil commented Jul 8, 2019

My test environment are as follows and all worked well.

  • Win 10: Version 10.0.17134.829
    • Chrome 75.0.3770.100
    • IE 11.829.17134.0
    • Microsoft Edge:
      • Microsoft EdgeHTML 17.17134 (the non chromium)
      • Microsoft Edge Version 77.0.197.1 (chromium based)
    • Firefox 67.0.4

@maxwondercorn, try test below link and checkout if behaves as follows. If you get different result, it means the events for tooltip aren't correctly set.

1) check if onover/onout callbacks are triggered

I set callbacks with onover and onout option, which will print log on mouseover and out.
tooltip-win10-02

2) check if mouseover event set correctly to the <rect> elements

At the initialization, will print out the mouseover event handler function as string(6 times) bound to the event rect elements.

image

@maxwondercorn
Copy link
Author

I tried the test program with the same results. The in, over and out events were only logged with FireFox. I've tested on both Win10 Enterprise and Home. There were two different Edge versions used (44.17763 and 42.17134.10). Same results for Chrome 75.

The test program wouldn't run on IE but I'm sure it wouldn't have worked if it could have run.

Independently, I experimented with different versions of d3 with the current billboard.js version. I wanted to determine if a specific d3 version might work. I tried several 5.x versions and even dropped back to v4.13.0. They all displayed graphs but none would show tooltips on hover.

Not sure where to go from here

@netil
Copy link
Member

netil commented Jul 12, 2019

@maxwondercorn, thanks for the check.
I'm not sure either if this caused by d3's event attach or is from the billboard.js itself.

For tooltip implementation, there's nothing special on it. Just attaching mouse related events on the elements. If nothing logged, it means there's no event attached on it.

There will be some reason on it, but I can't determine what will be the cause until is reproducible.

Anyway, I'll be trying testing as many as possible for different environments.
Meanwhile if you find any clues, let me know.

@maxwondercorn
Copy link
Author

I spent more time digging into the issue. As suspected the default event listeners aren't being added. In Firefox all four event listeners are added, in Chrome only the click listener is added and Edge doesn't have any default listeners. See screen shot links below.

To be honest I don't understand where the default event listeners are being setup in the logic. I tried to put in a few debugger statements to catch the problem but no luck. I can see where/how custom events are added.

Firefox - https://github.com/maxwondercorn/bb-test/blob/master/Firefox%20-%20events.png

Chrome - https://github.com/maxwondercorn/bb-test/blob/master/chrome%20events.png

Edge - https://github.com/maxwondercorn/bb-test/blob/master/edge%20events.png

@netil
Copy link
Member

netil commented Jul 15, 2019

@maxwondercorn, can you do favor on checking below steps?

1

var chart = bb.generate({ ... });

// what value prints?
// in normal circumstance, it should print "mouse"
chart.internal.inputType;

2

if the above #1 code returns touch, try generating chart with the above option.

var chart = bb.generate({
	...

	interaction: {
		inputType: {
			mouse: true
		}
	  }
});

// what value prints?
chart.internal.inputType;

Possible cause

If #1 and #2 returned touch, it means is treated as touch support environment.

Internally, there're code to evaluate user's environment to determine mouse or touch event binding.

One of the condition to be determined as mouse, it shouldn't have touch capability. But I'm suspicious that there're laptops or desktops with screen touch capabilities.

// checkout if is evaluated as 'true' from each browser's devtool console.
window.navigator && "maxTouchPoints" in window.navigator && window.navigator.maxTouchPoints > 0;

In that case, there will be an issue to bind mouse related events.

@ajuvonen
Copy link

Hello, we are also experiencing this behavior. Could it be related to window.navigator.maxTouchPoints returning > 0 on EDGE with some configurations (possibly a laptop with a multi-touch capable touchpad or a touchscreen)?
https://developer.microsoft.com/en-us/microsoft-edge/platform/issues/20417074/

@maxwondercorn
Copy link
Author

I also found that issue yesterday. As @netil surmised it is an issue with touchscreens and laptops. If the touch screen is enabled it will register as a touch device based on the detection logic in convertInputType().

I see maxTouchPoints = 10 with touch screens enabled. I have only Dell computers to test on so the behavior may be different on others. In browsers other than Edge, if the touchscreen is disabled maxTouchPoints = 0. On Edge it's 2 as indicated in the bug report.

Interesting that Firefox has maxTouchPoints = 0 even if the touchscreen is enabled. Touch and mouse event's both work on Firefox so Windows is managing the inputs correctly.

To answer @netil questions, both test 1 & 2 return null on Edge and Chrome, mouse on Firefox. Checking the conditions logic

  • Edge = true
  • Chrome = true
  • Firefox = false

I'm experimenting a bit with the detection logic to see if this can be overcome. The problem is fixing the issue may then break it on mobile devices. maxTouchPoints is the recommended method. Some method use userAgent which is not recommended

@maxwondercorn
Copy link
Author

A potenital fix would be to change hasMouse to:

const hasMouse = config.interaction_inputType_mouse || !isMobile ? ("onmouseover" in window) : false;

This change along with using interaction fixes the issue.

interaction: {
  inputType: {
    mouse: true
  }
}

I did some limited testing with the touch screen enabled/disabled and on mobile Safari. Touch and the mouse still worked as expected. I don't do mobile so there may be some issues/unknown side affects that I'm not aware of.

@netil
Copy link
Member

netil commented Jul 16, 2019

Thanks for the investigation @maxwondercorn @ajuvonen.

As you noticed, there's no clear detection for desktop or mobile. Some browser(like Edge) return buggy value and for mobile browsers, they usually support mouse events as well.

So, how to determine the "correct" value on this? I'm against using browser agent sniffing, but in this case that could be a simple solution.

I'll be updating the detection code as follows. Try with the nightly when is built.

convertInputType() {
	const $$ = this;
	const config = $$.config;
	let isMobile = false;

	if (/Mobi/.test(window.navigator.userAgent) && config.interaction_inputType_touch) {
		const hasTouchPoints = window.navigator && "maxTouchPoints" in window.navigator && window.navigator.maxTouchPoints > 0;
		const hasTouch = ("ontouchmove" in window || (window.DocumentTouch && document instanceof window.DocumentTouch));

		isMobile = hasTouchPoints || hasTouch;
	}

	const hasMouse = config.interaction_inputType_mouse && !isMobile ? ("onmouseover" in window) : false;

	return (hasMouse && "mouse") || (isMobile && "touch") || null;
}

Ref.

netil added a commit that referenced this issue Jul 16, 2019
Add browser sniffing to determine 'mobile' touch to be distinguished
with the desktop with the touch support.

Ref #967
@maxwondercorn
Copy link
Author

The changes to the mouse detect fixed the problem. It's working correctly on Edge, Chrome and IE.

@netil netil closed this as completed Jul 22, 2019
netil pushed a commit that referenced this issue Aug 2, 2019
# [1.10.0-next.1](https://github.com/naver/billboard.js/compare/1.9.5...1.10.0-next.1@next) (2019-08-02)

### Bug Fixes

* **chart:** Correct the order to set '$' node values ([b97558c](b97558c)), closes [#994](#994)
* **event:** Update determination condition ([736ba56](736ba56)), closes [#967](#967)
* **radar:** Correct display of indexed axis ([9bac296](9bac296)), closes [#997](#997)
* **text:** Correct text vertical align ([6debb55](6debb55)), closes [#982](#982)
* **tooltip:** Correct tooltip on dynamic loading ([c24bddb](c24bddb)), closes [#963](#963)
* **tooltip:** Fix on contents template ([419144f](419144f)), closes [#972](#972)

### Features

* **axis:** Intent to ship y/y2 axis culling ([44c6c4c](44c6c4c)), closes [#915](#915)
* **bubble:** Intent to ship dimension ([27df7c3](27df7c3)), closes [#484](#484)
* **options:** Pass instance arg to callbacks ([61cf047](61cf047)), closes [#989](#989)
* **radar:** Intent to ship axis.text.position ([1720ec2](1720ec2)), closes [#998](#998)
netil pushed a commit that referenced this issue Aug 7, 2019
# [1.10.0](1.9.5...1.10.0) (2019-08-07)

### Bug Fixes

* **axis:** Correct label text position ([9beacfe](9beacfe)), closes [#1011](#1011)
* **chart:** Correct the order to set '$' node values ([b97558c](b97558c)), closes [#994](#994)
* **color:** Correct to not set stroke ([f18aa35](f18aa35)), closes [#754](#754) [#872](#872)
* **event:** Update determination condition ([736ba56](736ba56)), closes [#967](#967)
* **flow:** Fix data points removal ([5463150](5463150)), closes [#1006](#1006)
* **radar:** Correct display of indexed axis ([9bac296](9bac296)), closes [#997](#997)
* **text:** Correct text vertical align ([6debb55](6debb55)), closes [#982](#982)
* **tooltip:** Correct tooltip on dynamic loading ([c24bddb](c24bddb)), closes [#963](#963)
* **tooltip:** Fix on contents template ([419144f](419144f)), closes [#972](#972)

### Features

* **axis:** Intent to ship y/y2 axis culling ([44c6c4c](44c6c4c)), closes [#915](#915)
* **bubble:** Intent to ship dimension ([27df7c3](27df7c3)), closes [#484](#484)
* **options:** Pass instance arg to callbacks ([61cf047](61cf047)), closes [#989](#989)
* **radar:** Intent to ship axis.text.position ([1720ec2](1720ec2)), closes [#998](#998)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants