-
Notifications
You must be signed in to change notification settings - Fork 90
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
fix(track): ensure places node list will update when track name changes #1116
base: master
Are you sure you want to change the base?
Conversation
Resolves ngageoint#842 The caller updating the track name (or other values) still has to call track.dispatchEvent(new os.events.PropertyChangeEvent(os.annotation.EventType.UPDATE_PLACEMARK));
@@ -479,6 +480,7 @@ plugin.track.createAndAdd = function(options) { | |||
var trackNode = plugin.file.kml.ui.updatePlacemark({ | |||
'feature': track | |||
}); | |||
ol.events.listen(track, goog.events.EventType.PROPERTYCHANGE, trackNode.onFeatureChange, trackNode); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At a glance, the updatePlacemark
call is creating a plugin.file.kml.ui.KMLNode
which is already listening internally to this event in plugin.file.kml.ui.KMLNode.prototype.setFeature
. That feature change handler seems like the place where you would want to listen for updates to the name property.
One point of potential confusion here: we have out own PROPERTYCHANGE
events that we handle independently from the Openlayers events that fire whenever a property on an ol.Object
is changed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. I was thinking of the feature
in plugin.file.kml.ui.KMLNode.prototype.setFeature
as being the underlying linestring, rather than conceptually being the track. Will take another look.
On the second point, is there any guidance / conventions on which events get used where? Failing that, for a name or other track-level attribute, which should I use?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The feature in a KML node is sort of a data wrapper around a geometry, style, and a bunch of other metadata and utility for drawing the geometry to the map and interacting with it. Generally speaking, features and geometries have a 1 to 1 mapping, though through the use of styles, you can have multiple geometries on a single feature. I don't think tracks ever do this, but they do use a special feature class called DynamicFeature
which is how they update their geometries with changes in the timeline.
We generally have a convention of not using Openlayers' internal events whenever possible. They fire a lot of them, they add listeners for everything, and are generally unoptimized as a result. By default, Openlayers fires an event on every change to a property, which is a behavior we disable via mixin because it's just unnecessary overhead that we don't care about 99% of the time. In lieu of that, we'll fire our own property change events for things we know we care about.
In this case:
- Set the name property on the
Feature
instance. - Call
feature.dispatchEvent(new os.events.PropertyChangeEvent('label')
. - Add handling for a
'label'
property change inKMLNode
that sets the new label value on the node from the feature. - Fire the
PropertyChangeEvent
onward from theKMLNode
. This bubbles up the tree to the root, where the tree itself listens and should re-render itself on label change.
Resolves #842
The scenario is when the track is created off initial data (e.g. an aircraft ADS-B track using the ICAO code), and a later update provides the call sign. The track creator does something like:
track.values_.name = newName;
which updates the track label on the map, but the places node list keeps the old name
The caller updating the track name (or other values) still has to call
track.dispatchEvent(new os.events.PropertyChangeEvent(os.annotation.EventType.UPDATE_PLACEMARK));
This doesn't seem great, in that I had to override the access control. If there is a better way, would appreciate advice.