Permalink
Browse files

Fix GeoJSON Source processing in MockFeatureCollection (#1248)

If there were multiple features of the same type in a GeoJSON file and
the file did not include names for the components, one would receive
multiple components with the same name (e.g., LineString1). LineString
processing was also broken, resulting in a coordinate string without
values. Setting the Source to None... would remove the components but
not clear the features off the map.

This commit addresses three changes:

1. Refactors how naming of map features is handled during the loading
   of GeoJSON into a MockFeatureCollection so that each component has a
   unique name.
2. It corrects handling of LineString coordinates when the line string
   is not part of a MultiLineString construct.
3. It cleans up how features are removed when a feature collection is
   cleared.

Change-Id: Iccbcab65989169ab730239b8915b62ca6b6f636c
  • Loading branch information...
ewpatton authored and SusanRatiLane committed Nov 8, 2018
1 parent c51e829 commit 9d95098654ff5d6e02dfb462a7dd125163d0e277
@@ -104,6 +104,7 @@ private void setSourceProperty(String text) {
List<MockComponent> children = new ArrayList<MockComponent>(getChildren());
for (MockComponent component : children) {
removeComponent(component, true);
component.onRemoved();
}
return;
}
@@ -117,10 +118,8 @@ public void onFailure(Throwable caught) {
@Override
public void onSuccess(String result) {
setGeoJSONProperty(result);
for (MockMapFeature feature : features) {
MockFeatureCollection.this.addVisibleComponent((MockComponent) feature, -1);
feature.addToMap(map);
}
MockFeatureCollection.this.onSelectedChange(true); // otherwise the last imported component
// will be shown in the properties panel
}
});
}
@@ -32,7 +32,6 @@ public MockLineString(SimpleEditor editor) {
static MockLineString fromGeoJSON(MockFeatureCollection parent, JSONObject properties, JavaScriptObject layer) {
MockLineString line = new MockLineString(parent.editor);
line.feature = layer;
line.setContainer(parent);
String name = null;
for (String key : properties.keySet()) {
String value;
@@ -60,17 +59,7 @@ static MockLineString fromGeoJSON(MockFeatureCollection parent, JSONObject prope
line.onPropertyChange(PROPERTY_NAME_VISIBLE, value);
}
}
if (name == null) {
name = line.getPropertyValue(PROPERTY_NAME_TITLE);
}
name = name.replaceAll("[ \t]+", "_");
if (name.equalsIgnoreCase("")) {
name = ComponentsTranslation.getComponentName(TYPE) + "1";
}
name = ensureUniqueName(name, parent.editor.getComponentNames());
line.changeProperty(PROPERTY_NAME_NAME, name);
line.onPropertyChange(PROPERTY_NAME_NAME, name);
line.getForm().fireComponentRenamed(line, ComponentsTranslation.getComponentName(TYPE));
processFeatureName(line, parent, name);
line.preserveLayerData();
return line;
}
@@ -188,7 +177,11 @@ private native void preserveLayerData()/*-{
if (line) {
var latlngs = line.getLatLngs();
var resultJson = [];
if (latlngs[0][0] instanceof top.L.LatLng) {
if (latlngs[0] instanceof top.L.LatLng) {
for (var i = 0; i < latlngs.length; i++) {
resultJson.push([latlngs[i].lat, latlngs[i].lng]);
}
} else if (latlngs[0][0] instanceof top.L.LatLng) {
for (var i = 0; i < latlngs[0].length; i++) {
resultJson.push([latlngs[0][i].lat, latlngs[0][i].lng]);
}
@@ -5,6 +5,7 @@

package com.google.appinventor.client.editor.simple.components;

import com.google.appinventor.client.ComponentsTranslation;
import com.google.appinventor.client.editor.simple.SimpleEditor;
import com.google.gwt.core.client.JavaScriptObject;
import com.google.gwt.resources.client.ImageResource;
@@ -125,7 +126,7 @@ protected static String ensureUniqueName(String name, Collection<String> names)
}
int i = name.length() - 1;
while (name.charAt(i) >= '0' && name.charAt(i) <= '9') i--;
String base = name.substring(0, i);
String base = name.substring(0, i + 1);
int counter = 1;
if (i < name.length() - 1) {
counter = Integer.parseInt(name.substring(i + 1)) + 1;
@@ -134,6 +135,25 @@ protected static String ensureUniqueName(String name, Collection<String> names)
return base + counter;
}

protected static void processFeatureName(MockMapFeatureBase feature, MockFeatureCollection parent,
String name) {
if (name == null) {
name = feature.getPropertyValue(PROPERTY_NAME_TITLE);
}
name = name.replaceAll("[ \t]+", "_");
if (name.equalsIgnoreCase("")) {
name = ComponentsTranslation.getComponentName(feature.getType()) + "1";
}
name = ensureUniqueName(name, parent.editor.getComponentNames());
parent.addVisibleComponent(feature, -1);
final String oldName = feature.getPropertyValue(PROPERTY_NAME_NAME);
if (oldName == null || !oldName.equals(name)) {
feature.changeProperty(PROPERTY_NAME_NAME, name);
feature.onPropertyChange(PROPERTY_NAME_NAME, name);
feature.getForm().fireComponentRenamed(feature, oldName);
}
}

// JSNI Methods
public static native double distanceBetweenPoints(JavaScriptObject map, MockMap.LatLng point1, MockMap.LatLng point2)/*-{
var pt1 = [point1.@com.google.appinventor.client.editor.simple.components.MockMap.LatLng::latitude,
@@ -53,7 +53,6 @@ public MockMarker(SimpleEditor editor) {
static MockMarker fromGeoJSON(MockFeatureCollection parent, JSONObject properties, JavaScriptObject layer) {
MockMarker marker = new MockMarker(parent.editor);
marker.feature = layer;
marker.setContainer(parent);
String name = null;
boolean hadImageAsset = false;
for (String key : properties.keySet()) {
@@ -107,17 +106,7 @@ static MockMarker fromGeoJSON(MockFeatureCollection parent, JSONObject propertie
marker.onPropertyChange(PROPERTY_NAME_DESCRIPTION, value);
}
}
if (name == null) {
name = marker.getPropertyValue(PROPERTY_NAME_TITLE);
}
name = name.replaceAll("[ \t]+", "_");
if (name.equalsIgnoreCase("")) {
name = ComponentsTranslation.getComponentName("Marker") + "1";
}
name = ensureUniqueName(name, parent.editor.getComponentNames());
marker.changeProperty(PROPERTY_NAME_NAME, name);
marker.onPropertyChange(PROPERTY_NAME_NAME, name);
marker.getForm().fireComponentRenamed(marker, ComponentsTranslation.getComponentName("Marker"));
processFeatureName(marker, parent, name);
if (!hadImageAsset) {
marker.setImageAsset(null);
}
@@ -34,6 +34,7 @@ public MockPolygon(SimpleEditor editor) {
static MockPolygon fromGeoJSON(MockFeatureCollection parent, JSONObject properties, JavaScriptObject layer) {
MockPolygon polygon = new MockPolygon(parent.editor);
polygon.feature = layer;
String name = null;
boolean hadFillColor = false, hadStrokeColor = false, hadStrokeWidth = false;
for (String key : properties.keySet()) {
if (key.equalsIgnoreCase(PROPERTY_NAME_STROKEWIDTH) || key.equalsIgnoreCase(CSS_PROPERTY_STROKEWIDTH)) {
@@ -46,9 +47,7 @@ static MockPolygon fromGeoJSON(MockFeatureCollection parent, JSONObject properti
polygon.getProperties().changePropertyValue(PROPERTY_NAME_FILLCOLOR, properties.get(key).isString().stringValue());
hadFillColor = true;
} else if (key.equalsIgnoreCase(PROPERTY_NAME_NAME)) {
String name = properties.get(key).isString().stringValue();
name = name.replaceAll("[ \t]+", "_");
polygon.changeProperty(PROPERTY_NAME_NAME, name);
name = properties.get(key).isString().stringValue();
} else if (key.equalsIgnoreCase(PROPERTY_NAME_VISIBLE)) {
polygon.setVisibleProperty(properties.get(key).isString().stringValue());
}
@@ -62,6 +61,7 @@ static MockPolygon fromGeoJSON(MockFeatureCollection parent, JSONObject properti
if (!hadStrokeWidth) {
polygon.getProperties().changePropertyValue(PROPERTY_NAME_STROKEWIDTH, "1");
}
processFeatureName(polygon, parent, name);
polygon.preserveLayerData();
return polygon;
}
@@ -157,9 +157,12 @@ Blockly.ComponentDatabase.prototype.renameInstance = function(uid, oldName, newN
if (!this.hasInstance(uid)) {
return false;
}
if (oldName === newName) { // oldName is the same as newName... don't waste time
return false;
}
this.instances_[uid].name = newName;
this.instanceNameUid_[newName] = uid;
delete this.instanceNameUid_[oldName];
this.instanceNameUid_[newName] = uid;
return true;
};

0 comments on commit 9d95098

Please sign in to comment.