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

Buttons keep the "leaflet-active" class after mouse up (IE11) #2103

Closed
fnicollet opened this issue Oct 17, 2013 · 11 comments
Closed

Buttons keep the "leaflet-active" class after mouse up (IE11) #2103

fnicollet opened this issue Oct 17, 2013 · 11 comments
Assignees
Labels
Milestone

Comments

@fnicollet
Copy link
Collaborator

Hi,

A little something that might have to do with pointer event handing for IE11. Try any map with a zoom component in IE11 like http://leafletjs.com/. Click the zoom buttons, they will keep the "leaflet-active" class, even after you pushed them.

This class should be removed when up.

This might have to do with this issue #2102 because the "_onUp" function is never called.

In Pointer.js, I believe there is an issue. in the function addPointerListenerEnd:
this._pointers is undefined so no touch id is sent to the event

It might lead to bigger issues, although i didn't experience any other bad effect

Fabien

@danzel
Copy link
Member

danzel commented Oct 18, 2013

Thanks for these IE11 reports.
We have a windows 8.1 touch device now, so I'll try look at these on Monday

@ghost ghost assigned danzel Oct 18, 2013
@fnicollet
Copy link
Collaborator Author

Thanks Dave, much appreciated.
Multitouch seem to be broken as well, at least it doesn't work in the simulator when I try it. I have no touch device so that's the only way i can try it (TRANSITION seem to be false for some reason and basically the map is just stuck there)

Thanks,

@fnicollet
Copy link
Collaborator Author

Just tried it in IE11 Desktop, same behaviour, the class stays on the element. The #2102 fix doesn't change anything. I believe there is a deeper issue with touch events

@fnicollet
Copy link
Collaborator Author

The second fix in #2102 doesn't fix this bug unfortunately, the outline still stays.

@danzel
Copy link
Member

danzel commented Oct 20, 2013

Sent a PR #2111 that fixes this. Looks like there was some leftover code from when you tidied up draggable @mourner :)
Unless there is a reason this was being added in there? There doesn't seem to be any code that removes it except in tap, which pointer doesn't use.

Please give it a test @fnicollet

@danzel danzel closed this as completed in 91cc185 Oct 21, 2013
mourner added a commit that referenced this issue Oct 21, 2013
Remove leftover code from the draggable/tap cleanup. Fixes #2103
@mourner
Copy link
Member

mourner commented Oct 21, 2013

@danzel yep, you're totally right! Thanks a lot for figuring this out :)

@fnicollet
Copy link
Collaborator Author

@danzel just tried it, works :)

@fnicollet
Copy link
Collaborator Author

But the leaflet-active class is never set, even in Chrome. Is it really useful to keep leaflet-active?

.leaflet-container a.leaflet-active {
    outline: 2px solid orange;
    }

Because there is already a focus style with each browser (orange outline in Chrome, dotted outline in IE)

@mourner
Copy link
Member

mourner commented Oct 21, 2013

@fnicollet
Copy link
Collaborator Author

Maybe I'm doing something wrong, but in leaflet-src.js, if i set it to:

L.Map.Tap = L.Handler.extend({
    addHooks: function () {
    alert(1);
        L.DomEvent.on(this._map._container, 'touchstart', this._onDown, this);
    },

No alert is shown, Chrome & IE. Also, when I hover the Zoom In button in IE, there is a weird padding on the "+" sign

@danzel
Copy link
Member

danzel commented Oct 21, 2013

Tap is only used on android and iOS.
When you finger down on the +/- buttons it shows.

mourner added a commit that referenced this issue Oct 26, 2013
* 'master' of https://github.com/Leaflet/Leaflet:
  Fixing long line
  Adding bounceAtZoomLimits option
  The drag click avoid hack is needed for touch zooms on ie10 also, when releasing your fingers after a touch zoom click events are generated. Fixes #2094
  Remove leftover code from the draggable/tap cleanup. Fixes #2103
  Changed main module file to use un-minified version.
  adding alt tag to marker icons
  Fixing line too long error
  Do not scale above minzoom and maxzoom in touch zoom action
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