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

Store initial map interaction settings #696

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
5e8899b
working store and restore for map settings
Oct 17, 2017
46b3c0b
first start at storing initial state of map interaction
Oct 17, 2017
84b0e29
Fix function names for store and restore map config, remove argument
alexgleith Nov 14, 2017
816aef3
Ensure that the store only checks if the map context is available. In…
alexgleith Nov 14, 2017
191deb9
Revert "Ensure that the store only checks if the map context is avail…
alexgleith Nov 14, 2017
c5f1d7e
Check for undefined variables, rather than false.
alexgleith Nov 14, 2017
8734927
Ensure that the map and context are valid before checking values.
alexgleith Nov 14, 2017
d3e8ea0
Remove call count checks. These aren't valid.
alexgleith Nov 14, 2017
2acdf05
This call doesn't exist, so this is a bug fix.
alexgleith Nov 14, 2017
1342cad
Handle setting up mocks to better replicate the MapBoxGL object for t…
alexgleith Nov 14, 2017
941039a
Include store functions in list for test.
alexgleith Nov 14, 2017
f0971f2
add another check for store being in context
alexgleith Nov 14, 2017
d8e3c04
Merge branch 'master' into store-initial-map-interaction-settings
mcwhittemore Nov 14, 2017
db77441
Handle constants differently.
alexgleith Nov 14, 2017
9636496
Fix bugs in store functions around setting and getting interactions
alexgleith Nov 14, 2017
65eec5f
Fix an issue with the store's map not being passed in properly.
alexgleith Nov 15, 2017
f534412
Mock the interactions better.
alexgleith Nov 15, 2017
0a44030
Add a set of tests for the three store/restore/check interaction func…
alexgleith Nov 15, 2017
a521ee8
Merge branch 'store-initial-map-interaction-settings' of github.com:a…
alexgleith Nov 15, 2017
3e10847
update yarn.lock
alexgleith Nov 15, 2017
0533188
try a different object iterator to fix travis build issue
alexgleith Nov 15, 2017
52074ea
Merge branch 'master' into store-initial-map-interaction-settings
mcwhittemore Nov 15, 2017
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
14 changes: 14 additions & 0 deletions debug/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
<div id='start-polygon'>POLYGON</div>
</div>
<div class='toggle'>
<button id='doubleClickZoom'>disable dblclick zoom</button>
<button id='addBtn'>add draw</button>
<button id='removeBtn'>remove draw</button>
<button id='flipStyleBtn'>change style</button>
Expand Down Expand Up @@ -86,10 +87,12 @@
map.on('load', function() {

// toggle
var doubleClickZoom = document.getElementById('doubleClickZoom');
var addButton = document.getElementById('addBtn');
var removeButton = document.getElementById('removeBtn');
var flipStyleButton = document.getElementById('flipStyleBtn');
var currentStyle = 'streets-v9';
var doubleClickZoomOn = true;
addButton.onclick = function() {
if (drawIsActive) return;
drawIsActive = true;
Expand All @@ -105,6 +108,17 @@
map.setStyle('mapbox://styles/mapbox/' + style);
currentStyle = style;
}
doubleClickZoom.onclick = function() {
if (doubleClickZoomOn) {
doubleClickZoomOn = false;
map.doubleClickZoom.disable();
doubleClickZoom.innerText = 'enable dblclick zoom'
} else {
map.doubleClickZoom.enable();
doubleClickZoomOn = true;
doubleClickZoom.innerText = 'disable dblclick zoom'
}
}

var startPoint = document.getElementById('start-point');
var startLine = document.getElementById('start-line');
Expand Down
9 changes: 9 additions & 0 deletions src/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,15 @@ module.exports = {
ACTIVE: 'true',
INACTIVE: 'false'
},
interactions: [
'scrollZoom',
'boxZoom',
'dragRotate',
'dragPan',
'keyboard',
'doubleClickZoom',
'touchZoomRotate'
],
LAT_MIN: -90,
LAT_RENDERED_MIN: -85,
LAT_MAX: 90,
Expand Down
6 changes: 5 additions & 1 deletion src/lib/double_click_zoom.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
module.exports = {
enable(ctx) {
setTimeout(() => {
if (!ctx.map || !ctx.map.doubleClickZoom) return;
// First check we've got a map and some context.
if (!ctx.map || !ctx.map.doubleClickZoom || !ctx._ctx || !ctx._ctx.store || !ctx._ctx.store.getInitialConfigValue) return;
// Now check initial state wasn't false (we leave it disabled if so)
if (!ctx._ctx.store.getInitialConfigValue('doubleClickZoom')) return;
ctx.map.doubleClickZoom.enable();
}, 0);
},
disable(ctx) {
setTimeout(() => {
if (!ctx.map || !ctx.map.doubleClickZoom) return;
// Always disable here, as it's necessary in some cases.
ctx.map.doubleClickZoom.disable();
}, 0);
}
Expand Down
3 changes: 3 additions & 0 deletions src/setup.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ module.exports = function(ctx) {
const setup = {
onRemove: function() {
setup.removeLayers();
ctx.store.restoreMapConfig();
ctx.ui.removeButtons();
ctx.events.removeEventListeners();
ctx.map = null;
Expand All @@ -28,6 +29,7 @@ module.exports = function(ctx) {
ctx.container = map.getContainer();
ctx.store = new Store(ctx);


controlContainer = ctx.ui.addButtons();

if (ctx.options.boxSelect) {
Expand All @@ -44,6 +46,7 @@ module.exports = function(ctx) {
map.off('load', connect);
clearInterval(intervalId);
setup.addLayers();
ctx.store.storeMapConfig();
ctx.events.addEventListeners();
};

Expand Down
44 changes: 44 additions & 0 deletions src/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ const throttle = require('./lib/throttle');
const toDenseArray = require('./lib/to_dense_array');
const StringSet = require('./lib/string_set');
const render = require('./render');
const interactions = require('./constants').interactions;

const Store = module.exports = function(ctx) {
this._features = {};
Expand All @@ -11,6 +12,7 @@ const Store = module.exports = function(ctx) {
this._changedFeatureIds = new StringSet();
this._deletedFeaturesToEmit = [];
this._emitSelectionChange = false;
this._mapInitialConfig = {};
this.ctx = ctx;
this.sources = {
hot: [],
Expand Down Expand Up @@ -293,3 +295,45 @@ function refreshSelectedCoordinates(options) {
}
this._selectedCoordinates = newSelectedCoordinates;
}

/**
* Stores the initial config for a map, so that we can set it again after we're done.
*/
Store.prototype.storeMapConfig = function() {
interactions.forEach((interaction) => {
const interactionSet = this.ctx.map[interaction];
if (interactionSet) {
this._mapInitialConfig[interaction] = this.ctx.map[interaction].isEnabled();
}
});
};

/**
* Restores the initial config for a map, ensuring all is well.
*/
Store.prototype.restoreMapConfig = function() {
Object.keys(this._mapInitialConfig).forEach(key => {
const value = this._mapInitialConfig[key];
if (value) {
this.ctx.map[key].enable();
} else {
this.ctx.map[key].disable();
}
});
};

/**
* Returns the initial state of an interaction setting.
* @param {string} interaction
* @return {boolean} `true` if the interaction is enabled, `false` if not.
* Defaults to `true`. (Todo: include defaults.)
*/
Store.prototype.getInitialConfigValue = function(interaction) {
if (this._mapInitialConfig[interaction] !== undefined) {
return this._mapInitialConfig[interaction];
} else {
// This needs to be set to whatever the default is for that interaction
// It seems to be true for all cases currently, so let's send back `true`.
return true;
}
};
5 changes: 1 addition & 4 deletions test/draw_line_string.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,7 @@ test('draw_line_string stop with invalid line', t => {
], 'store.delete received correct arguments');
}

setTimeout(() => {
t.equal(context.map.doubleClickZoom.enable.callCount, 1);
t.end();
}, 10);
t.end();
});

test('draw_line_string render active line with 0 coordinates', t => {
Expand Down
5 changes: 1 addition & 4 deletions test/draw_polygon.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,7 @@ test('draw_polygon stop with invalid polygon', t => {
{ silent: true }
], 'store.delete received correct arguments');

setTimeout(() => {
t.equal(context.map.doubleClickZoom.enable.callCount, 1);
t.end();
}, 10);
t.end();
});

test('draw_polygon render active polygon with no coordinates', t => {
Expand Down
2 changes: 1 addition & 1 deletion test/static.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ test('static', t => {

const map = createMap();
spy(map, 'fire');
map.dragPan.disable.restore();
map.dragPan.disable();
spy(map.dragPan, 'disable');

const opts = {
Expand Down
26 changes: 24 additions & 2 deletions test/store.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import createMap from './utils/create_map';

function createStore() {
const map = createMap();
const ctx = { map };
const ctx = { map: map };
return new Store(ctx);
}

Expand All @@ -27,6 +27,7 @@ test('Store constructor and public API', t => {
cold: []
}, 'exposes store.sources');
t.equal(store.ctx, ctx, 'exposes store.ctx');
t.equal(store.ctx.map, map, 'exposes store.ctx.map');
t.equal(store.isDirty, false, 'exposes store.isDirty');
t.equal(typeof store.render, 'function', 'exposes store.render');

Expand Down Expand Up @@ -54,8 +55,11 @@ test('Store constructor and public API', t => {
t.equal(typeof Store.prototype.getSelectedCoordinates, 'function', 'exposes store.getSelectedCoordinates');
t.equal(typeof Store.prototype.clearSelectedCoordinates, 'function', 'exposes store.clearSelectedCoordinates');
t.equal(typeof Store.prototype.setFeatureProperty, 'function', 'exposes store.setFeatureProperty');
t.equal(typeof Store.prototype.storeMapConfig, 'function', 'exposes store.storeMapConfig');
t.equal(typeof Store.prototype.restoreMapConfig, 'function', 'exposes store.restoreMapConfig');
t.equal(typeof Store.prototype.getInitialConfigValue, 'function', 'exposes store.getInitialConfigValue');

t.equal(getPublicMemberKeys(Store.prototype).length, 21, 'no untested prototype members');
t.equal(getPublicMemberKeys(Store.prototype).length, 24, 'no untested prototype members');

t.end();
});
Expand Down Expand Up @@ -254,3 +258,21 @@ test('Store#setFeatureProperty', t => {
t.end();
});

test('Store#storeAndRestoreMapConfig', t => {
const map = createMap();
// Disable doubleClickZoom
map.doubleClickZoom.disable();
// Check it's disabled
t.equal(map.doubleClickZoom.isEnabled(), false, 'Disables doubleClickZoom on the map');
const ctx = { map: map };
const store = new Store(ctx);
store.storeMapConfig();
// Check we can get the initial state of it
console.log(store);

Choose a reason for hiding this comment

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

Hi! I noticed this on my test output.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, crap. This is already merged. Should I create another PR to fix?

Copy link
Contributor

Choose a reason for hiding this comment

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

That would be great. Thanks! Can you look at adding no console.log to the eslint config as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This has been done, see: #716

t.equal(store.getInitialConfigValue('doubleClickZoom'), false, 'Retrieves the initial value for the doubleClickZoom');
// Enable it again, byt then use restore to reset the initial state
map.doubleClickZoom.enable();
store.restoreMapConfig();
t.equal(map.doubleClickZoom.isEnabled(), false, 'Restores doubleClickZoom on the map');
t.end();
});
11 changes: 11 additions & 0 deletions test/utils/create_map.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import mapboxgl from 'mapbox-gl-js-mock';
import { interactions } from '../../src/constants';

export default function createMap(mapOptions = {}) {

Expand All @@ -13,6 +14,16 @@ export default function createMap(mapOptions = {}) {
map.getContainer = () => mapOptions.container;
}

// Mock up the interaction functions
interactions.forEach((interaction) => {
map[interaction] = {
enabled: true,
disable: function () { this.enabled = false; },
enable: function () { this.enabled = true; },
isEnabled: function () { return this.enabled; },
};
});

map.getCanvas = function() {
return map.getContainer();
};
Expand Down
37 changes: 18 additions & 19 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -91,12 +91,12 @@
"@turf/helpers" "^3.13.0"
jsts "1.3.0"

"@turf/centroid@^4.0.0":
version "4.1.0"
resolved "https://registry.yarnpkg.com/@turf/centroid/-/centroid-4.1.0.tgz#e5c0a539748e35456d837f87b10515cf785bcfe6"
"@turf/centroid@^5.0.4":
version "5.0.4"
resolved "https://registry.yarnpkg.com/@turf/centroid/-/centroid-5.0.4.tgz#31fcef5c2ca94b234217c7bb779091473358c7ec"
dependencies:
"@turf/helpers" "^4.1.0"
"@turf/meta" "^4.1.0"
"@turf/helpers" "^5.0.4"
"@turf/meta" "^5.0.4"

"@turf/combine@^3.14.0":
version "3.14.0"
Expand All @@ -108,17 +108,19 @@
version "3.13.0"
resolved "https://registry.yarnpkg.com/@turf/helpers/-/helpers-3.13.0.tgz#d06078a1464cf56cdb7ea624ea1e13a71b88b806"

"@turf/helpers@^4.1.0":
version "4.1.0"
resolved "https://registry.yarnpkg.com/@turf/helpers/-/helpers-4.1.0.tgz#013a9c53db6a9386e47c7ab445d93ca2035c4b81"
"@turf/helpers@^5.0.4":
version "5.0.4"
resolved "https://registry.yarnpkg.com/@turf/helpers/-/helpers-5.0.4.tgz#e47a4e4f38dee3b47a3177a69de162d7a7a5837f"

"@turf/meta@^3.14.0":
version "3.14.0"
resolved "https://registry.yarnpkg.com/@turf/meta/-/meta-3.14.0.tgz#8d3050c1a0f44bf406a633b6bd28c510f7bcee27"

"@turf/meta@^4.1.0":
version "4.1.0"
resolved "https://registry.yarnpkg.com/@turf/meta/-/meta-4.1.0.tgz#7b0715832ff483d28d2669051f1e00c6cefcbf75"
"@turf/meta@^5.0.4":
version "5.0.4"
resolved "https://registry.yarnpkg.com/@turf/meta/-/meta-5.0.4.tgz#b41d08f19d2ecc934805b6d713a663abd9f83213"
dependencies:
"@turf/helpers" "^5.0.4"

"@turf/union@^3.0.10":
version "3.14.0"
Expand Down Expand Up @@ -390,7 +392,7 @@ babel-code-frame@^6.22.0:
esutils "^2.0.2"
js-tokens "^3.0.0"

babel-core@^6.0.14, babel-core@^6.24.1, babel-core@^6.9.1:
babel-core@^6.24.1, babel-core@^6.9.1:
version "6.24.1"
resolved "https://registry.yarnpkg.com/babel-core/-/babel-core-6.24.1.tgz#8c428564dce1e1f41fb337ec34f4c3b022b5ad83"
dependencies:
Expand Down Expand Up @@ -793,12 +795,9 @@ babel-types@^6.18.0, babel-types@^6.19.0, babel-types@^6.23.0, babel-types@^6.24
lodash "^4.2.0"
to-fast-properties "^1.0.1"

babelify@^7.2.0:
version "7.3.0"
resolved "https://registry.yarnpkg.com/babelify/-/babelify-7.3.0.tgz#aa56aede7067fd7bd549666ee16dc285087e88e5"
dependencies:
babel-core "^6.0.14"
object-assign "^4.0.0"
babelify@^8.0.0:
version "8.0.0"
resolved "https://registry.yarnpkg.com/babelify/-/babelify-8.0.0.tgz#6f60f5f062bfe7695754ef2403b842014a580ed3"

babylon@^6.11.0, babylon@^6.15.0, babylon@^6.17.0:
version "6.17.0"
Expand Down Expand Up @@ -3334,7 +3333,7 @@ oauth-sign@~0.8.1:
version "0.8.2"
resolved "https://registry.yarnpkg.com/oauth-sign/-/oauth-sign-0.8.2.tgz#46a6ab7f0aead8deae9ec0565780b7d4efeb9d43"

object-assign@^4.0.0, object-assign@^4.0.1, object-assign@^4.1.0:
object-assign@^4.0.1, object-assign@^4.1.0:
version "4.1.1"
resolved "https://registry.yarnpkg.com/object-assign/-/object-assign-4.1.1.tgz#2109adc7965887cfc05cbbd442cac8bfbb360863"

Expand Down