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

#2498: Don't show cooperativeGestures messages when scrollZoom is dis… #3240

Closed
wants to merge 14 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/ui/control/control.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import type {Map} from '../map';
* A position defintion for the control to be placed, can be in one of the corners of the map.
* When two or more controls are places in the same location they are stacked toward the center of the map.
*/
export type ControlPosition = 'top-left' | 'top-right' | 'bottom-left' | 'bottom-right';
export type ControlPosition = 'top-left' | 'top-right' | 'bottom-left' | 'bottom-right' | 'full';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how I feel about this, since there isn't another control that is full and I'm not sure what would be the expected behavior.
Can this control simply ignore this input maybe?
IDK...

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason for 'full' position to be there is following method in map.ts
https://github.com/arekgotfryd/maplibre-gl-js/blob/3da57effa8c864fd7cad9682f5a84816eed42f33/src/ui/map.ts#L717
if I remove getDefaultPosition implementation from CooperativeGestureControl class addControl method will assume top-right position for it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While not "accurate" the default seems to be OK I think.
Otherwise we'll need to support configuring the scale control, or zoom control to be full screen, which is not what we want I believe.


/**
* Interface for interactive controls added to the map. This is a
Expand Down
113 changes: 113 additions & 0 deletions src/ui/control/cooperative_gesture_contol.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
import {DOM} from '../../util/dom';

import type {Map} from '../map';
import type {ControlPosition, IControl} from './control';

/**
* The {@link CooperativeGestureControl} options object
*/
/**
* An options object for the gesture settings
* @example
* ```ts
* let options = {
* windowsHelpText: "Use Ctrl + scroll to zoom the map",
* macHelpText: "Use ⌘ + scroll to zoom the map",
* mobileHelpText: "Use two fingers to move the map",
* }
* ```
*/
export type GestureOptions = {
windowsHelpText?: string;
macHelpText?: string;
mobileHelpText?: string;
};

/**
* A `CooperativeGestureControl` is a control that adds cooperative gesture info when user tries to zoom in/out.
*
* @group Markers and Controls
*
* @example
* ```ts
* map.addControl(new maplibregl.CooperativeGestureControl({
* windowsHelpText: "Use Ctrl + scroll to zoom the map",
* macHelpText: "Use ⌘ + scroll to zoom the map",
* mobileHelpText: "Use two fingers to move the map",
* }));
* ```
**/
export class CooperativeGestureControl implements IControl {
options: boolean | GestureOptions;
_map: Map;
_container: HTMLElement;
_metaKey: keyof MouseEvent = navigator.userAgent.indexOf('Mac') !== -1 ? 'metaKey' : 'ctrlKey';

constructor(options: boolean | GestureOptions = {}) {
this.options = options;
}

getDefaultPosition(): ControlPosition {
return 'full';
}

/** {@inheritDoc IControl.onAdd} */
onAdd(map:Map) {
this._map = map;
const mapCanvasContainer = this._map.getCanvasContainer();
const cooperativeGestures = this._map.getCooperativeGestures();
this._container = DOM.create('div', 'maplibregl-cooperative-gesture-screen', this._map.getContainer());
let desktopMessage = typeof cooperativeGestures !== 'boolean' && cooperativeGestures.windowsHelpText ? cooperativeGestures.windowsHelpText : 'Use Ctrl + scroll to zoom the map';
if (this._metaKey === 'metaKey') {
desktopMessage = typeof cooperativeGestures !== 'boolean' && cooperativeGestures.macHelpText ? cooperativeGestures.macHelpText : 'Use ⌘ + scroll to zoom the map';
}
const mobileMessage = typeof cooperativeGestures !== 'boolean' && cooperativeGestures.mobileHelpText ? cooperativeGestures.mobileHelpText : 'Use two fingers to move the map';
// Create and append the desktop message div
const desktopDiv = document.createElement('div');
desktopDiv.className = 'maplibregl-desktop-message';
desktopDiv.textContent = desktopMessage;
this._container.appendChild(desktopDiv);
// Create and append the mobile message div
const mobileDiv = document.createElement('div');
mobileDiv.className = 'maplibregl-mobile-message';
mobileDiv.textContent = mobileMessage;
this._container.appendChild(mobileDiv);
// Remove cooperative gesture screen from the accessibility tree since screenreaders cannot interact with the map using gestures
this._container.setAttribute('aria-hidden', 'true');
// Add event to canvas container since gesture container is pointer-events: none
this._map.on('wheel', this._cooperativeGesturesOnWheel);
this._map.on('touchmove', this._cooperativeGesturesOnTouch);
// Add a cooperative gestures class (enable touch-action: pan-x pan-y;)
mapCanvasContainer.classList.add('maplibregl-cooperative-gestures');
return this._container;
}
/** {@inheritDoc IControl.onRemove} */
onRemove(map: Map) {
DOM.remove(this._container);
const mapCanvasContainer = map.getCanvasContainer();
map.off('wheel', this._cooperativeGesturesOnWheel);
map.off('touchmove', this._cooperativeGesturesOnTouch);
mapCanvasContainer.classList.remove('maplibregl-cooperative-gestures');
this._map = undefined;
}

_cooperativeGesturesOnTouch = () => {
this._onCooperativeGesture(false);
};

_cooperativeGesturesOnWheel = (event: WheelEvent) => {
this._onCooperativeGesture(event['originalEvent'][this._metaKey]);
};

_onCooperativeGesture(metaPress:boolean) {
if (!metaPress) {
// Alert user how to scroll/pan
this._container.classList.add('maplibregl-show');
setTimeout(() => {
this._container.classList.remove('maplibregl-show');
}, 100);
}
return false;
}

}
3 changes: 2 additions & 1 deletion src/ui/control/fullscreen_control.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@ import {DOM} from '../../util/dom';
import {warnOnce} from '../../util/util';

import {Event, Evented} from '../../util/evented';
import type {Map, GestureOptions} from '../map';
import type {Map} from '../map';
import type {IControl} from './control';
import {GestureOptions} from './cooperative_gesture_contol';

/**
* The {@link FullscreenControl} options
Expand Down
6 changes: 5 additions & 1 deletion src/ui/handler/scroll_zoom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ const maxScalePerFrame = 2;
export class ScrollZoomHandler implements Handler {
_map: Map;
_tr: TransformProvider;
_metaKey: keyof MouseEvent = navigator.userAgent.indexOf('Mac') !== -1 ? 'metaKey' : 'ctrlKey';
_el: HTMLElement;
_enabled: boolean;
_active: boolean;
Expand Down Expand Up @@ -136,6 +137,8 @@ export class ScrollZoomHandler implements Handler {
if (this.isEnabled()) return;
this._enabled = true;
this._aroundCenter = !!options && (options as AroundCenterOptions).around === 'center';
const cooperativeGestures = this._map.getCooperativeGestures();
if (cooperativeGestures) this._map.setCooperativeGestures(cooperativeGestures);
}

/**
Expand All @@ -147,14 +150,15 @@ export class ScrollZoomHandler implements Handler {
* ```
*/
disable() {
this._map.setCooperativeGestures(false);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't this disable cooperative gesture even if it was enabled to begin with when disabling scroll zoom?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't follow...
What is the point of cooperative gestures message when zooming is disabled?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can enable and disable scroll zoom after the map is initialized, this will "forget" the initial state I think.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I get it now but in that case I would need two additional methods on Map class:
enableCooperativeGestures
disableCooperativeGestures
Are you fine with that?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having these separate methods would avoid cluttering of setCooperativeGestures method.

if (!this.isEnabled()) return;
this._enabled = false;
}

wheel(e: WheelEvent) {
if (!this.isEnabled()) return;
if (this._map._cooperativeGestures) {
if (e[this._map._metaKey]) {
if (e[this._metaKey]) {
e.preventDefault();
} else {
return;
Expand Down
18 changes: 2 additions & 16 deletions src/ui/handler/touch_pan.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import Point from '@mapbox/point-geometry';
import {indexTouches} from './handler_util';
import type {Map} from '../map';
import {GestureOptions} from '../map';

import {Handler} from '../handler_manager';
import {GestureOptions} from '../control/cooperative_gesture_contol';

export class TouchPanHandler implements Handler {

Expand All @@ -15,7 +16,6 @@ export class TouchPanHandler implements Handler {
_clickTolerance: number;
_sum: Point;
_map: Map;
_cancelCooperativeMessage: boolean;

constructor(options: {
clickTolerance: number;
Expand All @@ -31,27 +31,13 @@ export class TouchPanHandler implements Handler {
this._active = false;
this._touches = {};
this._sum = new Point(0, 0);

// Put a delay on the cooperative gesture message so it's less twitchy
setTimeout(() => {
this._cancelCooperativeMessage = false;
}, 200);
}

touchstart(e: TouchEvent, points: Array<Point>, mapTouches: Array<Touch>) {
return this._calculateTransform(e, points, mapTouches);
}

touchmove(e: TouchEvent, points: Array<Point>, mapTouches: Array<Touch>) {
if (this._map._cooperativeGestures) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't seen this code moved to the controller, am I missing something?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because you removed it...

touchmove(e: TouchEvent, points: Array<Point>, mapTouches: Array<Touch>) {

if (this._minTouches === 2 && mapTouches.length < 2 && !this._cancelCooperativeMessage) {
// If coop gesture enabled, show panning info to user
this._map._onCooperativeGesture(e, false, mapTouches.length);
} else if (!this._cancelCooperativeMessage) {
// If user is successfully navigating, we don't need this warning until the touch resets
this._cancelCooperativeMessage = true;
}
}
if (!this._active || mapTouches.length < this._minTouches) return;
e.preventDefault();
return this._calculateTransform(e, points, mapTouches);
Expand Down
6 changes: 6 additions & 0 deletions src/ui/map.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2677,6 +2677,12 @@ describe('Map', () => {

expect(map.getContainer().querySelector('.maplibregl-cooperative-gesture-screen').getAttribute('aria-hidden')).toBeTruthy();
});

test('cooperativeGesture container element is not available when scrollZoom disabled', () => {
const map = createMap({cooperativeGestures: true});
map.scrollZoom.disable();
expect(map.getContainer().querySelector('.maplibregl-cooperative-gesture-screen')).toBeFalsy();
});
});

describe('getCameraTargetElevation', () => {
Expand Down
87 changes: 13 additions & 74 deletions src/ui/map.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ import {Terrain} from '../render/terrain';
import {RenderToTexture} from '../render/render_to_texture';
import {config} from '../util/config';
import type {QueryRenderedFeaturesOptions, QuerySourceFeatureOptions} from '../source/query_features';
import {CooperativeGestureControl, GestureOptions} from './control/cooperative_gesture_contol';

const version = packageJSON.version;

Expand Down Expand Up @@ -328,23 +329,6 @@ export type MapOptions = {
maxCanvasSize?: [number, number];
};

/**
* An options object for the gesture settings
* @example
* ```ts
* let options = {
* windowsHelpText: "Use Ctrl + scroll to zoom the map",
* macHelpText: "Use ⌘ + scroll to zoom the map",
* mobileHelpText: "Use two fingers to move the map",
* }
* ```
*/
export type GestureOptions = {
windowsHelpText?: string;
macHelpText?: string;
mobileHelpText?: string;
};

export type AddImageOptions = {

}
Expand Down Expand Up @@ -450,15 +434,13 @@ export class Map extends Camera {
style: Style;
painter: Painter;
handlers: HandlerManager;

_container: HTMLElement;
_canvasContainer: HTMLElement;
_controlContainer: HTMLElement;
_controlPositions: {[_: string]: HTMLElement};
_interactive: boolean;
_cooperativeGestures: boolean | GestureOptions;
_cooperativeGesturesScreen: HTMLElement;
_metaKey: keyof MouseEvent;
_cooperativeGesturesControl: CooperativeGestureControl;
_showTileBoundaries: boolean;
_showCollisionBoxes: boolean;
_showPadding: boolean;
Expand Down Expand Up @@ -583,7 +565,6 @@ export class Map extends Camera {

this._interactive = options.interactive;
this._cooperativeGestures = options.cooperativeGestures;
this._metaKey = navigator.platform.indexOf('Mac') === 0 ? 'metaKey' : 'ctrlKey';
this._maxTileCacheSize = options.maxTileCacheSize;
this._maxTileCacheZoomLevels = options.maxTileCacheZoomLevels;
this._failIfMajorPerformanceCaveat = options.failIfMajorPerformanceCaveat;
Expand Down Expand Up @@ -656,8 +637,8 @@ export class Map extends Camera {

this.handlers = new HandlerManager(this, options as CompleteMapOptions);

if (this._cooperativeGestures) {
this._setupCooperativeGestures();
if (options.cooperativeGestures) {
this.setCooperativeGestures(options.cooperativeGestures);
}

const hashName = (typeof options.hash === 'string' && options.hash) || undefined;
Expand Down Expand Up @@ -751,7 +732,7 @@ export class Map extends Camera {
const positionContainer = this._controlPositions[position];
if (position.indexOf('bottom') !== -1) {
positionContainer.insertBefore(controlElement, positionContainer.firstChild);
} else {
} else if (position !== 'full') {
positionContainer.appendChild(controlElement);
}
return this;
Expand Down Expand Up @@ -1176,9 +1157,14 @@ export class Map extends Camera {
setCooperativeGestures(gestureOptions?: GestureOptions | boolean | null): Map {
this._cooperativeGestures = gestureOptions;
if (this._cooperativeGestures) {
this._setupCooperativeGestures();
if (this._cooperativeGesturesControl) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a more elegant way instead of remove and then add - simply keep it as is, or call some enable method...?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't come up with better way to handle this without changing public interface of Map class but I'm open to any suggestions.

//remove existing control
this.removeControl(this._cooperativeGesturesControl);
}
this._cooperativeGesturesControl = new CooperativeGestureControl(this._cooperativeGestures);
this.addControl(this._cooperativeGesturesControl);
} else {
this._destroyCooperativeGestures();
this.removeControl(this._cooperativeGesturesControl);
}

return this;
Expand Down Expand Up @@ -2934,45 +2920,12 @@ export class Map extends Camera {
const controlContainer = this._controlContainer = DOM.create('div', 'maplibregl-control-container', container);
const positions = this._controlPositions = {};
['top-left', 'top-right', 'bottom-left', 'bottom-right'].forEach((positionName) => {
positions[positionName] = DOM.create('div', `maplibregl-ctrl-${positionName} `, controlContainer);
positions[positionName] = DOM.create('div', `maplibregl-ctrl-${positionName}`, controlContainer);
});

this._container.addEventListener('scroll', this._onMapScroll, false);
}

_cooperativeGesturesOnWheel = (event: WheelEvent) => {
this._onCooperativeGesture(event, event[this._metaKey], 1);
};

_setupCooperativeGestures() {
const container = this._container;
this._cooperativeGesturesScreen = DOM.create('div', 'maplibregl-cooperative-gesture-screen', container);
let desktopMessage = typeof this._cooperativeGestures !== 'boolean' && this._cooperativeGestures.windowsHelpText ? this._cooperativeGestures.windowsHelpText : 'Use Ctrl + scroll to zoom the map';
if (navigator.platform.indexOf('Mac') === 0) {
desktopMessage = typeof this._cooperativeGestures !== 'boolean' && this._cooperativeGestures.macHelpText ? this._cooperativeGestures.macHelpText : 'Use ⌘ + scroll to zoom the map';
}
const mobileMessage = typeof this._cooperativeGestures !== 'boolean' && this._cooperativeGestures.mobileHelpText ? this._cooperativeGestures.mobileHelpText : 'Use two fingers to move the map';
this._cooperativeGesturesScreen.innerHTML = `
<div class="maplibregl-desktop-message">${desktopMessage}</div>
<div class="maplibregl-mobile-message">${mobileMessage}</div>
`;

// Remove cooperative gesture screen from the accessibility tree since screenreaders cannot interact with the map using gestures
this._cooperativeGesturesScreen.setAttribute('aria-hidden', 'true');

// Add event to canvas container since gesture container is pointer-events: none
this._canvasContainer.addEventListener('wheel', this._cooperativeGesturesOnWheel, false);

// Add a cooperative gestures class (enable touch-action: pan-x pan-y;)
this._canvasContainer.classList.add('maplibregl-cooperative-gestures');
}

_destroyCooperativeGestures() {
DOM.remove(this._cooperativeGesturesScreen);
this._canvasContainer.removeEventListener('wheel', this._cooperativeGesturesOnWheel, false);
this._canvasContainer.classList.remove('maplibregl-cooperative-gestures');
}

_resizeCanvas(width: number, height: number, pixelRatio: number) {
// Request the required canvas size taking the pixelratio into account.
this._canvas.width = Math.floor(pixelRatio * width);
Expand Down Expand Up @@ -3047,17 +3000,6 @@ export class Map extends Camera {
return false;
};

_onCooperativeGesture(event: any, metaPress, touches) {
if (!metaPress && touches < 2) {
// Alert user how to scroll/pan
this._cooperativeGesturesScreen.classList.add('maplibregl-show');
setTimeout(() => {
this._cooperativeGesturesScreen.classList.remove('maplibregl-show');
}, 100);
}
return false;
}

/**
* Returns a Boolean indicating whether the map is fully loaded.
*
Expand Down Expand Up @@ -3285,9 +3227,6 @@ export class Map extends Camera {
this._canvas.removeEventListener('webglcontextlost', this._contextLost, false);
DOM.remove(this._canvasContainer);
DOM.remove(this._controlContainer);
if (this._cooperativeGestures) {
this._destroyCooperativeGestures();
}
this._container.classList.remove('maplibregl-map');

PerformanceUtils.clearMetrics();
Expand Down