Better logic for adding area=yes (closes #2578)

This fixes a few issues:

1. before: checked first key in applyTags and break loop, now: check all of them
(this was what caused `area=yes` to be added to 'branded' presets:
the first key is for these is `name` which isn't in areaKeys.)

2. add `area=yes` if user is drawing an area but the preset can be an
area or a line (e.g. `barrier=city_wall`)

3. remove `area=yes` when switching to another preset
This commit is contained in:
Bryan Housel
2015-04-22 15:15:12 -04:00
parent 8bacf9858a
commit 99d037e97f
2 changed files with 36 additions and 10 deletions

View File

@@ -3,6 +3,7 @@ iD.presets.Preset = function(id, preset, fields) {
preset.id = id;
preset.fields = (preset.fields || []).map(getFields);
preset.geometry = (preset.geometry || []);
function getFields(f) {
return fields[f];
@@ -76,6 +77,7 @@ iD.presets.Preset = function(id, preset, fields) {
}
}
delete tags.area;
return tags;
};
@@ -93,11 +95,23 @@ iD.presets.Preset = function(id, preset, fields) {
}
}
// Add area=yes if necessary
for (k in applyTags) {
if (geometry === 'area' && !(k in iD.areaKeys))
// Add area=yes if necessary.
// This is necessary if the geometry is already an area (e.g. user drew an area) AND any of:
// 1. chosen preset could be either an area or a line (`barrier=city_wall`)
// 2. chosen preset doesn't have a key in areaKeys (`railway=station`)
if (geometry === 'area') {
var needsAreaTag = true;
if (preset.geometry.indexOf('line') === -1) {
for (k in applyTags) {
if (k in iD.areaKeys) {
needsAreaTag = false;
break;
}
}
}
if (needsAreaTag) {
tags.area = 'yes';
break;
}
}
for (var f in preset.fields) {

View File

@@ -77,7 +77,7 @@ describe('iD.presets.Preset', function() {
it("adds default tags of fields with matching geometry", function() {
var field = iD.presets.Field('field', {key: 'building', geometry: 'area', default: 'yes'}),
preset = iD.presets.Preset('test', {fields: ['field']}, {field: field});
expect(preset.applyTags({}, 'area')).to.eql({building: 'yes'});
expect(preset.applyTags({}, 'area')).to.eql({area: 'yes', building: 'yes'});
});
it("adds no default tags of fields with non-matching geometry", function() {
@@ -86,15 +86,22 @@ describe('iD.presets.Preset', function() {
expect(preset.applyTags({}, 'point')).to.eql({});
});
context("with an area preset whose primary tag is not in areaKeys", function() {
var preset = iD.presets.Preset('test', {geometry: ['line', 'area'], tags: {highway: 'pedestrian'}});
context("for a preset with no tag in areaKeys", function() {
var preset = iD.presets.Preset('test', {geometry: ['line', 'area'], tags: {name: 'testname', highway: 'pedestrian'}});
it("adds no area=yes to non-areas", function() {
expect(preset.applyTags({}, 'line')).to.eql({highway: 'pedestrian'});
it("doesn't add area=yes to non-areas", function() {
expect(preset.applyTags({}, 'line')).to.eql({name: 'testname', highway: 'pedestrian'});
});
it("adds area=yes to areas", function() {
expect(preset.applyTags({}, 'area')).to.eql({highway: 'pedestrian', area: 'yes'});
expect(preset.applyTags({}, 'area')).to.eql({name: 'testname', highway: 'pedestrian', area: 'yes'});
});
});
context("for a preset with a tag in areaKeys", function() {
var preset = iD.presets.Preset('test', {geometry: ['area'], tags: {name: 'testname', natural: 'water'}});
it("doesn't add area=yes", function() {
expect(preset.applyTags({}, 'area')).to.eql({name: 'testname', natural: 'water'});
});
});
});
@@ -111,6 +118,11 @@ describe('iD.presets.Preset', function() {
expect(preset.removeTags({building: 'yes'}, 'area')).to.eql({});
});
it('removes area=yes', function() {
var preset = iD.presets.Preset('test', {tags: {highway: 'pedestrian'}});
expect(preset.removeTags({highway: 'pedestrian', area: 'yes'}, 'area')).to.eql({});
});
it('preserves tags that do not match field default tags', function() {
var field = iD.presets.Field('field', {key: 'building', geometry: 'area', default: 'yes'}),
preset = iD.presets.Preset('test', {fields: ['field']}, {field: field});