From 261b42e67d11ae2a7f4ca725c751900180b56b60 Mon Sep 17 00:00:00 2001 From: David Manthey Date: Wed, 5 Sep 2018 12:22:14 -0400 Subject: [PATCH] Add a selectionAPI='auto' mode for features. This is the new default. If any feature event is bound on the feature, the selectionAPI is enabled. Binding and unbinding feature events will update the selection. The selectionAPI can still be a boolean to directly control the behavior. This resolves #871. --- src/feature.js | 70 ++++++++++++++++++++++++++++++++++++------ tests/cases/feature.js | 22 +++++++++++++ 2 files changed, 83 insertions(+), 9 deletions(-) diff --git a/src/feature.js b/src/feature.js index 3cc178307e..6146fea620 100644 --- a/src/feature.js +++ b/src/feature.js @@ -9,9 +9,11 @@ var geo_event = require('./event'); * * @typedef {object} geo.feature.spec * @property {geo.layer} [layer] the parent layer associated with the feature. - * @property {boolean} [selectionAPI=false] If truthy, enable selection events - * on the feature. Selection events are those in {@link geo.event.feature}. - * They can be bound via a call like + * @property {boolean|'auto'} [selectionAPI='auto'] If `'auto'`, enable + * selection events if any {@link geo.event.feature} events are bound to the + * feature. Otherwise, if truthy, enable selection events on the feature. + * Selection events are those in {@link geo.event.feature}. They can be + * bound via a call like * ``` * feature.geoOn(geo.event.feature.mousemove, function (evt) { * // do something with the feature @@ -111,8 +113,10 @@ var feature = function (arg) { var m_this = this, s_exit = this._exit, + s_geoOn = this.geoOn, + s_geoOff = this.geoOff, m_ready, - m_selectionAPI = arg.selectionAPI === undefined ? false : !!arg.selectionAPI, + m_selectionAPI = arg.selectionAPI === undefined ? 'auto' : arg.selectionAPI, m_style = {}, m_layer = arg.layer === undefined ? null : arg.layer, m_gcs = arg.gcs, @@ -772,22 +776,29 @@ var feature = function (arg) { /** * Get/Set if the selection API is enabled for this feature. * - * @param {boolean} [arg] `undefined` to return the selectionAPI state, or a - * boolean to change the state. + * @param {boolean|string} [arg] `undefined` to return the selectionAPI + * state, a boolean to change the state, or `'auto'` to set the state + * based on the existence of event handlers. When getting the state, if + * `direct` is not specified, `'auto'` is never returned. * @param {boolean} [direct] If `true`, when getting the selectionAPI state, * disregard the state of the parent layer, and when setting, refresh the * state regardless of whether it has changed or not. - * @returns {boolean|this} Either the selectionAPI state (if getting) or the - * feature (if setting). + * @returns {boolean|string|this} Either the selectionAPI state or the + * feature instance. */ this.selectionAPI = function (arg, direct) { if (arg === undefined) { if (!direct && m_layer && m_layer.selectionAPI && !m_layer.selectionAPI()) { return false; } + if (!direct && m_selectionAPI === 'auto') { + return !!m_this.geoIsOn(Object.values(geo_event.feature)); + } return m_selectionAPI; } - arg = !!arg; + if (arg !== 'auto') { + arg = !!arg; + } if (arg !== m_selectionAPI || direct) { m_selectionAPI = arg; m_this._unbindMouseHandlers(); @@ -847,6 +858,47 @@ var feature = function (arg) { this._update = function () { }; + /** + * Bind an event handler to this object. + * + * @param {string} event An event from {@link geo.event} or a user-defined + * value. + * @param {function} handler A function that is called when `event` is + * triggered. The function is passed a {@link geo.event} object. + * @returns {this} + */ + this.geoOn = function (event, handler) { + var isAuto = m_this.selectionAPI(undefined, true) === 'auto', + selection = isAuto && m_this.selectionAPI(); + var result = s_geoOn.apply(m_this, arguments); + if (isAuto && !selection && m_this.selectionAPI()) { + m_this._bindMouseHandlers(); + } + return result; + }; + + /** + * Remove handlers from one event or an array of events. If no event is + * provided all handlers will be removed. + * + * @param {string|string[]} [event] An event or a list of events from + * {@link geo.event} or defined by the user, or `undefined` to remove all + * events (in which case `arg` is ignored). + * @param {(function|function[])?} [arg] A function or array of functions to + * remove from the events or a falsy value to remove all handlers from the + * events. + * @returns {this} + */ + this.geoOff = function (event, arg) { + var isAuto = m_this.selectionAPI(undefined, true) === 'auto', + selection = isAuto && m_this.selectionAPI(); + var result = s_geoOff.apply(m_this, arguments); + if (isAuto && selection && !m_this.selectionAPI()) { + m_this._unbindMouseHandlers(); + } + return result; + }; + /** * Destroy. Unbind mouse handlers, clear internal variables, and call the * parent destroy method. diff --git a/tests/cases/feature.js b/tests/cases/feature.js index 9dd81d5ad1..5882a377cc 100644 --- a/tests/cases/feature.js +++ b/tests/cases/feature.js @@ -59,6 +59,7 @@ describe('geo.feature', function () { feat = geo.feature({layer: layer}); expect(feat.style('opacity')).toBe(1); expect(feat.selectionAPI()).toBe(false); + expect(feat.selectionAPI(undefined, true)).toBe('auto'); }); it('_exit', function () { feat = geo.feature({layer: layer}); @@ -338,6 +339,8 @@ describe('geo.feature', function () { expect(feat.style('data')).toEqual([1]); }); it('selectionAPI', function () { + var bindSpy, unbindSpy; + expect(feat.selectionAPI()).toBe(false); feat = geo.feature({layer: layer, renderer: layer.renderer(), selectionAPI: true}); expect(feat.selectionAPI()).toBe(true); @@ -362,6 +365,25 @@ describe('geo.feature', function () { expect(feat.selectionAPI()).toBe(false); expect(feat.selectionAPI(true, true)).toBe(feat); expect(feat.selectionAPI()).toBe(true); + + // auto will change the selection mode depending on bound events + bindSpy = sinon.spy(feat, '_bindMouseHandlers'); + unbindSpy = sinon.spy(feat, '_unbindMouseHandlers'); + expect(feat.selectionAPI('auto')).toBe(feat); + expect(feat.selectionAPI()).toBe(false); + expect(feat.selectionAPI(undefined, true)).toBe('auto'); + expect(bindSpy.callCount).toBe(1); + expect(unbindSpy.callCount).toBe(1); + feat.geoOn(geo.event.feature.mouseover, function () {}); + expect(feat.selectionAPI()).toBe(true); + expect(feat.selectionAPI(undefined, true)).toBe('auto'); + expect(bindSpy.callCount).toBe(2); + expect(unbindSpy.callCount).toBe(2); + feat.geoOff(geo.event.feature.mouseover); + expect(feat.selectionAPI()).toBe(false); + expect(feat.selectionAPI(undefined, true)).toBe('auto'); + expect(bindSpy.callCount).toBe(2); + expect(unbindSpy.callCount).toBe(3); }); it('ready', function () { feat = geo.feature({layer: layer});