Map instance and behavior settings

[paul121]

Why?

Started exploring this idea in #94 - we might not have the need for "multiple maps" on a page, but many of these limitations could be addressed to improve support of "configurable maps" and reusable behaviors.

Many of our current Drupal behaviors define "global" behavior settings in the Drupal.settings.farm_map.behaviors.{behavior_name} JS namespace. When these behaviors are attached, they use these variables to change what they do (hence I'm calling them "settings", could also be considered "state"). Some behaviors also depend on "instance specific" settings in a similar way: Drupal.settings.farm_map.wkt[instance.target].

Also, in farmOS 2.x we've created both "map type" and "map behavior" config entities. Config entities are a great mechanism for providing low-code configuration and would be a great way of providing these map instance and behavior "settings".

What's missing? A standard interface to provide map-instance specific "settings" in farmOS-map! This would have some benefits:

• Each map type (used in different contexts: eg dashboard, plans, reports, etc) can provide its own instance specific "settings".
  • To start, this might only be the options used when creating a map eg: enable/disable controls and interactions. (this is already possible in 2.x actually: https://github.com/farmOS/farmOS/blob/2.x/modules/core/map/js/farm_map.js#L26)
• A map's settings could save "behavior settings" as well eg: settings.behaviors.popup.enabled. This provides a standard place for behaviors and controls to provide settings. It also allows for one behavior to modify another behaviors settings (throughout the lifetime of the map in a page)
• Map behaviors can be become re-usable & decoupled from their application context
  • Possibly pull map behaviors from farmOS server into FK

Example

A good example of both "behavior settings" and "instance settings" is the WKT behavior:

• It checks if zoom is set, and if so, calls instance.zoomToLayer(layer): https://github.com/farmOS/farmOS/blob/42a8a730ad242c62a40095a011318c5c2a2c62b2/modules/farm/farm_map/js/farmOS.map.behaviors.wkt.js#L38.
• A problem with this is that whenever the WKT behavior is included on a page, maps will always use its setting. If another behavior wants to change how the zoom works, it has to alter the WKT settings which requires implicit knowledge of how the WKT behavior works.
• WKT is an exception to this "global" settings pattern, too: it saves the actual WKT data specific to the map instance in a one-off way: https://github.com/farmOS/farmOS/blob/42a8a730ad242c62a40095a011318c5c2a2c62b2/modules/farm/farm_map/js/farmOS.map.behaviors.wkt.js#L6
  • This is why we need "instance specific" settings - currently we can't have multiple instances with different WKT settings.

Implementation

I think we have a few options here...

1. A simple option might be to add instance.getSettings()/instance.setSetting() methods to the farmOS-map instance itself. Perhaps instance.getBehaviorSettings()/instance.setBehaviorSetting() helper methods as well. The settings object could live on the instance here: https://github.com/farmOS/farmOS-map/blob/1.x/src/instance/instance.js#L27

2. A more robust (and even simpler?) option might be to use what's already available to us! The OL Map extends the OL BaseObject which provides the concept of "properties" and relevant methods that we could reuse for this purpose: get(), getKeys(), getProperties(), set(), setProperties(), unset(). Since BaseObject extends Observable each of these properties can be observed as well:

You can add your own observable properties with object.set('prop', 'value'), and retrieve that with object.get('prop'). You can listen for changes on that property value with object.on('change:prop', listener). You can get a list of all properties with module:ol/Object~BaseObject#getProperties.

3. Perhaps a hybrid of both? Leverage the bulk of the logic form instance.map but provide some helper methods on instance?

[paul121]

Something that still needs more thought is the "make behaviors more reusable" piece. I believe that instance + behavior settings are a prerequisite for this and specific behaviors can be tackled separate of this issue, but want to leave one idea I'm pondering: a reusable "zoom" behavior.

There's already an instance.zoom method - the missing piece is allowing multiple behaviors to have control over this. I'm not sure if this is necessary...or if it needs to be a "behavior"... but a common "zoom" setting would at least make this more feasible. And allow for additional options like #95

[symbioquine]

@paul121 I like where this is going - especially the part about making a BehaviorSetting class derived from ol.BaseObject.

Just to play devil's advocate though;

• How are these settings different than the options which one can pass when adding a behavior?
• Do we really need a new "mechanism" in farmOS-map for this? An alternate pattern could be to only use farmOS.map.behaviors for common behaviors that don't require per-instance configuration and instead let the code calling farmOS.map.create decide how to configure the rest of the behaviors by passing options to instance.addBehavior. In farmOS this would probably imply some sort of hook so separate modules could call instance.addBehavior at the right time(s).
• How often do we anticipate settings changing after a map instance is created? Is it reasonable to expect (well mannered) behaviors to watch their settings and update themselves if the setting changes at any time?

[paul121]

How are these settings different than the options which one can pass when adding a behavior?

Good question, I had kind of forgotten about this really. But looking at implementations it seems like it's only used for one thing really, and it's not really a behavior "setting". The only core behaviors that use it are measure and edit to configure the target layer. An example of these behaviors being used in farmOS core code for the movement behavior..

But obviously a layer isn't something that could be hard coded in configuration, these are more like "run time" options. It doesn't seem like "behavior settings" are ever loaded via the options parameter, they use Drupal.settings.farm_map.behavior.* instead - see mapknitter and plan

Interestingly, the core farmOS-map behaviors load units from the "instance options" already (edit example) - which is fairly similar to "instance settings".

Do we really need a new "mechanism" in farmOS-map for this? An alternate pattern could be to only use farmOS.map.behaviors for common behaviors that don't require per-instance configuration and instead let the code calling farmOS.map.create decide how to configure the rest of the behaviors by passing options to instance.addBehavior.

Right, a new "mechanism" probably isn't necessary. Part of the problem is that Drupal modules can't populate the farmOS.map.behaviors namespace directly - all JS data has to go through Drupal.settings.* when it is first loaded on the page. So the question is where do we want to put that? For Drupal, this would be the responsibility of farm_map.js

I think farmOS.map.behaviors has advantages over Drupal.settings, but introducing a new mechanism associated with each instance would be better since it allows for behaviors to be configured per-instance. This would really just be the "recommend" way for behaviors to load their settings - there's no stopping them from loading from settings from any other global JS variable. One valid use case for this method might be a third party API key - it likely doesn't need to be configured per-instance.

In farmOS this would probably imply some sort of hook so separate modules could call instance.addBehavior at the right time(s).

Yes! Instead of a hook there is a MapRenderEvent in 2.x. This has an event.addBehavior(name, settings) function that populates a drupalSettings.farm_map.behaviors.* variable.

Instead of that, I'd like to propose behaviors start populating their settings specific to each map instance from which farm_map.js could populate into the new mechanism when creating a map instance.

How often do we anticipate settings changing after a map instance is created? Is it reasonable to expect (well mannered) behaviors to watch their settings and update themselves if the setting changes at any time?

Yeah, fair question. I suppose this depends on the nature of the setting and behavior... Just as an example, obviously it would be a lot of work to have a control to toggle the units and have that dynamically update an existing popup... but maybe it's reasonable to assume the units setting would be checked each time before creating a popup. If all behaviors using the instance units respected this, then a units toggle control would be pretty feasible to implement.

But looking forward to bringing more capabilities into the map it would be great to have this ability. Not so much changing "settings" over time, but changing the "state" of the map is better way to think of it? The ol.Observable reactivity would be a nice thing to have, but maybe these things would be better implemented in a full blown Vue app.

[symbioquine]

Maybe it's useful to make a distinction between one-off behaviors which target a limited page/purpose vs ones which provide general functionality and are likely to need configuration on a per-page or per-instance basis.

Currently the recommended way to add a behavior is via farmOS.map.behaviors like this;

(function () {
  farmOS.map.behaviors.myMapCustomizations = {
    attach: function (instance) {
        // Do something here - perhaps parameterized on global state or `instance.target` (elem id)
    },
  };
}());

Even with the current API you can have a "glue behavior" which delegates to a more general behavior and passes page/instance specific settings;

(function () {
  farmOS.map.behaviors.myMapCustomizations = {
    attach: function (instance) {
        instance.attachBehavior(window.someOtherBehaviorRegistery.generalAwesomeness, { /* settings here */ });
    },
  };
}());

In my 2.x playground PR, I've simplified this a little by exposing the "registry" where instance.addBehavior finds behaviors by name as farmOS.map.namedBehaviors.

That means that the above example could be simplified to;

(function () {
  farmOS.map.behaviors.myMapCustomizations = {
    attach: function (instance) {
        instance.attachBehavior('generalAwesomeness', { /* settings here */ });
    },
  };
}());

I guess the point I'm trying to make is that the farmOS-map API could already be considered functionally complete - even without my 2.x changes. Behaviors already can be configured at a fine-granularity just by not putting behaviors that need per-instance customization into farmOS.map.behaviors.

I'd argue that this issue should be a farmOS issue about what kind of API/patterns farmOS needs to have for plumbing settings/data gracefully into farmOS-map. New requirements for the farmOS-map API clearly could emerge from there, but I'm not sure it's necessary.