Show more items in the navbar by jessetrinity · Pull Request #33040 · microsoft/TypeScript

jessetrinity · 2019-08-23T02:09:20Z

Currently the navigation bar in Visual Studio is not very useful for JavaScript files. We make the choice to show an item in the navbar based on the node type of its child items, and in some cases, its parent node.

By relaxing most the rules around what makes a navigation item "top level," we will get a more informative code hierarchy in the middle "object" dropdown, similar to how the navtree is presented in VSCode.

The right "member" dropdown always shows the child nodes of whichever node is selected in the middle dropdown, so we can keep navigation leaf nodes out of the middle dropdown to reduce clutter.

Property assignments and class this properties will also be added to the navigation tree with this change.

Take the following sample:

const foo = {
    bar: {
        baz: {}
    },
    biz: function qux() {
        var quim;
    }
};

var someFunctions = function () {
    var a = () => { };
    var b = function () { };
    var c = function d() { var C; };
    var e = function () {
        var f = function () { var F; };
    };
};

Before

Property assignments do not show up, so we don't see any structure of nested objects (see foo) unless they have an 'important' node such as a function (qux). In those cases, the name of the function shows up in the child items list rather than the more intuitive member name.

All of the child functions of someFunctions are deemed unimportant and only show in the member dropdown. Indeed even function e is woefully unimportant until its child f gets its own important child.

After

Property assignments now show up in navigation, and the name of the property shows instead of the function name.

We keep the decision to not show functions with empty bodies in the middle dropdown (though they do show as a child item in the right dropdown). e and f both show, as having any child item is now considered a good enough reason to show up in navigation.

jessetrinity · 2019-08-29T21:39:51Z

@amcasey this is ready for a look when you get a chance.

amcasey

I saw three distinct changes:

Property assignments go in the navtree
Anything with a child is "top-level" (vs special heuristics)
We use the property/variable name, rather than the initializer name, if the initializer is a function

Does that sound right? Did I miss anything?

This basically looks good - my main concern is that there might be other interesting SyntaxKinds.

I haven't read through the test changes yet, but I'm a little surprised to see that there aren't new tests. Maybe existing tests were expanded?

amcasey

Was there a binding pattern test? I didn't see one.

jessetrinity · 2019-08-30T20:36:12Z

I saw three distinct changes:

Property assignments go in the navtree

Anything with a child is "top-level" (vs special heuristics)

We use the property/variable name, rather than the initializer name, if the initializer is a function

Does that sound right? Did I miss anything?

This basically looks good - my main concern is that there might be other interesting SyntaxKinds.

I haven't read through the test changes yet, but I'm a little surprised to see that there aren't new tests. Maybe existing tests were expanded?

Your summary sounds correct. Mostly I just updated tests because this change should largely just make existing navbar tests more like the navtree ones.

I have added some more specific test for nested object literals to address (1) and a test for the function initializer ScriptElementKind.

jessetrinity · 2019-08-30T20:42:21Z

Was there a binding pattern test? I didn't see one.

There is an existing BindingElement test. I have updated it to check that a binding element with a function initializer is correctly named.

Also offline discussions have indicated that while people would prefer to see the method ScriptElementKind for properties with function initializers, there was less enthusiasm for doing the same for variables and binding elements. I have left those the same for now.

jessetrinity · 2019-09-11T01:49:34Z

@mjbvz

The changes here that would show up in VSCode are:

PropertyAssignments, ShorthandPropertyAssignments, and SpreadAssignments will show up in the navtree. Essentially all properties in a below

const a = {
    b: 0,
    c: function d(){},
    d,
    ....e
}

Where a variable declaration or a property assignment has a named function initializer, we use the name of the variable or property rather than the name of the function. In the above, we would expect to see c rather than d in navigation items.

amcasey

LGTM, but I think the "primary navbar" requires further explanation (i.e. in a comment).

amcasey · 2019-09-11T17:30:53Z

    /** Flattens the NavNode tree to a list, keeping only the top-level items. */
-    function topLevelItems(root: NavigationBarNode): NavigationBarNode[] {
-        const topLevel: NavigationBarNode[] = [];
+    function primaryNavBarItems(root: NavigationBarNode): NavigationBarNode[] {


Is the "primary" navbar the middle dropdown?

Explained it the comment where the selection actually occurs. I was trying to be 'editor agnostic' but I will just update 'primary' to 'middle' since (I think) this is VS only anyway.

What about something like "container"?

mjbvz

Description of new behavior sounds good for VS Code. We will see if users have any feedback once this is available in TS@next

jessetrinity requested review from amcasey and mjbvz August 23, 2019 02:09

jessetrinity mentioned this pull request Aug 23, 2019

Better outlining spans for prototype methods #32782

Merged

jessetrinity force-pushed the showMoreItemsInNavBar branch from bc25d91 to d006ef0 Compare August 28, 2019 17:55

jessetrinity marked this pull request as ready for review August 29, 2019 15:20

Jesse Trinity added 9 commits August 29, 2019 10:29

show more items in navbar

9c879c9

fixed missing node kind for property assignments

059c03d

updated navBarNestedCommonJsExports test

a589d8d

updated navigationBarMerging_grandchildren test

c1683d8

updated navigationBarItemsFunctions test

954f4b3

updated navigationBarAnonymousClassAndFunctionExpressions test

913c522

updated navigationBarFunctionIndirectlyInVariableDeclaration test

aac1ba1

updated navigationBarInitializerSpans test

92369c2

updated navigationBarItemsPropertiesDefinedInConstructors test

446708a

jessetrinity force-pushed the showMoreItemsInNavBar branch from a67c5e4 to 446708a Compare August 29, 2019 17:31

updated tests

7fb2903

amcasey reviewed Aug 29, 2019

View reviewed changes

Comment thread src/services/utilities.ts

Comment thread src/services/navigationBar.ts Outdated

amcasey reviewed Aug 29, 2019

View reviewed changes

Comment thread tests/cases/fourslash/navigationBarAnonymousClassAndFunctionExpressions.ts Outdated

Jesse Trinity added 4 commits August 30, 2019 10:32

change nav icon for properties with function-like initializers

5b36a6a

add test case for binding element with function-like initializer

6d3aae1

add navigationBarNestedObjectLiterals test

850c349

add navigationBarFunctionLikePropertyAssignments test

9b7b213

Jesse Trinity added 2 commits August 30, 2019 14:01

made some silly names less silly (?)

4ccc06c

added SpreadAssignments and ShorthandPropertyAssignments

0e32e48

jessetrinity requested a review from amcasey September 11, 2019 01:43

amcasey approved these changes Sep 11, 2019

View reviewed changes

mjbvz approved these changes Sep 11, 2019

View reviewed changes

new wording for primary menu items

81903e0

jessetrinity merged commit fd6fbdf into microsoft:master Sep 11, 2019

jessetrinity deleted the showMoreItemsInNavBar branch September 12, 2019 16:41

microsoft locked as resolved and limited conversation to collaborators Oct 21, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Show more items in the navbar#33040

Show more items in the navbar#33040
jessetrinity merged 17 commits into
microsoft:masterfrom
jessetrinity:showMoreItemsInNavBar

jessetrinity commented Aug 23, 2019 •

edited

Loading

Uh oh!

jessetrinity commented Aug 29, 2019

Uh oh!

amcasey left a comment

Uh oh!

Uh oh!

Uh oh!

amcasey left a comment

Uh oh!

Uh oh!

jessetrinity commented Aug 30, 2019

Uh oh!

jessetrinity commented Aug 30, 2019

Uh oh!

jessetrinity commented Sep 11, 2019 •

edited

Loading

Uh oh!

amcasey left a comment

Uh oh!

amcasey Sep 11, 2019

Uh oh!

jessetrinity Sep 11, 2019

Uh oh!

amcasey Sep 11, 2019

Uh oh!

mjbvz left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

jessetrinity commented Aug 23, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jessetrinity commented Aug 29, 2019

Uh oh!

amcasey left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

amcasey left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jessetrinity commented Aug 30, 2019

Uh oh!

jessetrinity commented Aug 30, 2019

Uh oh!

jessetrinity commented Sep 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

amcasey left a comment

Choose a reason for hiding this comment

Uh oh!

amcasey Sep 11, 2019

Choose a reason for hiding this comment

Uh oh!

jessetrinity Sep 11, 2019

Choose a reason for hiding this comment

Uh oh!

amcasey Sep 11, 2019

Choose a reason for hiding this comment

Uh oh!

mjbvz left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jessetrinity commented Aug 23, 2019 •

edited

Loading

jessetrinity commented Sep 11, 2019 •

edited

Loading