Skip to content

Always include a root node in the navigation bar. - #8812

Merged
6 commits merged into
masterfrom
navbar_root
May 27, 2016
Merged

Always include a root node in the navigation bar.#8812
6 commits merged into
masterfrom
navbar_root

Conversation

@ghost

@ghostDeleted user (ghost) commented May 25, 2016

Copy link
Copy Markdown

This should wait for #8764 to be resolved so that the root has all the children it needs.

@msftclas

Microsoft Contribution License Agreements (msftclas) commented May 25, 2016

Copy link
Copy Markdown

Hi Andy (Andrewkraft) (@Andy-MS), I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!


It looks like you're a Microsoft contributor (Andy Hanson). If you're full-time, we DON'T require a Contribution License Agreement. If you are a vendor, please DO sign the electronic Contribution License Agreement. It will take 2 minutes and there's no faxing! https://cla.microsoft.com.

TTYL, MSBOT;

@ghostghost mentioned this pull request May 25, 2016
Closed
@mhegazy

Mohamed Hegazy (mhegazy) commented May 25, 2016

Copy link
Copy Markdown
Contributor

👍

Andy Hanson added 2 commits May 26, 2016 08:40
Andy Hanson
Always include a root node in the navigation bar.
fe77f54
This lets us change the navigation bar counting algorithm to traverse from the root only, so it never has duplicate nodes.
Andy Hanson
Change tests to use the full JSON output of the navigation bar.
fe970ab
This reduces the confusion of verify.navigationBarCount() counting duplicate items.
@ghost
ghost force-pushed the navbar_root branch from 53a9393 to fe970abCompareMay 26, 2016 15:40
@ghost

Deleted user (ghost) commented May 26, 2016

Copy link
Copy Markdown
Author

Added a new commit which needs review.

@ghostghost mentioned this pull request May 26, 2016
Closed
Andy Hanson added 2 commits May 26, 2016 09:01
Andy Hanson
Fix lint errors
12914ea
Andy Hanson
Fix localeCompare differences between node versions
58d69cd
node 6.2.0: "a".localeCompare("A") is -1.
node 0.10.45: "a".localeCompare("A") is 32.
@ghost
ghost force-pushed the navbar_root branch from c861d56 to 58d69cdCompareMay 26, 2016 17:36
mhegazy
mhegazy reviewed May 26, 2016
View reviewed changes
Comment threadsrc/harness/fourslash.ts Outdated

// Remove any properties that tend to all have the same value so that test data is easier to read.
private simplifyNavigationBar(items: ts.NavigationBarItem[]): any {
return items.map(item => {

@mhegazyMohamed Hegazy (mhegazy)May 26, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if this is only for comparison purposes, i would do this in the replacer function for JSON.stringify(items, (k,v)=> { if (key === "indent") return undefined; return v; });

@mhegazy

Mohamed Hegazy (mhegazy) commented May 26, 2016

Copy link
Copy Markdown
Contributor

test changes look good. i would give the JSON.stringify(item, replacerFunction) a try.

@ghost
ghost force-pushed the navbar_root branch from 54663cf to b2664e7CompareMay 27, 2016 13:27
@ghost
ghost merged commit a1f110f into masterMay 27, 2016
@ghost
ghost deleted the navbar_root branch May 27, 2016 13:56
@ghostghost mentioned this pull request May 27, 2016
Closed
@microsoftMicrosoft (microsoft) locked and limited conversation to collaborators Jun 19, 2018
This pull request was closed.
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@msftclas@mhegazy