fix(aio): remove links from sub-menu toggles (#21695)

Navigating to a document while trying to expand or collapse a sub-menu
is undesirable and confusing. All sub-menu toggles should have no other
effect than expanding/collapsing the corresponding sub-menu.

PR Close #21695
This commit is contained in:
George Kalpakas 2018-01-16 12:40:24 +02:00 committed by Miško Hevery
parent 8f6047340e
commit 4f869ff755
4 changed files with 78 additions and 15 deletions

View File

@ -74,7 +74,6 @@
},
{
"url": "tutorial",
"title": "Tutorial",
"tooltip": "The Tour of Heroes tutorial takes you through the steps of creating an Angular application in TypeScript.",
"children": [
@ -122,7 +121,6 @@
},
{
"url": "guide/architecture",
"title": "Fundamentals",
"tooltip": "The fundamentals of Angular",
"children": [

View File

@ -128,6 +128,12 @@ button.vertical-menu-item {
transition-timing-function: ease-out;
}
.no-animations {
.heading-children.expanded, .heading-children.collapsed {
transition: none! important;
}
}
.level-1 {
font-family: $main-font;
font-size: 14px;

View File

@ -1,4 +1,4 @@
import { browser, by, element } from 'protractor';
import { browser, by, element, ElementFinder } from 'protractor';
import { SitePage } from './app.po';
describe('site App', function() {
@ -11,7 +11,7 @@ describe('site App', function() {
it('should show features text after clicking "Features"', () => {
page.navigateTo('');
page.getTopMenuLink('features').click();
page.click(page.getTopMenuLink('features'));
expect(page.getDocViewerText()).toMatch(/Progressive web apps/i);
});
@ -19,28 +19,74 @@ describe('site App', function() {
page.navigateTo('');
expect(browser.getTitle()).toBe('Angular');
page.getTopMenuLink('features').click();
page.click(page.getTopMenuLink('features'));
expect(browser.getTitle()).toBe('Angular - FEATURES & BENEFITS');
page.homeLink.click();
page.click(page.homeLink);
expect(browser.getTitle()).toBe('Angular');
});
it('should not navigate when clicking on nav-item headings (sub-menu toggles)', () => {
// Show the sidenav.
page.navigateTo('docs');
expect(page.locationPath()).toBe('/docs');
// Get the top-level nav-item headings (sub-menu toggles).
const navItemHeadings = page.getNavItemHeadings(page.sidenav, 1);
// Test all headings (and sub-headings).
expect(navItemHeadings.count()).toBeGreaterThan(0);
navItemHeadings.each(heading => testNavItemHeading(heading!, 1));
// Helpers
function expectToBeCollapsed(element: ElementFinder) {
expect(element.getAttribute('class')).toMatch(/\bcollapsed\b/);
expect(element.getAttribute('class')).not.toMatch(/\bexpanded\b/);
}
function expectToBeExpanded(element: ElementFinder) {
expect(element.getAttribute('class')).not.toMatch(/\bcollapsed\b/);
expect(element.getAttribute('class')).toMatch(/\bexpanded\b/);
}
function testNavItemHeading(heading: ElementFinder, level: number) {
const children = page.getNavItemHeadingChildren(heading, level);
// Headings are initially collapsed.
expectToBeCollapsed(children);
// Ensure heading does not cause navigation when expanding.
page.click(heading);
expectToBeExpanded(children);
expect(page.locationPath()).toBe('/docs');
// Recursively test child-headings (while this heading is expanded).
const nextLevel = level + 1;
const childNavItemHeadings = page.getNavItemHeadings(children, nextLevel);
childNavItemHeadings.each(childHeading => testNavItemHeading(childHeading!, nextLevel));
// Ensure heading does not cause navigation when collapsing.
page.click(heading);
expectToBeCollapsed(children);
expect(page.locationPath()).toBe('/docs');
}
});
it('should show the tutorial index page at `/tutorial` after jitterbugging through features', () => {
// check that we can navigate directly to the tutorial page
page.navigateTo('tutorial');
expect(page.getDocViewerText()).toMatch(/Tutorial: Tour of Heroes/i);
// navigate to a different page
page.getTopMenuLink('features').click();
page.click(page.getTopMenuLink('features'));
expect(page.getDocViewerText()).toMatch(/Progressive web apps/i);
// Show the menu
page.docsMenuLink.click();
page.click(page.docsMenuLink);
// Tutorial folder should still be expanded because this test runs in wide mode
// Navigate to the tutorial introduction via a link in the sidenav
page.getNavItem(/introduction/i).click();
page.click(page.getNavItem(/introduction/i));
expect(page.getDocViewerText()).toMatch(/Tutorial: Tour of Heroes/i);
});
@ -57,8 +103,7 @@ describe('site App', function() {
page.scrollToBottom();
expect(page.getScrollTop()).toBeGreaterThan(0);
page.getNavItem(/api/i).click();
browser.waitForAngular();
page.click(page.getNavItem(/api/i));
expect(page.locationPath()).toBe('/api');
expect(page.getScrollTop()).toBe(0);
});
@ -69,8 +114,7 @@ describe('site App', function() {
page.scrollToBottom();
expect(page.getScrollTop()).toBeGreaterThan(0);
page.getNavItem(/security/i).click();
browser.waitForAngular();
page.click(page.getNavItem(/security/i));
expect(page.locationPath()).toBe('/guide/security');
expect(page.getScrollTop()).toBe(0);
});
@ -102,7 +146,7 @@ describe('site App', function() {
it('should call ga with new URL on navigation', done => {
let path: string;
page.navigateTo('');
page.getTopMenuLink('features').click();
page.click(page.getTopMenuLink('features'));
page.locationPath()
.then(p => path = p)
.then(() => page.ga())
@ -125,7 +169,7 @@ describe('site App', function() {
expect(element(by.css('meta[name="googlebot"][content="noindex"]')).isPresent()).toBeTruthy();
expect(element(by.css('meta[name="robots"][content="noindex"]')).isPresent()).toBeTruthy();
page.getTopMenuLink('features').click();
page.click(page.getTopMenuLink('features'));
expect(element(by.css('meta[name="googlebot"]')).isPresent()).toBeFalsy();
expect(element(by.css('meta[name="robots"]')).isPresent()).toBeFalsy();
});

View File

@ -7,6 +7,7 @@ export class SitePage {
links = element.all(by.css('md-toolbar a'));
homeLink = element(by.css('a.home'));
docsMenuLink = element(by.cssContainingText('aio-top-menu a', 'Docs'));
sidenav = element(by.css('mat-sidenav'));
docViewer = element(by.css('aio-doc-viewer'));
codeExample = element.all(by.css('aio-doc-viewer pre > code'));
ghLink = this.docViewer
@ -24,7 +25,17 @@ export class SitePage {
.filter(element => element.getText().then(text => pattern.test(text)))
.first();
}
getNavItemHeadings(parent: ElementFinder, level: number) {
const targetSelector = `aio-nav-item .vertical-menu-item.heading.level-${level}`;
return parent.all(by.css(targetSelector));
}
getNavItemHeadingChildren(heading: ElementFinder, level: number) {
const targetSelector = `.heading-children.level-${level}`;
const script = `return arguments[0].parentNode.querySelector('${targetSelector}');`;
return element(() => browser.executeScript(script, heading));
}
getTopMenuLink(path) { return element(by.css(`aio-top-menu a[href="${path}"]`)); }
ga() { return browser.executeScript('return window["ga"].q') as promise.Promise<any[][]>; }
locationPath() { return browser.executeScript('return document.location.pathname') as promise.Promise<string>; }
@ -53,6 +64,10 @@ export class SitePage {
return browser.executeScript('window.scrollTo(0, document.body.scrollHeight)');
}
click(element: ElementFinder) {
return element.click().then(() => browser.waitForAngular());
}
enterSearch(query: string) {
const input = element(by.css('.search-container input[type=search]'));
input.clear();