Merge pull request #5632 from quincylvania/sidebar-behavior-on-feature-selection

Change the initial inspector of the sidebar upon feature selection

modules/osm/entity.js

@@ -143,11 +143,13 @@ osmEntity.prototype = {
     },
 
 
-    isUsed: function(resolver) {
-        return _without(Object.keys(this.tags), 'area').length > 0 ||
-            resolver.parentRelations(this).length > 0;
+    hasNonGeometryTags: function() {
+        return _without(Object.keys(this.tags), 'area').length > 0;
     },
 
+    hasParentRelations: function(resolver) {
+        return resolver.parentRelations(this).length > 0;
+    },
 
     hasInterestingTags: function() {
         return _keys(this.tags).some(osmIsInterestingTag);

modules/ui/inspector.js

@@ -14,7 +14,7 @@ export function uiInspector(context) {
     var _newFeature = false;
 
 
-    function inspector(selection) {
+    function inspector(selection, newFeature) {
         presetList
             .entityID(_entityID)
             .autofocus(_newFeature)
@@ -44,19 +44,18 @@ export function uiInspector(context) {
         var presetPane = wrap.selectAll('.preset-list-pane');
         var editorPane = wrap.selectAll('.entity-editor-pane');
 
-        var graph = context.graph();
         var entity = context.entity(_entityID);
 
-        var showEditor = _state === 'hover' ||
-            entity.isUsed(graph) ||
-            entity.isHighwayIntersection(graph);
+        var isTaglessVertex = entity.geometry(context.graph()) === 'vertex' && !entity.hasNonGeometryTags();
+        // start with the preset list if the feature is new or is a vertex with no tags
+        var showPresetList = newFeature || isTaglessVertex;
 
-        if (showEditor) {
-            wrap.style('right', '0%');
-            editorPane.call(entityEditor);
-        } else {
+        if (showPresetList) {
             wrap.style('right', '-100%');
             presetPane.call(presetList);
+        } else {
+            wrap.style('right', '0%');
+            editorPane.call(entityEditor);
         }
 
         var footer = selection.selectAll('.footer')

modules/ui/preset_list.js

@@ -8,7 +8,6 @@ import {
 import { t, textDirection } from '../util/locale';
 import { actionChangePreset } from '../actions/index';
 import { operationDelete } from '../operations/index';
-import { modeBrowse } from '../modes/index';
 import { svgIcon } from '../svg/index';
 import { uiPresetIcon } from './preset_icon';
 import { uiTagReference } from './tag_reference';
@@ -43,21 +42,11 @@ export function uiPresetList(context) {
             .append('h3')
             .text(t('inspector.choose'));
 
-        if (context.entity(_entityID).isUsed(context.graph())) {
-            messagewrap
-                .append('button')
-                .attr('class', 'preset-choose')
-                .on('click', function() { dispatch.call('choose', this, _currentPreset); })
-                .call(svgIcon((textDirection === 'rtl') ? '#iD-icon-backward' : '#iD-icon-forward'));
-        } else {
-            messagewrap
-                .append('button')
-                .attr('class', 'close')
-                .on('click', function() {
-                    context.enter(modeBrowse(context));
-                })
-                .call(svgIcon('#iD-icon-close'));
-        }
+        messagewrap
+            .append('button')
+            .attr('class', 'preset-choose')
+            .on('click', function() { dispatch.call('choose', this, _currentPreset); })
+            .call(svgIcon((textDirection === 'rtl') ? '#iD-icon-backward' : '#iD-icon-forward'));
 
         function initialKeydown() {
             // hack to let delete shortcut work when search is autofocused

modules/ui/sidebar.js

@@ -204,7 +204,7 @@ export function uiSidebar(context) {
                         .newFeature(newFeature);
 
                     inspectorWrap
-                        .call(inspector);
+                        .call(inspector, newFeature);
                 }
 
             } else {

test/spec/osm/entity.js

@@ -189,30 +189,35 @@ describe('iD.osmEntity', function () {
         });
     });
 
-    describe('#isUsed', function () {
+    describe('#hasNonGeometryTags', function () {
         it('returns false for an entity without tags', function () {
-            var node = iD.Node(),
-                graph = iD.Graph([node]);
-            expect(node.isUsed(graph)).to.equal(false);
+            var node = iD.Node();
+            expect(node.hasNonGeometryTags()).to.equal(false);
         });
 
         it('returns true for an entity with tags', function () {
-            var node = iD.Node({tags: {foo: 'bar'}}),
-                graph = iD.Graph([node]);
-            expect(node.isUsed(graph)).to.equal(true);
+            var node = iD.Node({tags: {foo: 'bar'}});
+            expect(node.hasNonGeometryTags()).to.equal(true);
         });
 
         it('returns false for an entity with only an area=yes tag', function () {
-            var node = iD.Node({tags: {area: 'yes'}}),
-                graph = iD.Graph([node]);
-            expect(node.isUsed(graph)).to.equal(false);
+            var node = iD.Node({tags: {area: 'yes'}});
+            expect(node.hasNonGeometryTags()).to.equal(false);
         });
+    });
 
+    describe('#hasParentRelations', function () {
         it('returns true for an entity that is a relation member', function () {
             var node = iD.Node(),
                 relation = iD.Relation({members: [{id: node.id}]}),
                 graph = iD.Graph([node, relation]);
-            expect(node.isUsed(graph)).to.equal(true);
+            expect(node.hasParentRelations(graph)).to.equal(true);
+        });
+
+        it('returns false for an entity that is not a relation member', function () {
+            var node = iD.Node(),
+                graph = iD.Graph([node]);
+            expect(node.hasParentRelations(graph)).to.equal(false);
         });
     });