From 557cf7dc63fe768900f296252e1a96e6166b24df Mon Sep 17 00:00:00 2001 From: dario-piotrowicz Date: Mon, 27 Jun 2022 23:03:35 +0100 Subject: [PATCH] fix(docs-infra): convert docs select for versions into navigation (#46674) convert the select for the docs versions into a proper navigation to make it more clear for users and also to improve its accessibility resolves #44339 PR Close #46674 --- aio/src/app/app.component.html | 2 +- aio/src/app/app.component.spec.ts | 91 ++++++++----------- aio/src/app/app.component.ts | 48 ++++------ .../app/layout/nav-menu/nav-menu.component.ts | 17 +++- .../styles/1-layouts/sidenav/_sidenav.scss | 5 - goldens/size-tracking/aio-payloads.json | 4 +- 6 files changed, 73 insertions(+), 94 deletions(-) diff --git a/aio/src/app/app.component.html b/aio/src/app/app.component.html index a11c0ba996c..c5339642d2d 100644 --- a/aio/src/app/app.component.html +++ b/aio/src/app/app.component.html @@ -64,7 +64,7 @@
- +
diff --git a/aio/src/app/app.component.spec.ts b/aio/src/app/app.component.spec.ts index 01144ddef6f..a1980fd238a 100644 --- a/aio/src/app/app.component.spec.ts +++ b/aio/src/app/app.component.spec.ts @@ -1,6 +1,6 @@ import { APP_BASE_HREF } from '@angular/common'; import { HttpClient } from '@angular/common/http'; -import { DebugElement, NO_ERRORS_SCHEMA } from '@angular/core'; +import { NO_ERRORS_SCHEMA } from '@angular/core'; import { ComponentFixture, fakeAsync, flushMicrotasks, inject, TestBed, tick } from '@angular/core/testing'; import { MatProgressBar } from '@angular/material/progress-bar'; import { MatSidenav } from '@angular/material/sidenav'; @@ -19,7 +19,6 @@ import { LocationService } from 'app/shared/location.service'; import { Logger } from 'app/shared/logger.service'; import { ScrollService } from 'app/shared/scroll.service'; import { SearchResultsComponent } from 'app/shared/search-results/search-results.component'; -import { SelectComponent } from 'app/shared/select/select.component'; import { TocItem, TocService } from 'app/shared/toc.service'; import { SwUpdatesService } from 'app/sw-updates/sw-updates.service'; import { of, Subject, timer } from 'rxjs'; @@ -381,68 +380,52 @@ describe('AppComponent', () => { }); }); - describe('SideNav version selector', () => { - let selectElement: DebugElement; - let selectComponent: SelectComponent; - - async function setupSelectorForTesting(mode?: string) { + describe('SideNav version navigation', () => { + async function setupVersionsNavForTesting(mode?: string) { createTestingModule('a/b', mode); await initializeTest(); component.onResize(dockSideNavWidth + 1); // wide view - selectElement = fixture.debugElement.query(By.directive(SelectComponent)); - selectComponent = selectElement.componentInstance; } - it('should select the version that matches the deploy mode', async () => { - await setupSelectorForTesting(); - expect(selectComponent.selected.title).toContain('stable'); - await setupSelectorForTesting('next'); - expect(selectComponent.selected.title).toContain('next'); - await setupSelectorForTesting('archive'); - expect(selectComponent.selected.title).toContain('v4'); + function getAllVersionUrls() { + return (component.docVersions?.[0].children ?? []).map( + item => item?.url + ) as string[]; + } + + it('should present as current the version that matches the deploy mode', async () => { + await setupVersionsNavForTesting(); + expect(component.currentDocsVersionNode?.title).toContain('stable'); + await setupVersionsNavForTesting('next'); + expect(component.currentDocsVersionNode?.title).toContain('next'); + await setupVersionsNavForTesting('archive'); + expect(component.currentDocsVersionNode?.title).toContain('v4'); }); - it('should add the current raw version string to the selected version', async () => { - await setupSelectorForTesting(); - expect(selectComponent.selected.title).toContain(`(v${component.versionInfo.raw})`); - await setupSelectorForTesting('next'); - expect(selectComponent.selected.title).toContain(`(v${component.versionInfo.raw})`); - await setupSelectorForTesting('archive'); - expect(selectComponent.selected.title).toContain(`(v${component.versionInfo.raw})`); + it('should provide the correct url for each nav item', async () => { + await setupVersionsNavForTesting(); + const allVersionUrls = getAllVersionUrls(); + expect(allVersionUrls.length).toBeGreaterThan(0); + allVersionUrls.forEach(versionUrl => + expect(versionUrl).toMatch(/^https:\/\/.*\/a\/b$/) + ); }); - // Older docs versions have an href - it('should navigate when change to a version with a url', async () => { - await setupSelectorForTesting(); - locationService.urlSubject.next('new-page?id=1#section-1'); - const versionWithUrlIndex = component.docVersions.findIndex(v => !!v.url); - const versionWithUrl = component.docVersions[versionWithUrlIndex]; - const versionWithUrlAndPage = `${versionWithUrl.url}new-page?id=1#section-1`; - selectElement.triggerEventHandler('change', { option: versionWithUrl, index: versionWithUrlIndex}); - expect(locationService.go).toHaveBeenCalledWith(versionWithUrlAndPage); - }); + it('should update the urls on page changes', async () => { + function testUrlsOnRoute(route: string) { + locationService.urlSubject.next(route); + const allVersionUrls = getAllVersionUrls(); + expect(allVersionUrls.length).toBeGreaterThan(0); + const escapedRoute = route.replace('?', '\\?'); + allVersionUrls.forEach(versionUrl => + expect(versionUrl).toMatch(new RegExp(`^https://.*/${escapedRoute}`)) + ); + } - it('should not navigate when change to a version without a url', async () => { - await setupSelectorForTesting(); - const versionWithoutUrlIndex = component.docVersions.length; - const versionWithoutUrl = component.docVersions[versionWithoutUrlIndex] = { title: 'foo' }; - selectElement.triggerEventHandler('change', { option: versionWithoutUrl, index: versionWithoutUrlIndex }); - expect(locationService.go).not.toHaveBeenCalled(); - }); - - it('should navigate when change to a version with a url that does not end with `/`', async () => { - await setupSelectorForTesting(); - locationService.urlSubject.next('docs#section-1'); - const versionWithoutSlashIndex = component.docVersions.length; - const versionWithoutSlashUrl = (component.docVersions[versionWithoutSlashIndex] = { - url: 'https://next.angular.io', - title: 'foo', - }); - selectElement.triggerEventHandler('change', { - option: versionWithoutSlashUrl, - index: versionWithoutSlashIndex, - }); - expect(locationService.go).toHaveBeenCalledWith('https://next.angular.io/docs#section-1'); + await setupVersionsNavForTesting(); + testUrlsOnRoute('new-page?id=1#section-1'); + testUrlsOnRoute('new-new-page?id=2#section-2'); + testUrlsOnRoute('new/new-new-page'); }); }); diff --git a/aio/src/app/app.component.ts b/aio/src/app/app.component.ts index 9c6b057e726..ab95d52b460 100644 --- a/aio/src/app/app.component.ts +++ b/aio/src/app/app.component.ts @@ -22,7 +22,6 @@ import { TocService } from 'app/shared/toc.service'; import { SwUpdatesService } from 'app/sw-updates/sw-updates.service'; import { BehaviorSubject, combineLatest, Observable } from 'rxjs'; import { first, map } from 'rxjs/operators'; -import { SelectComponent } from './shared/select/select.component'; const sideNavView = 'SideNav'; export const showTopMenuWidth = 1150; @@ -89,9 +88,9 @@ export class AppComponent implements OnInit { tocMaxHeight: string; private tocMaxHeightOffset = 0; - versionInfo: VersionInfo; + currentDocsVersionNode?: NavigationNode; - private currentUrl: string; + versionInfo: VersionInfo; get isOpened() { return this.dockSideNav && this.isSideNavDoc; } get mode() { return this.isOpened ? 'side' : 'over'; } @@ -117,9 +116,6 @@ export class AppComponent implements OnInit { @ViewChildren('themeToggle, externalIcons', { read: ElementRef }) toolbarIcons: QueryList; - @ViewChild(SelectComponent, { read: ElementRef }) - docVersionSelectElement: ElementRef; - constructor( public deployment: Deployment, private documentService: DocumentService, @@ -173,7 +169,8 @@ export class AppComponent implements OnInit { combineLatest([ this.navigationService.versionInfo, this.navigationService.navigationViews.pipe(map(views => views.docVersions)), - ]).subscribe(([versionInfo, versions]) => { + this.locationService.currentUrl, + ]).subscribe(([versionInfo, versions, currentUrl]) => { // TODO(pbd): consider whether we can lookup the stable and next versions from the internet const computedVersions: NavigationNode[] = [ { title: 'next', url: 'https://next.angular.io/' }, @@ -183,13 +180,22 @@ export class AppComponent implements OnInit { if (this.deployment.mode === 'archive') { computedVersions.push({ title: `v${versionInfo.major}` }); } - this.docVersions = [...computedVersions, ...versions]; - + const allDocsVersionNodes = [...computedVersions, ...versions].map(version => ({ + ...version, + // Update the urls so that they point to the same page the user is currently at + url: `${version.url}${(version.url?.endsWith('/') ? '' : '/' )}${currentUrl}`, + })); // Find the current version - either title matches the current deployment mode // or its title matches the major version of the current version info - this.currentDocVersion = this.docVersions.find(version => - version.title === this.deployment.mode || version.title === `v${versionInfo.major}`) as NavigationNode; - this.currentDocVersion.title += ` (v${versionInfo.raw})`; + this.currentDocsVersionNode = allDocsVersionNodes.find( + version => version.title === this.deployment.mode || version.title === `v${versionInfo.major}` + ); + this.docVersions = [ + { + title: 'Docs Versions', + children : allDocsVersionNodes + } + ]; }); this.navigationService.navigationViews.subscribe(views => { @@ -216,8 +222,6 @@ export class AppComponent implements OnInit { ]).pipe(first()) .subscribe(() => this.updateShell()); - this.locationService.currentUrl.subscribe(url => this.currentUrl = url); - // Start listening for SW version update events. this.swUpdatesService.enable(); } @@ -260,14 +264,6 @@ export class AppComponent implements OnInit { this.isTransitioning = false; } - onDocVersionChange(versionIndex: number) { - const version = this.docVersions[versionIndex]; - if (version.url) { - const versionUrl = version.url + (!version.url.endsWith('/') ? '/' : ''); - this.locationService.go(`${versionUrl}${this.currentUrl}`); - } - } - @HostListener('window:resize', ['$event.target.innerWidth']) onResize(width: number) { this.showTopMenu = width >= showTopMenuWidth; @@ -481,12 +477,4 @@ export class AppComponent implements OnInit { } } } - - scrollSelectIntoView() { - // this method is used to scroll the select component into view when it is clicked/opened - // the setTimeout is needed so that the scroll happens after the component has expanded - setTimeout(() => - this.docVersionSelectElement.nativeElement?.scrollIntoView({behavior: 'smooth'}) - ); - } } diff --git a/aio/src/app/layout/nav-menu/nav-menu.component.ts b/aio/src/app/layout/nav-menu/nav-menu.component.ts index 12a78c1b6df..e5c3130289c 100644 --- a/aio/src/app/layout/nav-menu/nav-menu.component.ts +++ b/aio/src/app/layout/nav-menu/nav-menu.component.ts @@ -7,15 +7,28 @@ import { CurrentNode, NavigationNode } from 'app/navigation/navigation.service'; ` }) export class NavMenuComponent { - @Input() currentNode: CurrentNode | undefined; + @Input() currentNode: CurrentNode | NavigationNode | undefined; @Input() isWide = false; @Input() nodes: NavigationNode[]; @Input() navLabel: string; get filteredNodes() { return this.nodes ? this.nodes.filter(n => !n.hidden) : []; } + + get selectedNodes(): NavigationNode[]|undefined { + if(!this.currentNode) { + return undefined; + } + + const currentNodeNodes = (this.currentNode as CurrentNode).nodes; + if (currentNodeNodes) { + return currentNodeNodes; + } + + return [this.currentNode as NavigationNode]; + } } diff --git a/aio/src/styles/1-layouts/sidenav/_sidenav.scss b/aio/src/styles/1-layouts/sidenav/_sidenav.scss index f2a3ea9585d..ad59e4e6ae4 100644 --- a/aio/src/styles/1-layouts/sidenav/_sidenav.scss +++ b/aio/src/styles/1-layouts/sidenav/_sidenav.scss @@ -36,11 +36,6 @@ mat-sidenav-container.sidenav-container { @media (max-width: 599px) { top: 56px; } - - // Angular Version Selector - .doc-version { - padding: 8px; - } } } diff --git a/goldens/size-tracking/aio-payloads.json b/goldens/size-tracking/aio-payloads.json index 200f2dda138..1f2529edcd2 100755 --- a/goldens/size-tracking/aio-payloads.json +++ b/goldens/size-tracking/aio-payloads.json @@ -2,7 +2,7 @@ "aio": { "uncompressed": { "runtime": 4325, - "main": 461922, + "main": 457028, "polyfills": 33814, "styles": 73640, "light-theme": 78276, @@ -12,7 +12,7 @@ "aio-local": { "uncompressed": { "runtime": 4325, - "main": 462546, + "main": 457651, "polyfills": 33922, "styles": 73640, "light-theme": 78276,