From 969486a7d15fc4382ad05a73be6964a8dda92ed9 Mon Sep 17 00:00:00 2001 From: Stepan Kuzmin Date: Tue, 30 Jul 2024 15:48:48 +0300 Subject: [PATCH 1/7] Refactor events --- src/api.js | 34 +++++++------ src/modes/mode_interface_accessors.js | 4 +- src/store.js | 11 ++++- test/api.test.js | 4 +- test/interaction_events.test.js | 70 +++++++++++++++++++++++++++ 5 files changed, 102 insertions(+), 21 deletions(-) diff --git a/src/api.js b/src/api.js index 0a99dc406..aa1bd3fcd 100644 --- a/src/api.js +++ b/src/api.js @@ -21,26 +21,28 @@ const featureTypes = { }; export default function(ctx, api) { - api.modes = Constants.modes; + // API doesn't emit events by default + const silent = ctx.options.silent !== undefined ? !!ctx.options.silent : true; + api.getFeatureIdsAt = function(point) { const features = featuresAt.click({ point }, null, ctx); return features.map(feature => feature.properties.id); }; - api.getSelectedIds = function () { + api.getSelectedIds = function() { return ctx.store.getSelectedIds(); }; - api.getSelected = function () { + api.getSelected = function() { return { type: Constants.geojsonTypes.FEATURE_COLLECTION, features: ctx.store.getSelectedIds().map(id => ctx.store.get(id)).map(feature => feature.toGeoJSON()) }; }; - api.getSelectedPoints = function () { + api.getSelectedPoints = function() { return { type: Constants.geojsonTypes.FEATURE_COLLECTION, features: ctx.store.getSelectedCoordinates().map(coordinate => ({ @@ -72,7 +74,7 @@ export default function(ctx, api) { return newIds; }; - api.add = function (geojson) { + api.add = function(geojson) { const featureCollection = JSON.parse(JSON.stringify(normalize(geojson))); const ids = featureCollection.features.map((feature) => { @@ -89,7 +91,7 @@ export default function(ctx, api) { throw new Error(`Invalid geometry type: ${feature.geometry.type}.`); } const internalFeature = new Model(ctx, feature); - ctx.store.add(internalFeature); + ctx.store.add(internalFeature, { silent }); } else { // If a feature of that id has already been created, and we are swapping it out ... const internalFeature = ctx.store.get(feature.id); @@ -110,7 +112,7 @@ export default function(ctx, api) { }; - api.get = function (id) { + api.get = function(id) { const feature = ctx.store.get(id); if (feature) { return feature.toGeoJSON(); @@ -125,11 +127,11 @@ export default function(ctx, api) { }; api.delete = function(featureIds) { - ctx.store.delete(featureIds, { silent: true }); + ctx.store.delete(featureIds, { silent }); // If we were in direct select mode and our selected feature no longer exists // (because it was deleted), we need to get out of that mode. if (api.getMode() === Constants.modes.DIRECT_SELECT && !ctx.store.getSelectedIds().length) { - ctx.events.changeMode(Constants.modes.SIMPLE_SELECT, undefined, { silent: true }); + ctx.events.changeMode(Constants.modes.SIMPLE_SELECT, undefined, { silent }); } else { ctx.store.render(); } @@ -138,11 +140,11 @@ export default function(ctx, api) { }; api.deleteAll = function() { - ctx.store.delete(ctx.store.getAllIds(), { silent: true }); + ctx.store.delete(ctx.store.getAllIds(), { silent }); // If we were in direct select mode, now our selected feature no longer exists, // so escape that mode. if (api.getMode() === Constants.modes.DIRECT_SELECT) { - ctx.events.changeMode(Constants.modes.SIMPLE_SELECT, undefined, { silent: true }); + ctx.events.changeMode(Constants.modes.SIMPLE_SELECT, undefined, { silent }); } else { ctx.store.render(); } @@ -156,7 +158,7 @@ export default function(ctx, api) { if (stringSetsAreEqual((modeOptions.featureIds || []), ctx.store.getSelectedIds())) return api; // And if we are changing the selection within simple_select mode, just change the selection, // instead of stopping and re-starting the mode - ctx.store.setSelected(modeOptions.featureIds, { silent: true }); + ctx.store.setSelected(modeOptions.featureIds, { silent }); ctx.store.render(); return api; } @@ -166,7 +168,7 @@ export default function(ctx, api) { return api; } - ctx.events.changeMode(mode, modeOptions, { silent: true }); + ctx.events.changeMode(mode, modeOptions, { silent }); return api; }; @@ -175,17 +177,17 @@ export default function(ctx, api) { }; api.trash = function() { - ctx.events.trash({ silent: true }); + ctx.events.trash({ silent }); return api; }; api.combineFeatures = function() { - ctx.events.combineFeatures({ silent: true }); + ctx.events.combineFeatures({ silent }); return api; }; api.uncombineFeatures = function() { - ctx.events.uncombineFeatures({ silent: true }); + ctx.events.uncombineFeatures({ silent }); return api; }; diff --git a/src/modes/mode_interface_accessors.js b/src/modes/mode_interface_accessors.js index 1aeeb2669..ee5eed86f 100644 --- a/src/modes/mode_interface_accessors.js +++ b/src/modes/mode_interface_accessors.js @@ -107,8 +107,8 @@ ModeInterface.prototype.deleteFeature = function(id, opts = {}) { * @name this.addFeature * @param {DrawFeature} feature - the feature to add */ -ModeInterface.prototype.addFeature = function(feature) { - return this._ctx.store.add(feature); +ModeInterface.prototype.addFeature = function(feature, opts = {}) { + return this._ctx.store.add(feature, opts); }; /** diff --git a/src/store.js b/src/store.js index 8b649d529..d32a41c3d 100644 --- a/src/store.js +++ b/src/store.js @@ -117,13 +117,22 @@ Store.prototype.getAllIds = function() { /** * Adds a feature to the store. * @param {Object} feature + * @param {Object} [options] + * @param {Object} [options.silent] - If `true`, this invocation will not fire an event. * * @return {Store} this */ -Store.prototype.add = function(feature) { +Store.prototype.add = function(feature, options = {}) { this.featureChanged(feature.id); this._features[feature.id] = feature; this._featureIds.add(feature.id); + + if (options.silent != null && options.silent === false) { + this.ctx.map.fire(Constants.events.CREATE, { + features: [this._features[feature.id].toGeoJSON()] + }); + } + return this; }; diff --git a/test/api.test.js b/test/api.test.js index 34388a656..5f4a1c0e6 100644 --- a/test/api.test.js +++ b/test/api.test.js @@ -139,11 +139,11 @@ test('Draw.set', () => { 'Draw.add called with new collection'); assert.equal(deleteSpy.callCount, 1, 'Draw.delete called'); - assert.deepEqual(deleteSpy.getCall(0).args, [[ + assert.deepEqual(deleteSpy.getCall(0).args[0], [ pointId, lineId, polygonId - ]], 'Draw.delete called with old features'); + ], 'Draw.delete called with old features'); // Then set to another that contains a feature // with an already-used id diff --git a/test/interaction_events.test.js b/test/interaction_events.test.js index 9e8b8d513..0c245681b 100644 --- a/test/interaction_events.test.js +++ b/test/interaction_events.test.js @@ -982,3 +982,73 @@ test('ensure user interactions fire right events', async (t) => { assert.deepEqual(flushDrawEvents(), [], 'no unexpected draw events'); }); }); + +test('ensure API fire right events', async () => { + const container = document.createElement('div'); + document.body.appendChild(container); + const map = createMap({ container }); + const fireSpy = spy(map, 'fire'); + const afterNextRender = setupAfterNextRender(map); + const Draw = new MapboxDraw({ silent: false }); + + map.addControl(Draw); + await map.on('load'); + + document.body.removeChild(container); + + function flushDrawEvents() { + const drawEvents = []; + for (let i = 0; i < fireSpy.callCount; i++) { + const eventName = fireSpy.getCall(i).args[0]; + if (typeof eventName !== 'string' || eventName.indexOf('draw.') !== 0) continue; + // Ignore draw.render events for now + if (eventName === 'draw.render') continue; + // Ignore draw.actionable events for now + if (eventName === 'draw.actionable') continue; + drawEvents.push(eventName); + } + fireSpy.resetHistory(); + return drawEvents; + } + + Draw.add({ + type: 'Feature', + id: 'point', + properties: {}, + geometry: { + type: 'Point', + coordinates: [10, 10] + } + }); + + Draw.deleteAll(); + + await afterNextRender(); + + Draw.add({ + type: 'Feature', + id: 'line', + properties: {}, + geometry: { + type: 'LineString', + coordinates: [[10, 10], [20, 20]] + } + }); + + await afterNextRender(); + + Draw.changeMode('draw_point'); + Draw.changeMode('draw_line_string'); + Draw.changeMode('draw_polygon'); + Draw.changeMode('simple_select'); + + await afterNextRender(); + + Draw.delete('line'); + + await afterNextRender(); + + assert.deepEqual(flushDrawEvents(), [ + 'draw.create', 'draw.delete', 'draw.create', 'draw.modechange', 'draw.modechange', 'draw.modechange', 'draw.modechange', 'draw.delete' + ], 'no unexpected draw events'); +}); From 0d536f9004455281503f42052ad37fdca237f701 Mon Sep 17 00:00:00 2001 From: Stepan Kuzmin Date: Tue, 30 Jul 2024 18:03:17 +0300 Subject: [PATCH 2/7] cleanup --- test/interaction_events.test.js | 77 ++++++++++++++++++--------------- 1 file changed, 41 insertions(+), 36 deletions(-) diff --git a/test/interaction_events.test.js b/test/interaction_events.test.js index 0c245681b..0389dc2df 100644 --- a/test/interaction_events.test.js +++ b/test/interaction_events.test.js @@ -983,12 +983,13 @@ test('ensure user interactions fire right events', async (t) => { }); }); -test('ensure API fire right events', async () => { +test('ensure API fire right events', { only: true }, async (t) => { const container = document.createElement('div'); document.body.appendChild(container); - const map = createMap({ container }); + const map = createMap({container}); const fireSpy = spy(map, 'fire'); - const afterNextRender = setupAfterNextRender(map); + + // Explicitly set silent to false to ensure events are fired const Draw = new MapboxDraw({ silent: false }); map.addControl(Draw); @@ -996,22 +997,7 @@ test('ensure API fire right events', async () => { document.body.removeChild(container); - function flushDrawEvents() { - const drawEvents = []; - for (let i = 0; i < fireSpy.callCount; i++) { - const eventName = fireSpy.getCall(i).args[0]; - if (typeof eventName !== 'string' || eventName.indexOf('draw.') !== 0) continue; - // Ignore draw.render events for now - if (eventName === 'draw.render') continue; - // Ignore draw.actionable events for now - if (eventName === 'draw.actionable') continue; - drawEvents.push(eventName); - } - fireSpy.resetHistory(); - return drawEvents; - } - - Draw.add({ + const point = { type: 'Feature', id: 'point', properties: {}, @@ -1019,13 +1005,9 @@ test('ensure API fire right events', async () => { type: 'Point', coordinates: [10, 10] } - }); - - Draw.deleteAll(); - - await afterNextRender(); + }; - Draw.add({ + const line = { type: 'Feature', id: 'line', properties: {}, @@ -1033,22 +1015,45 @@ test('ensure API fire right events', async () => { type: 'LineString', coordinates: [[10, 10], [20, 20]] } + }; + + await t.test('Draw#add fires draw.create event', async () => { + Draw.add(point); + assert.strictEqual(fireSpy.lastCall.firstArg, 'draw.create'); + assert.deepStrictEqual(fireSpy.lastCall.lastArg, {features: [point]}); + + Draw.add(line); + assert.strictEqual(fireSpy.lastCall.firstArg, 'draw.create'); + assert.deepStrictEqual(fireSpy.lastCall.lastArg, {features: [line]}); }); - await afterNextRender(); + await t.test('Draw#delete fires draw.delete event', async () => { + Draw.delete(point.id); + assert.strictEqual(fireSpy.lastCall.firstArg, 'draw.delete'); + assert.deepStrictEqual(fireSpy.lastCall.lastArg, {features: [point]}); + }); - Draw.changeMode('draw_point'); - Draw.changeMode('draw_line_string'); - Draw.changeMode('draw_polygon'); - Draw.changeMode('simple_select'); + await t.test('Draw#deleteAll fires draw.delete event', async () => { + Draw.deleteAll(); + assert.strictEqual(fireSpy.lastCall.firstArg, 'draw.delete'); + assert.deepStrictEqual(fireSpy.lastCall.lastArg, {features: [line]}); + }); - await afterNextRender(); + await t.test('Draw#changeMode fires draw.modechange event', async () => { + Draw.changeMode('draw_point'); + assert.strictEqual(fireSpy.lastCall.firstArg, 'draw.modechange', 'Draw.changeMode triggers draw.modechange event'); + assert.deepStrictEqual(fireSpy.lastCall.lastArg, { mode: 'draw_point' }, 'Draw.changeMode triggers draw.modechange event with correct data'); - Draw.delete('line'); + Draw.changeMode('draw_line_string'); + assert.strictEqual(fireSpy.lastCall.firstArg, 'draw.modechange', 'Draw.changeMode triggers draw.modechange event'); + assert.deepStrictEqual(fireSpy.lastCall.lastArg, { mode: 'draw_line_string' }, 'Draw.changeMode triggers draw.modechange event with correct data'); - await afterNextRender(); + Draw.changeMode('draw_polygon'); + assert.strictEqual(fireSpy.lastCall.firstArg, 'draw.modechange', 'Draw.changeMode triggers draw.modechange event'); + assert.deepStrictEqual(fireSpy.lastCall.lastArg, { mode: 'draw_polygon' }, 'Draw.changeMode triggers draw.modechange event with correct data'); - assert.deepEqual(flushDrawEvents(), [ - 'draw.create', 'draw.delete', 'draw.create', 'draw.modechange', 'draw.modechange', 'draw.modechange', 'draw.modechange', 'draw.delete' - ], 'no unexpected draw events'); + Draw.changeMode('simple_select'); + assert.strictEqual(fireSpy.lastCall.firstArg, 'draw.modechange', 'Draw.changeMode triggers draw.modechange event'); + assert.deepStrictEqual(fireSpy.lastCall.lastArg, { mode: 'simple_select' }, 'Draw.changeMode triggers draw.modechange event with correct data'); + }); }); From 46ece49ae2dcd65e49bd2e1d3f9948594fb1f569 Mon Sep 17 00:00:00 2001 From: Stepan Kuzmin Date: Tue, 30 Jul 2024 18:38:11 +0300 Subject: [PATCH 3/7] Draw#setFeatureProperty fires draw.update event --- src/api.js | 2 +- src/constants.js | 1 + src/store.js | 11 ++++++-- test/interaction_events.test.js | 47 +++++++++++++++++++++++++++++++++ 4 files changed, 58 insertions(+), 3 deletions(-) diff --git a/src/api.js b/src/api.js index aa1bd3fcd..4936b9eb4 100644 --- a/src/api.js +++ b/src/api.js @@ -192,7 +192,7 @@ export default function(ctx, api) { }; api.setFeatureProperty = function(featureId, property, value) { - ctx.store.setFeatureProperty(featureId, property, value); + ctx.store.setFeatureProperty(featureId, property, value, { silent }); return api; }; diff --git a/src/constants.js b/src/constants.js index 655d16184..581e0e57f 100644 --- a/src/constants.js +++ b/src/constants.js @@ -68,6 +68,7 @@ export const events = { export const updateActions = { MOVE: 'move', + CHANGE_PROPERTIES: 'change_properties', CHANGE_COORDINATES: 'change_coordinates' }; diff --git a/src/store.js b/src/store.js index d32a41c3d..e87e8250e 100644 --- a/src/store.js +++ b/src/store.js @@ -128,7 +128,7 @@ Store.prototype.add = function(feature, options = {}) { this._featureIds.add(feature.id); if (options.silent != null && options.silent === false) { - this.ctx.map.fire(Constants.events.CREATE, { + this.ctx.events.fire(Constants.events.CREATE, { features: [this._features[feature.id].toGeoJSON()] }); } @@ -322,9 +322,16 @@ Store.prototype.isSelected = function(featureId) { * @param {string} property property * @param {string} property value */ -Store.prototype.setFeatureProperty = function(featureId, property, value) { +Store.prototype.setFeatureProperty = function(featureId, property, value, options) { this.get(featureId).setProperty(property, value); this.featureChanged(featureId); + + if (options.silent != null && options.silent === false) { + this.ctx.events.fire(Constants.events.UPDATE, { + action: Constants.updateActions.CHANGE_PROPERTIES, + features: [this.get(featureId).toGeoJSON()] + }); + } }; function refreshSelectedCoordinates(store, options) { diff --git a/test/interaction_events.test.js b/test/interaction_events.test.js index 0389dc2df..a14271fd5 100644 --- a/test/interaction_events.test.js +++ b/test/interaction_events.test.js @@ -1017,6 +1017,10 @@ test('ensure API fire right events', { only: true }, async (t) => { } }; + t.afterEach(() => { + fireSpy.resetHistory(); + }); + await t.test('Draw#add fires draw.create event', async () => { Draw.add(point); assert.strictEqual(fireSpy.lastCall.firstArg, 'draw.create'); @@ -1039,6 +1043,49 @@ test('ensure API fire right events', { only: true }, async (t) => { assert.deepStrictEqual(fireSpy.lastCall.lastArg, {features: [line]}); }); + await t.test('Draw#set fires draw.create event', async () => { + const collection = { + type: 'FeatureCollection', + features: [point, line] + }; + + Draw.set(collection); + + assert.strictEqual(fireSpy.callCount, 2, 'fires draw.create event for each feature'); + + assert.strictEqual(fireSpy.firstCall.firstArg, 'draw.create'); + assert.deepStrictEqual(fireSpy.firstCall.lastArg, {features: [point]}); + + assert.strictEqual(fireSpy.lastCall.firstArg, 'draw.create'); + assert.deepStrictEqual(fireSpy.lastCall.lastArg, {features: [line]}); + }); + + await t.test('Draw#set fires draw.delete event', async () => { + const collection = { + type: 'FeatureCollection', + features: [line] + }; + + Draw.set(collection); + + assert.strictEqual(fireSpy.callCount, 1, 'fires draw.delete event for deleted feature'); + + assert.strictEqual(fireSpy.lastCall.firstArg, 'draw.delete'); + assert.deepStrictEqual(fireSpy.lastCall.lastArg, {features: [point]}); + }); + + await t.test('Draw#setFeatureProperty fires draw.update event', () => { + Draw.add(point); + + Draw.setFeatureProperty(point.id, 'price', 200); + + assert.strictEqual(fireSpy.lastCall.firstArg, 'draw.update'); + assert.deepStrictEqual(fireSpy.lastCall.lastArg, { + action: 'change_properties', + features: [{...point, properties: {price: 200}}] + }); + }); + await t.test('Draw#changeMode fires draw.modechange event', async () => { Draw.changeMode('draw_point'); assert.strictEqual(fireSpy.lastCall.firstArg, 'draw.modechange', 'Draw.changeMode triggers draw.modechange event'); From 27f387cc8122d2c0e558c2f0cb5008f7d0f5a799 Mon Sep 17 00:00:00 2001 From: Stepan Kuzmin Date: Wed, 31 Jul 2024 12:22:29 +0300 Subject: [PATCH 4/7] hotfix --- src/store.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/store.js b/src/store.js index e87e8250e..01b7d5c4e 100644 --- a/src/store.js +++ b/src/store.js @@ -322,7 +322,7 @@ Store.prototype.isSelected = function(featureId) { * @param {string} property property * @param {string} property value */ -Store.prototype.setFeatureProperty = function(featureId, property, value, options) { +Store.prototype.setFeatureProperty = function(featureId, property, value, options = {}) { this.get(featureId).setProperty(property, value); this.featureChanged(featureId); @@ -334,7 +334,7 @@ Store.prototype.setFeatureProperty = function(featureId, property, value, option } }; -function refreshSelectedCoordinates(store, options) { +function refreshSelectedCoordinates(store, options = {}) { const newSelectedCoordinates = store._selectedCoordinates.filter(point => store._selectedFeatureIds.has(point.feature_id)); if (store._selectedCoordinates.length !== newSelectedCoordinates.length && !options.silent) { store._emitSelectionChange = true; From 3108a243a453f7d8cc60e928fabfd14671b59944 Mon Sep 17 00:00:00 2001 From: Stepan Kuzmin Date: Wed, 31 Jul 2024 13:21:47 +0300 Subject: [PATCH 5/7] Emit events in Store#featureChanged --- docs/API.md | 5 +++-- src/api.js | 2 +- src/feature_types/feature.js | 8 ++++---- src/options.js | 3 ++- src/store.js | 34 ++++++++++++++++++++------------- test/feature.test.js | 4 +--- test/interaction_events.test.js | 2 +- test/options.test.js | 18 +++++++++++------ test/store.test.js | 3 +++ 9 files changed, 48 insertions(+), 31 deletions(-) diff --git a/docs/API.md b/docs/API.md index 79695e808..0a2dcce99 100644 --- a/docs/API.md +++ b/docs/API.md @@ -36,6 +36,7 @@ All of the following options are optional. - `modes`, Object: over ride the default modes with your own. `MapboxDraw.modes` can be used to see the default values. More information on custom modes [can be found here](https://github.com/mapbox/mapbox-gl-draw/blob/main/docs/MODES.md). - `defaultMode`, String (default: `'simple_select'`): the mode (from `modes`) that user will first land in. - `userProperties`, boolean (default: `false`): properties of a feature will also be available for styling and prefixed with `user_`, e.g., `['==', 'user_custom_label', 'Example']` +- `silent`, boolean (default: `true`): Whether or not to emit events when calling Draw API methods. If `false`, events will be emitted. ## Modes @@ -449,7 +450,7 @@ This event will *not* fire when a feature is created or deleted. To track those The event data is an object with the following shape: ```js -{ +{ features: Array, // Array of features that were updated action: string // Name of the action that triggered the update } @@ -493,7 +494,7 @@ This event is fired just after the current mode stops and just before the next m The event data is an object with the following shape: ```js -{ +{ mode: string // The next mode, i.e. the mode that Draw is changing to } ``` diff --git a/src/api.js b/src/api.js index 4936b9eb4..d2b87d3f8 100644 --- a/src/api.js +++ b/src/api.js @@ -98,7 +98,7 @@ export default function(ctx, api) { const originalProperties = internalFeature.properties; internalFeature.properties = feature.properties; if (!isEqual(originalProperties, feature.properties)) { - ctx.store.featureChanged(internalFeature.id); + ctx.store.featureChanged(internalFeature.id, { silent }); } if (!isEqual(internalFeature.getCoordinates(), feature.geometry.coordinates)) { internalFeature.incomingCoords(feature.geometry.coordinates); diff --git a/src/feature_types/feature.js b/src/feature_types/feature.js index 0d5c6b21e..d8171d744 100644 --- a/src/feature_types/feature.js +++ b/src/feature_types/feature.js @@ -9,17 +9,17 @@ const Feature = function(ctx, geojson) { this.type = geojson.geometry.type; }; -Feature.prototype.changed = function() { - this.ctx.store.featureChanged(this.id); +Feature.prototype.changed = function(options = {}) { + this.ctx.store.featureChanged(this.id, options); }; Feature.prototype.incomingCoords = function(coords) { this.setCoordinates(coords); }; -Feature.prototype.setCoordinates = function(coords) { +Feature.prototype.setCoordinates = function(coords, options = {}) { this.coordinates = coords; - this.changed(); + this.changed(options); }; Feature.prototype.getCoordinates = function() { diff --git a/src/options.js b/src/options.js index 0f81b9158..d98642091 100644 --- a/src/options.js +++ b/src/options.js @@ -14,7 +14,8 @@ const defaultOptions = { styles, modes, controls: {}, - userProperties: false + userProperties: false, + silent: true }; const showControls = { diff --git a/src/store.js b/src/store.js index 01b7d5c4e..aa7fca925 100644 --- a/src/store.js +++ b/src/store.js @@ -84,8 +84,14 @@ Store.prototype.setDirty = function() { * @param {string} featureId * @return {Store} this */ -Store.prototype.featureChanged = function(featureId) { +Store.prototype.featureChanged = function(featureId, options = {}) { this._changedFeatureIds.add(featureId); + + const silent = options.silent != null ? options.silent : this.ctx.options.silent; + if (silent !== true && options.eventName && options.eventData) { + this.ctx.events.fire(options.eventName, options.eventData); + } + return this; }; @@ -123,16 +129,15 @@ Store.prototype.getAllIds = function() { * @return {Store} this */ Store.prototype.add = function(feature, options = {}) { - this.featureChanged(feature.id); + this.featureChanged(feature.id, { + silent: options.silent, + eventName: Constants.events.CREATE, + eventData: {features: [feature.toGeoJSON()]} + }); + this._features[feature.id] = feature; this._featureIds.add(feature.id); - if (options.silent != null && options.silent === false) { - this.ctx.events.fire(Constants.events.CREATE, { - features: [this._features[feature.id].toGeoJSON()] - }); - } - return this; }; @@ -321,17 +326,20 @@ Store.prototype.isSelected = function(featureId) { * @param {string} featureId * @param {string} property property * @param {string} property value + * @param {Object} [options] + * @param {Object} [options.silent] - If `true`, this invocation will not fire an event. */ Store.prototype.setFeatureProperty = function(featureId, property, value, options = {}) { this.get(featureId).setProperty(property, value); - this.featureChanged(featureId); - if (options.silent != null && options.silent === false) { - this.ctx.events.fire(Constants.events.UPDATE, { + this.featureChanged(featureId, { + silent: options.silent, + eventName: Constants.events.UPDATE, + eventData: { action: Constants.updateActions.CHANGE_PROPERTIES, features: [this.get(featureId).toGeoJSON()] - }); - } + } + }); }; function refreshSelectedCoordinates(store, options = {}) { diff --git a/test/feature.test.js b/test/feature.test.js index 66138168a..01bbfbb71 100644 --- a/test/feature.test.js +++ b/test/feature.test.js @@ -50,9 +50,7 @@ test('Feature#changed', () => { ctx.store.featureChanged.resetHistory(); feature.changed(); assert.equal(ctx.store.featureChanged.callCount, 1, 'called function on store'); - assert.deepEqual(ctx.store.featureChanged.getCall(0).args, [featureGeoJson.id], 'with correct args'); - - + assert.deepEqual(ctx.store.featureChanged.getCall(0).args[0], featureGeoJson.id, 'with correct args'); }); test('Feature#incomingCoords', () => { diff --git a/test/interaction_events.test.js b/test/interaction_events.test.js index a14271fd5..5752b14ab 100644 --- a/test/interaction_events.test.js +++ b/test/interaction_events.test.js @@ -983,7 +983,7 @@ test('ensure user interactions fire right events', async (t) => { }); }); -test('ensure API fire right events', { only: true }, async (t) => { +test('ensure API fire right events', async (t) => { const container = document.createElement('div'); document.body.appendChild(container); const map = createMap({container}); diff --git a/test/options.test.js b/test/options.test.js index a8d34a451..58e2a39a9 100644 --- a/test/options.test.js +++ b/test/options.test.js @@ -32,7 +32,8 @@ test('Options test', async (t) => { trash: true, combine_features: true, uncombine_features: true - } + }, + silent: true }; assert.deepEqual(defaultOptions, Draw.options); assert.deepEqual(styleWithSourcesFixture, Draw.options.styles); @@ -58,7 +59,8 @@ test('Options test', async (t) => { trash: true, combine_features: true, uncombine_features: true - } + }, + silent: true }; assert.deepEqual(defaultOptions, Draw.options); @@ -84,7 +86,8 @@ test('Options test', async (t) => { trash: false, combine_features: false, uncombine_features: false - } + }, + silent: true }; assert.deepEqual(defaultOptions, Draw.options); }); @@ -109,7 +112,8 @@ test('Options test', async (t) => { trash: false, combine_features: false, uncombine_features: false - } + }, + silent: true }; assert.deepEqual(defaultOptions, Draw.options); @@ -135,7 +139,8 @@ test('Options test', async (t) => { trash: true, combine_features: true, uncombine_features: true - } + }, + silent: true }; assert.deepEqual(defaultOptions, Draw.options); @@ -161,7 +166,8 @@ test('Options test', async (t) => { trash: true, combine_features: true, uncombine_features: true - } + }, + silent: true }; assert.deepEqual(defaultOptions, Draw.options); assert.deepEqual(styleWithSourcesFixture, Draw.options.styles); diff --git a/test/store.test.js b/test/store.test.js index e93bf56e9..e5e6c7966 100644 --- a/test/store.test.js +++ b/test/store.test.js @@ -9,6 +9,9 @@ import createMap from './utils/create_map.js'; function createStore() { const ctx = { + options: { + silent: true + }, map: createMap(), events: { fire: spy() From db018edd584ef095e6b623ba48fbfa42203253d9 Mon Sep 17 00:00:00 2001 From: Stepan Kuzmin Date: Wed, 31 Jul 2024 13:45:08 +0300 Subject: [PATCH 6/7] Split Store#featureCreated --- src/feature_types/feature.js | 8 ++++---- src/store.js | 40 +++++++++++++++++++++++------------- test/store.test.js | 3 ++- 3 files changed, 32 insertions(+), 19 deletions(-) diff --git a/src/feature_types/feature.js b/src/feature_types/feature.js index d8171d744..0d5c6b21e 100644 --- a/src/feature_types/feature.js +++ b/src/feature_types/feature.js @@ -9,17 +9,17 @@ const Feature = function(ctx, geojson) { this.type = geojson.geometry.type; }; -Feature.prototype.changed = function(options = {}) { - this.ctx.store.featureChanged(this.id, options); +Feature.prototype.changed = function() { + this.ctx.store.featureChanged(this.id); }; Feature.prototype.incomingCoords = function(coords) { this.setCoordinates(coords); }; -Feature.prototype.setCoordinates = function(coords, options = {}) { +Feature.prototype.setCoordinates = function(coords) { this.coordinates = coords; - this.changed(options); + this.changed(); }; Feature.prototype.getCoordinates = function() { diff --git a/src/store.js b/src/store.js index aa7fca925..bd1f53a4e 100644 --- a/src/store.js +++ b/src/store.js @@ -79,6 +79,25 @@ Store.prototype.setDirty = function() { return this; }; +/** + * Sets a feature's state to changed. + * @param {string} featureId + * @return {Store} this + */ +Store.prototype.featureCreated = function(featureId, options = {}) { + this._changedFeatureIds.add(featureId); + + const silent = options.silent != null ? options.silent : this.ctx.options.silent; + if (silent !== true) { + const feature = this.get(featureId); + this.ctx.events.fire(Constants.events.CREATE, { + features: [feature.toGeoJSON()] + }); + } + + return this; +}; + /** * Sets a feature's state to changed. * @param {string} featureId @@ -88,8 +107,11 @@ Store.prototype.featureChanged = function(featureId, options = {}) { this._changedFeatureIds.add(featureId); const silent = options.silent != null ? options.silent : this.ctx.options.silent; - if (silent !== true && options.eventName && options.eventData) { - this.ctx.events.fire(options.eventName, options.eventData); + if (silent !== true) { + this.ctx.events.fire(Constants.events.UPDATE, { + action: options.action ? options.action : Constants.updateActions.CHANGE_COORDINATES, + features: [this.get(featureId).toGeoJSON()] + }); } return this; @@ -129,15 +151,9 @@ Store.prototype.getAllIds = function() { * @return {Store} this */ Store.prototype.add = function(feature, options = {}) { - this.featureChanged(feature.id, { - silent: options.silent, - eventName: Constants.events.CREATE, - eventData: {features: [feature.toGeoJSON()]} - }); - this._features[feature.id] = feature; this._featureIds.add(feature.id); - + this.featureCreated(feature.id, {silent: options.silent}); return this; }; @@ -334,11 +350,7 @@ Store.prototype.setFeatureProperty = function(featureId, property, value, option this.featureChanged(featureId, { silent: options.silent, - eventName: Constants.events.UPDATE, - eventData: { - action: Constants.updateActions.CHANGE_PROPERTIES, - features: [this.get(featureId).toGeoJSON()] - } + action: Constants.updateActions.CHANGE_PROPERTIES }); }; diff --git a/test/store.test.js b/test/store.test.js index e5e6c7966..41e22d956 100644 --- a/test/store.test.js +++ b/test/store.test.js @@ -46,6 +46,7 @@ test('Store constructor and public API', () => { // prototype members assert.equal(typeof Store.prototype.setDirty, 'function', 'exposes store.setDirty'); assert.equal(typeof Store.prototype.createRenderBatch, 'function', 'exposes store.createRenderBatch'); + assert.equal(typeof Store.prototype.featureCreated, 'function', 'exposes store.featureCreated'); assert.equal(typeof Store.prototype.featureChanged, 'function', 'exposes store.featureChanged'); assert.equal(typeof Store.prototype.getChangedIds, 'function', 'exposes store.getChangedIds'); assert.equal(typeof Store.prototype.clearChangedIds, 'function', 'exposes store.clearChangedIds'); @@ -69,7 +70,7 @@ test('Store constructor and public API', () => { assert.equal(typeof Store.prototype.restoreMapConfig, 'function', 'exposes store.restoreMapConfig'); assert.equal(typeof Store.prototype.getInitialConfigValue, 'function', 'exposes store.getInitialConfigValue'); - assert.equal(getPublicMemberKeys(Store.prototype).length, 24, 'no untested prototype members'); + assert.equal(getPublicMemberKeys(Store.prototype).length, 25, 'no untested prototype members'); }); test('Store#setDirty', () => { From 9a4b3fdb5d24675af7684eeb898680b787711885 Mon Sep 17 00:00:00 2001 From: Stepan Kuzmin Date: Wed, 31 Jul 2024 16:44:05 +0300 Subject: [PATCH 7/7] tag dev alpha --- package-lock.json | 4 ++-- package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index 6fddfd82c..aed93f209 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@mapbox/mapbox-gl-draw", - "version": "1.4.3", + "version": "1.4.4-alpha.db018ed", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@mapbox/mapbox-gl-draw", - "version": "1.4.3", + "version": "1.4.4-alpha.db018ed", "license": "ISC", "dependencies": { "@mapbox/geojson-area": "^0.2.2", diff --git a/package.json b/package.json index 62f51c6e7..b62e5bed6 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@mapbox/mapbox-gl-draw", - "version": "1.4.3", + "version": "1.4.4-alpha.db018ed", "engines": { "node": "^18.0.0 || >=20.0.0" },