Hide feature if _all_ rules hidden (was: if _any_ rule hidden)

This matters as we start to match more rules for hybrid features like rail
platforms, which now match both path and rail.  We want to show them unless the
user has hidden all the rules that they match.

Also this changes the test code slightly to actually test rule matching.
Before it was really just testing hiding.

modules/renderer/features.js

@@ -413,8 +413,10 @@ export function rendererFeatures(context) {
     features.isHiddenPreset = function(preset, geometry) {
         if (!_hidden.length) return false;
         if (!preset.tags) return false;
+
+        var test = preset.setTags({}, geometry);
         for (var key in _rules) {
-            if (_rules[key].filter(preset.setTags({}, geometry), geometry)) {
+            if (_rules[key].filter(test, geometry)) {
                 if (_hidden.indexOf(key) !== -1) {
                     return key;
                 }
@@ -429,12 +431,8 @@ export function rendererFeatures(context) {
         if (!_hidden.length) return false;
         if (!entity.version) return false;
 
-        var matches = features.getMatches(entity, resolver, geometry);
-
-        for (var i = 0; i < _hidden.length; i++) {
-            if (matches[_hidden[i]]) return true;
-        }
-        return false;
+        var matches = Object.keys(features.getMatches(entity, resolver, geometry));
+        return matches.every(function(k) { return features.hidden(k); });
     };
 
 
@@ -456,8 +454,8 @@ export function rendererFeatures(context) {
 
     features.hasHiddenConnections = function(entity, resolver) {
         if (!_hidden.length) return false;
-        var childNodes, connections;
 
+        var childNodes, connections;
         if (entity.type === 'midpoint') {
             childNodes = [resolver.entity(entity.edge[0]), resolver.entity(entity.edge[1])];
             connections = [];
@@ -503,6 +501,7 @@ export function rendererFeatures(context) {
 
     features.forceVisible = function(entityIDs) {
         if (!arguments.length) return Object.keys(_forceVisible);
+
         _forceVisible = {};
         for (var i = 0; i < entityIDs.length; i++) {
             _forceVisible[entityIDs[i]] = true;

modules/ui/preset_list.js

@@ -428,19 +428,17 @@ export function uiPresetList(context) {
         return item;
     }
 
-    function updateForFeatureHiddenState() {
 
+    function updateForFeatureHiddenState() {
         if (!context.hasEntity(_entityID)) return;
 
         var geometry = context.geometry(_entityID);
-
         var button = d3_selectAll('.preset-list .preset-list-button');
 
         // remove existing tooltips
         button.call(tooltip().destroyAny);
 
         button.each(function(item, index) {
-
             var hiddenPresetFeaturesId = context.features().isHiddenPreset(item.preset, geometry);
             var isHiddenPreset = !!hiddenPresetFeaturesId && item.preset !== _currentPreset;
 

test/spec/renderer/features.js

@@ -218,33 +218,34 @@ describe('iD.rendererFeatures', function() {
         var all = Object.values(graph.base().entities);
 
 
-        function doMatch(ids) {
+        function doMatch(rule, ids) {
             ids.forEach(function(id) {
-                var entity = graph.entity(id),
-                    geometry = entity.geometry(graph);
-                expect(features.isHidden(entity, graph, geometry), 'doMatch: ' + id).to.be.true;
+                var entity = graph.entity(id);
+                var geometry = entity.geometry(graph);
+                expect(features.getMatches(entity, graph, geometry), 'doMatch: ' + id)
+                    .to.have.property(rule);
             });
         }
 
-        function dontMatch(ids) {
+        function dontMatch(rule, ids) {
             ids.forEach(function(id) {
-                var entity = graph.entity(id),
-                    geometry = entity.geometry(graph);
-                expect(features.isHidden(entity, graph, geometry), 'dontMatch: ' + id).to.be.false;
+                var entity = graph.entity(id);
+                var geometry = entity.geometry(graph);
+                expect(features.getMatches(entity, graph, geometry), 'dontMatch: ' + id)
+                    .not.to.have.property(rule);
             });
         }
 
 
         it('matches points', function () {
-            features.disable('points');
             features.gatherStats(all, graph, dimensions);
 
-            doMatch([
+            doMatch('points', [
                 'point_bar', 'point_dock', 'point_rail_station',
                 'point_generator', 'point_old_rail_station'
             ]);
 
-            dontMatch([
+            dontMatch('points', [
                 'motorway', 'service', 'path', 'building_yes',
                 'forest', 'boundary', 'boundary_member', 'water', 'railway', 'power_line',
                 'motorway_construction', 'fence'
@@ -253,17 +254,16 @@ describe('iD.rendererFeatures', function() {
 
 
         it('matches traffic roads', function () {
-            features.disable('traffic_roads');
             features.gatherStats(all, graph, dimensions);
 
-            doMatch([
+            doMatch('traffic_roads', [
                 'motorway', 'motorway_link', 'trunk', 'trunk_link',
                 'primary', 'primary_link', 'secondary', 'secondary_link',
                 'tertiary', 'tertiary_link', 'residential', 'living_street',
                 'unclassified', 'boundary_road', 'inner3'
             ]);
 
-            dontMatch([
+            dontMatch('traffic_roads', [
                 'point_bar', 'service', 'road', 'track', 'path', 'building_yes',
                 'forest', 'boundary', 'boundary_member', 'water', 'railway', 'power_line',
                 'motorway_construction', 'fence'
@@ -272,14 +272,13 @@ describe('iD.rendererFeatures', function() {
 
 
         it('matches service roads', function () {
-            features.disable('service_roads');
             features.gatherStats(all, graph, dimensions);
 
-            doMatch([
+            doMatch('service_roads', [
                 'service', 'road', 'track'
             ]);
 
-            dontMatch([
+            dontMatch('service_roads', [
                 'point_bar', 'motorway', 'unclassified', 'living_street',
                 'path', 'building_yes', 'forest', 'boundary', 'boundary_member', 'water',
                 'railway', 'power_line', 'motorway_construction', 'fence'
@@ -288,15 +287,14 @@ describe('iD.rendererFeatures', function() {
 
 
         it('matches paths', function () {
-            features.disable('paths');
             features.gatherStats(all, graph, dimensions);
 
-            doMatch([
+            doMatch('paths', [
                 'path', 'footway', 'cycleway', 'bridleway',
                 'steps', 'pedestrian', 'corridor'
             ]);
 
-            dontMatch([
+            dontMatch('paths', [
                 'point_bar', 'motorway', 'service', 'building_yes',
                 'forest', 'boundary', 'boundary_member', 'water', 'railway', 'power_line',
                 'motorway_construction', 'fence'
@@ -305,15 +303,14 @@ describe('iD.rendererFeatures', function() {
 
 
         it('matches buildings', function () {
-            features.disable('buildings');
             features.gatherStats(all, graph, dimensions);
 
-            doMatch([
+            doMatch('buildings', [
                 'building_yes', 'building_part',
                 'garage1', 'garage2', 'garage3', 'garage4'
             ]);
 
-            dontMatch([
+            dontMatch('buildings', [
                 'building_no', 'point_bar', 'motorway', 'service', 'path',
                 'forest', 'boundary', 'boundary_member', 'water', 'railway', 'power_line',
                 'motorway_construction', 'fence'
@@ -322,16 +319,15 @@ describe('iD.rendererFeatures', function() {
 
 
         it('matches landuse', function () {
-            features.disable('landuse');
             features.gatherStats(all, graph, dimensions);
 
-            doMatch([
+            doMatch('landuse', [
                 'forest', 'scrub', 'industrial', 'parkinglot', 'building_no',
                 'rail_landuse', 'landuse_construction', 'retail',
                 'outer', 'inner1', 'inner2'  // non-interesting members of landuse multipolygon
             ]);
 
-            dontMatch([
+            dontMatch('landuse', [
                 'point_bar', 'motorway', 'service', 'path', 'building_yes',
                 'boundary', 'boundary_member', 'water', 'railway', 'power_line',
                 'motorway_construction', 'fence',
@@ -341,10 +337,9 @@ describe('iD.rendererFeatures', function() {
 
 
         it('matches boundaries', function () {
-            features.disable('boundaries');
             features.gatherStats(all, graph, dimensions);
 
-            doMatch([
+            doMatch('boundaries', [
                 'boundary',
                 // match ways that are part of boundary relations - #5601
                 'boundary_member', 'boundary_member2',
@@ -352,7 +347,7 @@ describe('iD.rendererFeatures', function() {
                 'boundary_relation', 'boundary_relation2'
             ]);
 
-            dontMatch([
+            dontMatch('boundaries', [
                 'boundary_road',   // because boundary also used as highway - #4973
                 'point_bar', 'motorway', 'service', 'path', 'building_yes',
                 'forest', 'water', 'railway', 'power_line',
@@ -362,15 +357,14 @@ describe('iD.rendererFeatures', function() {
 
 
         it('matches water', function () {
-            features.disable('water');
             features.gatherStats(all, graph, dimensions);
 
-            doMatch([
+            doMatch('water', [
                 'point_dock', 'water', 'coastline', 'bay', 'pond',
                 'basin', 'reservoir', 'salt_pond', 'river'
             ]);
 
-            dontMatch([
+            dontMatch('water', [
                 'point_bar', 'motorway', 'service', 'path', 'building_yes',
                 'forest', 'boundary', 'boundary_member', 'railway', 'power_line',
                 'motorway_construction', 'fence'
@@ -379,15 +373,14 @@ describe('iD.rendererFeatures', function() {
 
 
         it('matches rail', function () {
-            features.disable('rail');
             features.gatherStats(all, graph, dimensions);
 
-            doMatch([
+            doMatch('rail', [
                 'point_rail_station', 'point_old_rail_station',
                 'railway', 'rail_landuse', 'rail_disused'
             ]);
 
-            dontMatch([
+            dontMatch('rail', [
                 'rail_streetcar', 'rail_trail',  // because rail also used as highway
                 'point_bar', 'motorway', 'service', 'path', 'building_yes',
                 'forest', 'boundary', 'boundary_member', 'water', 'power_line',
@@ -397,14 +390,13 @@ describe('iD.rendererFeatures', function() {
 
 
         it('matches power', function () {
-            features.disable('power');
             features.gatherStats(all, graph, dimensions);
 
-            doMatch([
+            doMatch('power', [
                 'point_generator', 'power_line'
             ]);
 
-            dontMatch([
+            dontMatch('power', [
                 'point_bar', 'motorway', 'service', 'path', 'building_yes',
                 'forest', 'boundary', 'boundary_member', 'water', 'railway',
                 'motorway_construction', 'fence'
@@ -413,15 +405,14 @@ describe('iD.rendererFeatures', function() {
 
 
         it('matches past/future', function () {
-            features.disable('past_future');
             features.gatherStats(all, graph, dimensions);
 
-            doMatch([
+            doMatch('past_future', [
                 'point_old_rail_station', 'rail_disused',
                 'motorway_construction', 'cycleway_proposed', 'landuse_construction'
             ]);
 
-            dontMatch([
+            dontMatch('past_future', [
                 'rail_trail',  // because rail also used as highway
                 'point_bar', 'motorway', 'service', 'path', 'building_yes',
                 'forest', 'boundary', 'boundary_member', 'water', 'railway', 'power_line', 'fence'
@@ -430,14 +421,13 @@ describe('iD.rendererFeatures', function() {
 
 
         it('matches others', function () {
-            features.disable('others');
             features.gatherStats(all, graph, dimensions);
 
-            doMatch([
+            doMatch('others', [
                 'fence', 'pipeline'
             ]);
 
-            dontMatch([
+            dontMatch('others', [
                 'point_bar', 'motorway', 'service', 'path', 'building_yes',
                 'forest', 'boundary', 'boundary_member', 'water', 'railway', 'power_line',
                 'motorway_construction', 'retail', 'outer', 'inner1', 'inner2', 'inner3'