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

Map not responsive after click outside the map when on touch device and draggable:false option #332

Closed
yoadsn opened this issue Mar 5, 2017 · 10 comments · Fixed by #335
Closed

Comments

@yoadsn
Copy link
Contributor

yoadsn commented Mar 5, 2017

I may have found the wrong reproduction of this problem but I modified an old JSBin to show the problem:
Currently checked on chrome dev tools using the "mobile" tap input emulation.
https://jsbin.com/wihaxezera/edit?js,console,output

After the map loads - tapping the marker would produce a double "click" console log.. (should not be double)
Then, clicking on the red area to the right (just outside the map div container) would make click event to never fire again.

When removing either draggable:false or fullscreenControl:false this behaviour is not observed.

Did not have a chance to test yet on an actual mobile device..

@istarkov
Copy link
Collaborator

istarkov commented Mar 5, 2017

I have almost zero knowledge about tap devices and their events, any help will be very appreciated.

@yoadsn
Copy link
Contributor Author

yoadsn commented Mar 6, 2017

@istarkov Sure - I will try and dive deeper - just wanted to make sure it was not an obvious thing which is
related to the the change that was made to prevent "map" clicks to appear beneath "marker" clicks a few months ago - #185

@yoadsn
Copy link
Contributor Author

yoadsn commented Mar 9, 2017

@istarkov I am unable to find the cause for this problem since it reproduces without apparent logic. Although I have a feeling it has to do with #233 fix that was applied and following what I read in #297.
I know that you are not taking care of any mobile touch events since it's not your field of expertise but unfortunately this (#233) really breaks mobile behavior.
I will continue this discussion on the relevant issue. This issue can be closed until the other problem is solved and we can be sure it's not the cause.

@yoadsn yoadsn closed this as completed Mar 9, 2017
@yoadsn yoadsn reopened this Mar 9, 2017
@yoadsn
Copy link
Contributor Author

yoadsn commented Mar 9, 2017

@istarkov I have some new information. After removing the #233 fix this problem still occurs. While I was at the code I put the following traces:

_this._onMapMouseDownNative = function (event) {
      console.log('_onMapMouseDownNative')
      if (!_this.mouseInMap_) return;

      console.log('-- > _onMapMouseDown')
      _this._onMapMouseDown(event);
    };

And indeed, normal click on the map produces both trace lines. After clicking outside the map (some other div) only the first line is logged.

Now adding these two lines:

maps.event.addListener(map, 'mouseover', function () {
          // has advantage over div MouseLeave
          console.log('mouseover')
          this_.mouseInMap_ = true;
        });

        maps.event.addListener(map, 'mouseout', function () {
          // has advantage over div MouseLeave
          console.log('mouseout')
          this_.mouseInMap_ = false;
          this_.mouse_ = null;
          this_.markersDispatcher_.emit('kON_MOUSE_POSITION_CHANGE');
        });

Would show that on mobile touch devices - clicking outside the map would (stupidly) produce a "mouseout" event but would never produce a corresponding "mouseover" event.

However adding this logging line:

_this._onMapClick = function (event) {
      console.log('_onMapClick')
      if (_this.markersDispatcher_) {
        // support touch events and recalculate mouse position on click
        _this._onMapMouseMove(event);
        var currTime = new Date().getTime();
        if (currTime - _this.dragTime_ > K_IDLE_TIMEOUT) {
          if (_this.mouse_) {
            _this._onClick(_extends({}, _this.mouse_, {
              event: event
            }));
          }

          _this.markersDispatcher_.emit('kON_CLICK', event);
        }
      }
    };

shows that map click is logged on mobile (why not).

So doing:

_this.mouseInMap_ = true;

Within that handler seems to solve that problem since any tap on the top div would fire the click event and notify the component that "mouse is over again" in a different way.

Let me know if this seems a reasonable analysis and workaround - I know you do not plan to make this component 100% mobile ready and I am no expert to know how should that be done correctly.

@istarkov
Copy link
Collaborator

istarkov commented Mar 9, 2017

I wrote that mouseInMap_ to check that mouse is not above gmap controls, like zoom etc
So setting mouseInMap_ = true will broke a lot of existing code, as in such case every click on zoom control for example will be resolved as map click.

_onMapMouseDownNative is needed because interception of internal map mousedown event don't allow to create dragable markers.

As you see the codebase is a big hack around google map api ;-)

So what can we do with mobile having that mouseover never occurred, I don't know ;-)
let me think or provide any ideas

@istarkov
Copy link
Collaborator

istarkov commented Mar 9, 2017

Sometimes I think that the best is to setup internal tile server and use something beautiful written on react like https://github.com/mariusandra/pigeon-maps
No hacks, no problems.

@yoadsn
Copy link
Contributor Author

yoadsn commented Mar 9, 2017

@istarkov I trust you - If you write it - I will use it 🤡
Would you suggest migrating to the MapBox react component from uber if mobile is an important use case for me? https://github.com/uber/react-map-gl

To the point, since currently it's 100% broken on mobile - if we can improve mobile without affecting desktop that would seem like a good step. the google maps click event is not fired when clicking on the zoom control (for example) - would that event be a good place to turn mouseInMap_ on again?

@istarkov
Copy link
Collaborator

istarkov commented Mar 9, 2017

Initially this control was written for MapBox map, but that time webgl was at near 70% of computers. And my client had the computer from that other 30%. So instead of very good MapBox solution I wrote this.
If mapbox fees are ok for your company Ill always will recommend their solution.

Seems like your solution is reasonable,. Ill accept a PR

@yoadsn yoadsn changed the title broken child click handler breaks on tap while using both map option draggable:false and fullscreenControl:false Map not responsive after click outside the map when on touch device and draggable:false option Mar 10, 2017
@yoadsn
Copy link
Contributor Author

yoadsn commented Mar 10, 2017

For future reference -
We are using the maps 'click' events which only fires on actual map clicks (not google map on-map controls) to reactivate the flag 'mouseInMap_'.

The reason we need to activate it is the following:

  • When the map options of google maps are 'draggable:false' - google would initiate a "mouseout' event when an area outside of the map is tapped
  • the corresponding 'mouseover' event would never fire on touch devices
  • any tap after 'mouseout' on the map is ignored since there is an internal logic ignoring such clicks when 'mouseInMap_' is turned off
  • the 'click' event would fire also on touch devices and only as a response for a map tap and not an on-map control tap (including google map markers).

Warning If you are using native google map markers and on a touch device - clicking outside the map component and than on a marker would NOT be considered a click from the point of view of this wrapper component. So all the "children mouse logic" would not work.. (although this is assumed to be a weird setup).

@lock
Copy link

lock bot commented Dec 1, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Dec 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants