IITC-CE / ingress-intel-total-conversion

intel.ingress.com total conversion user script with some new features. Should allow easier extension of the intel map.
https://iitc.app
ISC License
290 stars 109 forks source link

drawtools doesn't appear to work correctly with mouse under IITC Mobile #162

Closed Whomiga closed 5 years ago

Whomiga commented 5 years ago

Works just fine on same phone in regular phone mode

Whomiga commented 5 years ago

Not sure but there may be a problem with IITC with relation to the resolution/position and such for the DeX mode as I've found selecting the portals can be difficult under DeX, even for info.

modos189 commented 5 years ago

What do I need to test this?

Whomiga commented 5 years ago

https://www.samsung.com/us/business/solutions/samsung-dex/

Whomiga commented 5 years ago

Just discovered it doesn't appear to be a DeX problem, it appears to be a mouse problem as I see the same problem as below using a mouse connected to my Note9, with no DeX connected.

It also doesn't matter if being done in Desktop or Phone mode

I've noticed that drawtools doesn't detect a mousepress (just selects the location on the map as if drawtools is not active) during draw polyline or draw polygon unless a drag is made in the location that a vertex is wanted. After the drag, the mouse pointer becomes a cross hair and a mousepress properly makes a vertex if the crosshair is not moved before mousepress ("refresh map movestart" is seen in debug console)

Whomiga commented 5 years ago

I've found that the Draw-Tools and all other operations of IITC Mobile work as designed when using DeX with the phone as a touchpad.

johnd0e commented 5 years ago

I confirm issue with mouse in IITCm. And official Leaflet.draw samples (e.g. https://leaflet.github.io/Leaflet.draw/docs/examples/full.html) does not have such problems.

johnd0e commented 5 years ago

I want to mention that there is another thing, which works great in leaflet.draw sample, but fails in iitc: it's a Circle (that's why we've disabled it in mobile mode).

johnd0e commented 5 years ago
  • It also can be some WebView-related issue.

It is not:

```total-conversion-build.user.js // ==UserScript== // @id ingress-intel-total-conversion@jonatkins // @name IITC: Ingress intel map total conversion // @version 0.29.1.20190423.135702 // @namespace https://github.com/IITC-CE/ingress-intel-total-conversion // @updateURL http://localhost:8101/total-conversion-build.meta.js // @downloadURL http://localhost:8101/total-conversion-build.user.js // @description [local-2019-04-23-135702] Total conversion for the ingress intel map. // @include https://intel.ingress.com/* // @match https://intel.ingress.com/* // @grant none // @run-at document-end // ==/UserScript== // REPLACE ORIG SITE /////////////////////////////////////////////////// window.onload = function() {}; document.body.onload = function() {}; document.getElementsByTagName('head')[0].innerHTML = ` Leaflet.draw popup event example ` // remove body element entirely to remove event listeners document.body = document.createElement('body'); document.body.innerHTML = '
'; function loadError(oError) { throw new URIError("The script " + oError.target.src + " didn't load correctly."); } function load(url, onloadFunction) { var newScript = document.createElement("script"); newScript.onerror = loadError; if (onloadFunction) { newScript.onload = onloadFunction; } document.head.appendChild(newScript); newScript.src = url; } function main () { var osmUrl = 'http://{s}.tile.openstreetmap.org/{z}/{x}/{y}.png', osmAttrib = '© OpenStreetMap contributors', osm = L.tileLayer(osmUrl, {maxZoom: 18, attribution: osmAttrib}), map = new L.Map('map', {center: new L.LatLng(29.9792, 31.1344), zoom: 15}), drawnItems = L.featureGroup().addTo(map); L.control.layers({ "osm": osm.addTo(map), "google": L.tileLayer('http://www.google.cn/maps/vt?lyrs=s@189&gl=cn&x={x}&y={y}&z={z}', { attribution: 'google' }) }, {'drawlayer':drawnItems}, { position: 'topright', collapsed: false }).addTo(map); map.addControl(new L.Control.Draw({ edit: { featureGroup: drawnItems, poly : { allowIntersection : false } }, draw: { polygon : { allowIntersection: false, showArea:true } } })); // Truncate value based on number of decimals var _round = function(num, len) { return Math.round(num*(Math.pow(10, len)))/(Math.pow(10, len)); }; // Helper method to format LatLng object (x.xxxxxx, y.yyyyyy) var strLatLng = function(latlng) { return "("+_round(latlng.lat, 6)+", "+_round(latlng.lng, 6)+")"; }; // Generate popup content based on layer type // - Returns HTML string, or null if unknown object var getPopupContent = function(layer) { // Marker - add lat/long if (layer instanceof L.Marker || layer instanceof L.CircleMarker) { return strLatLng(layer.getLatLng()); // Circle - lat/long, radius } else if (layer instanceof L.Circle) { var center = layer.getLatLng(), radius = layer.getRadius(); return "Center: "+strLatLng(center)+"
" +"Radius: "+_round(radius, 2)+" m"; // Rectangle/Polygon - area } else if (layer instanceof L.Polygon) { var latlngs = layer._defaultShape ? layer._defaultShape() : layer.getLatLngs(), area = L.GeometryUtil.geodesicArea(latlngs); return "Area: "+L.GeometryUtil.readableArea(area, true); // Polyline - distance } else if (layer instanceof L.Polyline) { var latlngs = layer._defaultShape ? layer._defaultShape() : layer.getLatLngs(), distance = 0; if (latlngs.length < 2) { return "Distance: N/A"; } else { for (var i = 0; i < latlngs.length-1; i++) { distance += latlngs[i].distanceTo(latlngs[i+1]); } return "Distance: "+_round(distance, 2)+" m"; } } return null; }; // Object created - bind popup to layer, add to feature group map.on(L.Draw.Event.CREATED, function(event) { var layer = event.layer; var content = getPopupContent(layer); if (content !== null) { layer.bindPopup(content); } drawnItems.addLayer(layer); }); // Object(s) edited - update popups map.on(L.Draw.Event.EDITED, function(event) { var layers = event.layers, content = null; layers.eachLayer(function(layer) { content = getPopupContent(layer); if (content !== null) { layer.setPopupContent(content); } }); }); } load('https://unpkg.com/leaflet@1.4.0/dist/leaflet-src.js', function () { load('https://unpkg.com/leaflet-draw@1.0.4/dist/leaflet.draw-src.js', main) }); ```

So the problem is somewhere in iitc code.

johnd0e commented 5 years ago

This sample contain essentially the same code as above, but doesn't work:

```draw-tools.user.js // ==UserScript== // @id iitc-plugin-draw-tools@breunigs // @name IITC plugin: Draw tools // @category Draw // @version 0.7.1.20190423.135702 // @description [local-2019-04-23-135702] Allow drawing things onto the current map so you may plan your next move. // @updateURL http://localhost:8101/plugins/draw-tools.meta.js // @downloadURL http://localhost:8101/plugins/draw-tools.user.js // @namespace https://github.com/IITC-CE/ingress-intel-total-conversion // @include https://intel.ingress.com/* // @match https://intel.ingress.com/* // @grant none // ==/UserScript== function wrapper(plugin_info) { // ensure plugin framework is there, even if iitc is not yet loaded if(typeof window.plugin !== 'function') window.plugin = function() {}; function setup () { $('head').append(''); $.getScript('https://unpkg.com/leaflet-draw@1.0.4/dist/leaflet.draw-src.js', function () { var drawnItems = L.featureGroup().addTo(map); map.addControl(new L.Control.Draw({ edit: { featureGroup: drawnItems, poly : { allowIntersection : false } }, draw: { polygon : { allowIntersection: false, showArea:true } } })); // Truncate value based on number of decimals var _round = function(num, len) { return Math.round(num*(Math.pow(10, len)))/(Math.pow(10, len)); }; // Helper method to format LatLng object (x.xxxxxx, y.yyyyyy) var strLatLng = function(latlng) { return "("+_round(latlng.lat, 6)+", "+_round(latlng.lng, 6)+")"; }; // Generate popup content based on layer type // - Returns HTML string, or null if unknown object var getPopupContent = function(layer) { // Marker - add lat/long if (layer instanceof L.Marker || layer instanceof L.CircleMarker) { return strLatLng(layer.getLatLng()); // Circle - lat/long, radius } else if (layer instanceof L.Circle) { var center = layer.getLatLng(), radius = layer.getRadius(); return "Center: "+strLatLng(center)+"
" +"Radius: "+_round(radius, 2)+" m"; // Rectangle/Polygon - area } else if (layer instanceof L.Polygon) { var latlngs = layer._defaultShape ? layer._defaultShape() : layer.getLatLngs(), area = L.GeometryUtil.geodesicArea(latlngs); return "Area: "+L.GeometryUtil.readableArea(area, true); // Polyline - distance } else if (layer instanceof L.Polyline) { var latlngs = layer._defaultShape ? layer._defaultShape() : layer.getLatLngs(), distance = 0; if (latlngs.length < 2) { return "Distance: N/A"; } else { for (var i = 0; i < latlngs.length-1; i++) { distance += latlngs[i].distanceTo(latlngs[i+1]); } return "Distance: "+_round(distance, 2)+" m"; } } return null; }; // Object created - bind popup to layer, add to feature group map.on(L.Draw.Event.CREATED, function(event) { var layer = event.layer; var content = getPopupContent(layer); if (content !== null) { layer.bindPopup(content); } drawnItems.addLayer(layer); }); // Object(s) edited - update popups map.on(L.Draw.Event.EDITED, function(event) { var layers = event.layers, content = null; layers.eachLayer(function(layer) { content = getPopupContent(layer); if (content !== null) { layer.setPopupContent(content); } }); }); }) } setup.info = plugin_info; //add the script info data to the function as a property if(!window.bootPlugins) window.bootPlugins = []; window.bootPlugins.push(setup); // if IITC has already booted, immediately run the 'setup' function if(window.iitcLoaded && typeof setup === 'function') setup(); } // wrapper end // inject code into site context var script = document.createElement('script'); var info = {}; if (typeof GM_info !== 'undefined' && GM_info && GM_info.script) info.script = { version: GM_info.script.version, name: GM_info.script.name, description: GM_info.script.description }; script.appendChild(document.createTextNode('('+ wrapper +')('+JSON.stringify(info)+');')); (document.body || document.head || document.documentElement).appendChild(script); ```

So the problem is outside of draw-tools plugin code.

johnd0e commented 5 years ago

It's not oms, it's not taphold. Also I removed from code all event listeners attached by .on(..) and .click(...), and it not helped.

But if I create new map (after current map.remove()), then in new map all is right.

Whomiga commented 5 years ago

Do you have a fork in which you have made your changes to be tested and/or examined?

johnd0e commented 5 years ago

Use code from details above. As for oms, taphold, and numerous event listeners, then I didn't keep that quick-and-dirty patch, but it shouldn't be hard to repeat it yourself.

BTW, I tend to think that another issue can be indirectly related: on mobile it's impossible to create circle and rectangle. I'm going to try to debug this, as it is reproduceable even on desktop (mobile mode in Chrome)

johnd0e commented 5 years ago

So I've found the bug source.

For correct work we must load Leaflet.draw before map initialization.

It can be fixed in upstream: https://github.com/Leaflet/Leaflet.draw/issues/923

But (as long as we know what's happening) it's also easy to fix the issue in our code.

Whomiga commented 5 years ago

I think I fully updated my fork with your changes, but the circle icon still isn't showing up in the mobile version (shows on desktop) - am I missing something on updating scripts in the APK or something? Mouse works great though, nice work.

Whomiga commented 5 years ago

Note: Shouldn't the use of a mouse be Click, then see a running length and dashed line till the next click, etc? - that is how it operates on the normal desktop version.

johnd0e commented 5 years ago

am I missing something on updating

Perhaps you've missed smartphone.css

Whomiga commented 5 years ago

am I missing something on updating

Perhaps you've missed smartphone.css

Changed on my copy here, not sure if the APK is using it but I did the build.py local, etc...

johnd0e commented 5 years ago

After last commit we have issue with polyline https://github.com/Leaflet/Leaflet.draw/issues/789.

And the worst thing is that Leaflet.draw seems unmaintained.

johnd0e commented 5 years ago

I really doubt that we should include the fix into release, as it brings more problems than it solves. Leaflet.draw is buggy, and I do not feel that we are able to fix all cases. BTW, I tend to think that current draw-tools is also not convenient for most of our use cases, and we should look for alternatives (related: #144).

@Whomiga You still can use 'fixed' plugin as external. Or you can develop sort of smart behavior: activating DeX could affect TouchExtend handler.

P.S.

activating DeX

Perhaps better would be to detect if current pointer is mouse (it should be possible, I hope).

johnd0e commented 5 years ago

it brings more problems than it solves

I can try some compromise, like dynamically enable TouchExtend handler for Circle drawing only: #175

And would be happy to include fix, if you find it:

detect if current pointer is mouse

johnd0e commented 5 years ago

Changed on my copy here, not sure if the APK is using it but I did the build.py local, etc...

Make sure that you've updated total-conversion-build.user.js too.

Note: Shouldn't the use of a mouse be Click, then see a running length and dashed line till the next click, etc?

Report this to upstream.

johnd0e commented 5 years ago

@Whomiga

detect if current pointer is mouse

Good news, I've got an idea: use ':hover' selector to check if we have active mouse pointer. I am not sure how reliable it is, so please check it in all configurations (incl. plain desktop).

https://github.com/johnd0e/ingress-intel-total-conversion/commit/ede45794bf79dbbb8ad0951786fa7c9410cadaf8

I also have other ideas (in case if this one fails), so I keep some sources here:

- https://webplatform.github.io/docs/concepts/Pointer_Events/ - https://www.html5rocks.com/en/mobile/touchandmouse/ - https://pawelgrzybek.com/whats-the-deal-with-the-pointer-events-in-javascript/ - https://docs.google.com/document/d/1mpBR7J7kgTXvp0QACVjhxtwNJ7bgGoTMmxfxN2dupGg/edit#heading=h.9zs8i7jdl2ya - https://patrickhlauke.github.io/getting-touchy-presentation/ - https://stackoverflow.com/questions/21054126/how-to-detect-if-a-device-has-mouse-support/
1valdis commented 5 years ago

@johnd0e

Good news, I've got an idea: use ':hover' selector to check if we have active mouse pointer.

It isn't correct, because ':hover' is initiated in most mobile browsers on each tap by some element. So you tap a button, for example, it activates ':hover', and it stays ':hover'-ed, as if you used a mouse to click and stayed hovered on it after the click.

johnd0e commented 5 years ago

@1valdis Too bad.. May be you have another idea?

1valdis commented 5 years ago

@johnd0e There's a CSS media rule just for that: (pointer: fine). Unfortunately its browser support might be too poor to use with IITC. You may still check it out and use it only if it's supported. There's also solution which uses jQuery.

johnd0e commented 5 years ago

Unfortunately jQuery solution does not allow to make test immediately (only after click). And user can attach/detach mouse in any moment, so we need to test it dynamically.

There's a CSS media rule just for that: (pointer: fine).

May be it could be used, but also not in strait-forward way(

1valdis commented 5 years ago

@johnd0e

May be it could be used, but also not in strait-forward way(

if (matchMedia('(pointer:fine)').matches) {
  // Device has a mouse
}

(copied from answer on the same SO question) What's more straightforward? :)

johnd0e commented 5 years ago

What's more straightforward? :)

You are right, it is. Unfortunately even with mouse attached I get false.

But... I succeed in using another media query: matchMedia('(hover:hover)')

Thank you!

johnd0e commented 5 years ago

I succeed in using another media query: matchMedia('(hover:hover)')

This media query works the same way: matchMedia('(any-pointer:fine)')

Unfortunately both methods are only able to detect if mouse is attached. But none of them can detect if mouse is currently active.

It's obvious that browser itself has this info (we see pointer icon only when mouse is active)

1valdis commented 5 years ago

I believe you should change your way of telling if the mouse is used from simple CSS media matching to events, if you want more control. Specifically, you can use pointer events instead of mouse or touch ones, and use their pointerType property. Their browser support though... leaves much to be desired, but I still recommend to check them out.

johnd0e commented 5 years ago

I believe you should change your way of telling if the mouse is used from simple CSS media matching to events, if you want more control.

Well, you are right. And it's rather easy. But still it increases the whole complexity of this part of code. And as this part is not more than a workaround over Leaflet.draw bugs, I just try to keep it as simple as possible.

May be we should drop this hack, as I also have some progress from the opposite side: https://github.com/Leaflet/Leaflet.draw/pull/926

(Need more testing in different configurations..)