do not display fullscreen control on unsupported devices #4786 #4838
Conversation
src/ui/control/fullscreen_control.js
Outdated
| @@ -4,6 +4,8 @@ const DOM = require('../../util/dom'); | |||
| const util = require('../../util/util'); | |||
| const window = require('../../util/window'); | |||
|
|
|||
| const className = 'mapboxgl-ctrl'; | |||
There was a problem hiding this comment.
instead of another global variable could you store this as this._className?
There was a problem hiding this comment.
According to my interpretation of the docs, I think we should be checking for [[prefix]]fullscreenEnabled, not requestFullscreen https://developer.mozilla.org/en-US/docs/Web/API/Document/fullscreenEnabled
This will not only check browser support, but also whether or not the current element is available in full-screen mode
src/ui/control/fullscreen_control.js
Outdated
| if (this._checkFullscreenSupport()) { | ||
| this._setupUI(); | ||
| } else { | ||
| util.warnOnce('This device does not support fullscreen mode.'); |
There was a problem hiding this comment.
we should also set display:none; for the container when fullscreen is not supported
|
Done! |
|
Thanks for the contribution @stepankuzmin! I think this is almost ready to go, but it would be great to see a unit test that confirms the control container gets added to the map when fullscreen is enabled. |
|
Thanks for the review, @mollymerp! I've updated the PR. |
|
Thanks again @stepankuzmin! |
|
There's a problem now on Firefox as on Safari mobile: does not display the Fullscreen icon any more and logs "This device does not support fullscreen mode." to the console. Still working on Safari (desktop). Until api v. 0.38.0 it worked in Firefox as well. |
|
@dieterdreist Fullscreen was never supported on mobile devices. Mapbox was mistakenly showing the control, but it couldn't have worked, because the fullscreen API isn't supported (except for video and audio elements). |
|
Fixed Firefox in #5272. |
Do not display fullscreen control on unsupported devices according to #4786
I think, that it would be useful to warn the user if the device does not support fullscreen mode.