brunob / leaflet.fullscreen

Leaflet.Control.FullScreen for Leaflet
https://brunob.github.io/leaflet.fullscreen/
MIT License
376 stars 107 forks source link

Screenfull conditional #101

Closed sfusato closed 2 years ago

sfusato commented 2 years ago

Fixes #99.

Raruto commented 2 years ago

Hi @brunob,

by chance do you need a hand to evaluate this pull? try asking @hupe13, he told me that applying this fix https://github.com/Raruto/leaflet-elevation/issues/203#issuecomment-1137461165 within his plugin the problem doesn't seem to occur anymore.

Have a nice day, Raruto

brunob commented 2 years ago

Thx for the reminder @Raruto i'll merge it and push a new release asap :)

@sfusato maybe it would be cleaner to test this._screenfull.isEnabled instead of this._screenfull.raw can you test it on ios please ?

brunob commented 2 years ago

Since @sfusato is not available, @hupe13 can you test the patch with the modification i propose please ?

hupe13 commented 2 years ago

I'm sorry, I don't have an iPhone.

I found this pull request because iPhone users had complained about something else in the forum. I was looking for a solution for the problem and just installed the pull request. After that there was no support request about this anymore.

I will test it with all browsers I have and report.

brunob commented 2 years ago

I'd like to fix this one, maybe @pfrumau could test the change i propose on ios ?

hupe13 commented 2 years ago

maybe it would be cleaner to test this._screenfull.isEnabled instead of this._screenfull.raw

I replaced every string "this._screenfull.raw " with "this._screenfull.isEnabled". Is this correct? If I change this, I get a "Type Error" in my testing environment in Firefox. But remember, I use Bozdoz plugin. It may be that this is a special case.

brunob commented 2 years ago

@hupe13 from my side, the version https://github.com/brunob/leaflet.fullscreen/tree/issue_99 works on firefox, but my question is : does this fix the bug on ios ?

hupe13 commented 2 years ago

I tested it on iPadOS 15, it works. I will look for a user with an iPhone, let's see if I can find one. It also works on Firefox and Chrome and on Smartphone.

brunob commented 2 years ago

@hupe13 thx for the feedback !

pfrumau commented 2 years ago

Hey all! I can't work on this this month.. in august I will start development again, so ill try this new version and let you know what i find! But, since it is exactly the same code as i propose in my issue (and i am using that atm in https://www.borrelroutes.nl), I assume it works.

brunob commented 2 years ago

But, since it is exactly the same code as i propose in my issue

@pfrumau not exactly the same ;)

hupe13 commented 2 years ago

I was confirmed that it works with "Mozilla/5.0 (iPhone; CPU iPhone OS 15_5 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/15.5 Mobile/xxxxxx Safari/604.1".

brunob commented 2 years ago

Merged, tagged and published, thx for the feedback @hupe13 :)