Allow user to define element other than map container for full screen control #7548
Conversation
src/ui/control/fullscreen_control.js
Outdated
| this._fullscreen = false; | ||
| if (options && options.source) { | ||
| this._source = window.document.querySelector(options.source); |
There was a problem hiding this comment.
Our other APIs that need an HTML element reference, such as Map container and Marker element, accept an HTMLElement rather than a selector — I'd suggest keeping this consistent.
ryanhamley
force-pushed
the
enable-fullscreen-source-option
branch
from
80fb8e8 to
4ac4092
Compare
|
Added a unit test and changed |
| const control = map._controls.find((ctrl) => { | ||
| return ctrl.hasOwnProperty('_fullscreen'); | ||
| }); | ||
| control._onClickFullscreen(); |
There was a problem hiding this comment.
maybe set a spy on body.requestFullscreen and then check t.calledOnce after this?
There was a problem hiding this comment.
I might be missing something but it doesn't seem like JSDOM supports the Full Screen API. I don't see any documentation or code that mentions it and window.document.requestFullScreen and body.requestFullScreen are both undefined (along with their prefixed alternatives) in the context of the test suite.
fixes #7544
Launch Checklist
The intended use case is for displaying a map with an overlaid element that works with the map, such as radio buttons to toggle styles or a banner element with a legend. The current behavior of making the map container element full screen will not show these elements. I can't really think of any particular reason to disallow this behavior but let me know if there's a reason not to implement this.
Full screen with
{source: body}Default behavior (notice that the debug control in the upper left corner is not displayed)
