diff --git a/aio/src/app/app.component.html b/aio/src/app/app.component.html index bad7f69913..1a7d457127 100644 --- a/aio/src/app/app.component.html +++ b/aio/src/app/app.component.html @@ -18,7 +18,7 @@ - diff --git a/aio/src/app/app.component.spec.ts b/aio/src/app/app.component.spec.ts index 4ce21669d3..bc77fed2bd 100644 --- a/aio/src/app/app.component.spec.ts +++ b/aio/src/app/app.component.spec.ts @@ -34,6 +34,7 @@ import { TocItem, TocService } from 'app/shared/toc.service'; const sideBySideBreakPoint = 992; const hideToCBreakPoint = 800; +const startedDelay = 100; describe('AppComponent', () => { let component: AppComponent; @@ -910,36 +911,68 @@ describe('AppComponent', () => { }); describe('initial rendering', () => { + beforeEach(jasmine.clock().install); + afterEach(jasmine.clock().uninstall); + + it('should initially disable Angular animations until a document is rendered', () => { + initializeTest(false); + jasmine.clock().tick(1); // triggers the HTTP response for the document + + expect(component.isStarting).toBe(true); + expect(fixture.debugElement.properties['@.disabled']).toBe(true); + + triggerDocViewerEvent('docInserted'); + jasmine.clock().tick(startedDelay); + fixture.detectChanges(); + expect(component.isStarting).toBe(true); + expect(fixture.debugElement.properties['@.disabled']).toBe(true); + + triggerDocViewerEvent('docRendered'); + jasmine.clock().tick(startedDelay); + fixture.detectChanges(); + expect(component.isStarting).toBe(false); + expect(fixture.debugElement.properties['@.disabled']).toBe(false); + }); + it('should initially add the starting class until a document is rendered', () => { initializeTest(false); + jasmine.clock().tick(1); // triggers the HTTP response for the document const sidenavContainer = fixture.debugElement.query(By.css('mat-sidenav-container')).nativeElement; expect(component.isStarting).toBe(true); + expect(hamburger.classList.contains('starting')).toBe(true); expect(sidenavContainer.classList.contains('starting')).toBe(true); triggerDocViewerEvent('docInserted'); + jasmine.clock().tick(startedDelay); fixture.detectChanges(); expect(component.isStarting).toBe(true); + expect(hamburger.classList.contains('starting')).toBe(true); expect(sidenavContainer.classList.contains('starting')).toBe(true); triggerDocViewerEvent('docRendered'); + jasmine.clock().tick(startedDelay); fixture.detectChanges(); expect(component.isStarting).toBe(false); + expect(hamburger.classList.contains('starting')).toBe(false); expect(sidenavContainer.classList.contains('starting')).toBe(false); }); it('should initially disable animations on the DocViewer for the first rendering', () => { initializeTest(false); + jasmine.clock().tick(1); // triggers the HTTP response for the document expect(component.isStarting).toBe(true); expect(docViewer.classList.contains('no-animations')).toBe(true); triggerDocViewerEvent('docInserted'); + jasmine.clock().tick(startedDelay); fixture.detectChanges(); expect(component.isStarting).toBe(true); expect(docViewer.classList.contains('no-animations')).toBe(true); triggerDocViewerEvent('docRendered'); + jasmine.clock().tick(startedDelay); fixture.detectChanges(); expect(component.isStarting).toBe(false); expect(docViewer.classList.contains('no-animations')).toBe(false); diff --git a/aio/src/app/app.component.ts b/aio/src/app/app.component.ts index ae48e6533c..094688dccd 100644 --- a/aio/src/app/app.component.ts +++ b/aio/src/app/app.component.ts @@ -28,7 +28,7 @@ export class AppComponent implements OnInit { currentDocument: DocumentContents; currentDocVersion: NavigationNode; - currentNodes: CurrentNodes; + currentNodes: CurrentNodes = {}; currentPath: string; docVersions: NavigationNode[]; dtOn = false; @@ -57,9 +57,11 @@ export class AppComponent implements OnInit { @HostBinding('class') hostClasses = ''; - isFetching = false; + // Disable all Angular animations for the initial render. + @HostBinding('@.disabled') isStarting = true; isTransitioning = true; + isFetching = false; isSideBySide = false; private isFetchingTimeout: any; private isSideNavDoc = false; @@ -118,12 +120,6 @@ export class AppComponent implements OnInit { /* No need to unsubscribe because this root component never dies */ this.documentService.currentDocument.subscribe(doc => this.currentDocument = doc); - // Generally, we want to delay updating the host classes for the new document, until after the - // leaving document has been removed (to avoid having the styles for the new document applied - // prematurely). - // On the first document, though, (when we know there is no previous document), we want to - // ensure the styles are applied as soon as possible to avoid flicker. - this.documentService.currentDocument.first().subscribe(doc => this.updateHostClassesForDoc(doc)); this.locationService.currentPath.subscribe(path => { // Redirect to docs if we are in archive mode and are not hitting a docs page @@ -175,11 +171,22 @@ export class AppComponent implements OnInit { this.topMenuNarrowNodes = views['TopBarNarrow'] || this.topMenuNodes; }); - this.navigationService.versionInfo.subscribe( vi => this.versionInfo = vi ); + this.navigationService.versionInfo.subscribe(vi => this.versionInfo = vi); const hasNonEmptyToc = this.tocService.tocList.map(tocList => tocList.length > 0); combineLatest(hasNonEmptyToc, this.showFloatingToc) .subscribe(([hasToc, showFloatingToc]) => this.hasFloatingToc = hasToc && showFloatingToc); + + // Generally, we want to delay updating the shell (e.g. host classes, sidenav state) for the new + // document, until after the leaving document has been removed (to avoid having the styles for + // the new document applied prematurely). + // For the first document, though, (when we know there is no previous document), we want to + // ensure the styles are applied as soon as possible to avoid flicker. + combineLatest( + this.documentService.currentDocument, // ...needed to determine host classes + this.navigationService.currentNodes) // ...needed to determine `sidenav` state + .first() + .subscribe(() => this.updateShell()); } // Scroll to the anchor in the hash fragment or top of doc. @@ -205,14 +212,11 @@ export class AppComponent implements OnInit { } onDocInserted() { - // TODO: Find a better way to avoid `ExpressionChangedAfterItHasBeenChecked` error. - setTimeout(() => { - // Update the SideNav state (if necessary). - this.updateSideNav(); - - // Update the host classes to match the new document. - this.updateHostClassesForDoc(this.currentDocument); - }); + // Update the shell (host classes, sidenav state) to match the new document. + // This may be called as a result of actions initiated by view updates. + // In order to avoid errors (e.g. `ExpressionChangedAfterItHasBeenChecked`), updating the view + // (e.g. sidenav, host classes) needs to happen asynchronously. + setTimeout(() => this.updateShell()); // Scroll 500ms after the new document has been inserted into the doc-viewer. // The delay is to allow time for async layout to complete. @@ -220,7 +224,14 @@ export class AppComponent implements OnInit { } onDocRendered() { - this.isStarting = false; + if (this.isStarting) { + // In order to ensure that the initial sidenav-content left margin + // adjustment happens without animation, we need to ensure that + // `isStarting` remains `true` until the margin change is triggered. + // (Apparently, this happens with a slight delay.) + setTimeout(() => this.isStarting = false, 100); + } + this.isTransitioning = false; } @@ -241,7 +252,7 @@ export class AppComponent implements OnInit { // items in the top-bar, ensure the sidenav is closed. // (This condition can only be met when the resize event changes the value of `isSideBySide` // from `false` to `true` while on a non-sidenav doc.) - this.sideNavToggle(false); + this.sidenav.toggle(false); } } @@ -272,10 +283,6 @@ export class AppComponent implements OnInit { return true; } - sideNavToggle(value?: boolean) { - this.sidenav.toggle(value); - } - setPageId(id: string) { // Special case the home page this.pageId = (id === 'index') ? 'home' : id.replace('/', '-'); @@ -300,7 +307,7 @@ export class AppComponent implements OnInit { const sideNavOpen = `sidenav-${this.sidenav.opened ? 'open' : 'closed'}`; const pageClass = `page-${this.pageId}`; const folderClass = `folder-${this.folderId}`; - const viewClasses = Object.keys(this.currentNodes || {}).map(view => `view-${view}`).join(' '); + const viewClasses = Object.keys(this.currentNodes).map(view => `view-${view}`).join(' '); const notificationClass = `aio-notification-${this.notification.showNotification}`; const notificationAnimatingClass = this.notificationAnimating ? 'aio-notification-animating' : ''; @@ -315,9 +322,13 @@ export class AppComponent implements OnInit { ].join(' '); } - updateHostClassesForDoc(doc: DocumentContents) { - this.setPageId(doc.id); - this.setFolderId(doc.id); + updateShell() { + // Update the SideNav state (if necessary). + this.updateSideNav(); + + // Update the host classes. + this.setPageId(this.currentDocument.id); + this.setFolderId(this.currentDocument.id); this.updateHostClasses(); } @@ -333,7 +344,7 @@ export class AppComponent implements OnInit { } // May be open or closed when wide; always closed when narrow. - this.sideNavToggle(this.isSideBySide && openSideNav); + this.sidenav.toggle(this.isSideBySide && openSideNav); } // Dynamically change height of table of contents container diff --git a/aio/src/styles/1-layouts/_sidenav.scss b/aio/src/styles/1-layouts/_sidenav.scss index 4aa2f583f6..2e8b8c7baf 100644 --- a/aio/src/styles/1-layouts/_sidenav.scss +++ b/aio/src/styles/1-layouts/_sidenav.scss @@ -1,11 +1,6 @@ -// Disable sidenav animations while starting the app -// See https://github.com/angular/material2/blob/master/src/lib/sidenav/sidenav-transitions.scss -.starting.mat-sidenav-transition { - .mat-sidenav, - .mat-sidenav-content, - .mat-sidenav-backdrop.mat-sidenav-shown { - transition: none; - } +// Disable sidenav animations for the initial render. +.starting.mat-drawer-transition .mat-drawer-content { + transition: none; } aio-nav-menu { @@ -42,19 +37,19 @@ mat-sidenav.mat-sidenav.sidenav { } mat-sidenav-container.sidenav-container { - min-height: 100%; - height: auto !important; - max-width: 100%; - margin: 0; - transform: none; + min-height: 100%; + height: auto !important; + max-width: 100%; + margin: 0; + transform: none; - &.has-floating-toc { - max-width: 82%; - } + &.has-floating-toc { + max-width: 82%; + } } mat-sidenav-container div.mat-sidenav-content { - height: auto; + height: auto; } .vertical-menu-item { @@ -165,7 +160,6 @@ button.vertical-menu-item { .level-1:not(.expanded) .mat-icon, .level-2:not(.expanded) .mat-icon { @include rotate(0deg); - // margin: 4px; } aio-nav-menu.top-menu { diff --git a/aio/src/styles/1-layouts/_top-menu.scss b/aio/src/styles/1-layouts/_top-menu.scss index 5fc6611596..71da0aaf21 100644 --- a/aio/src/styles/1-layouts/_top-menu.scss +++ b/aio/src/styles/1-layouts/_top-menu.scss @@ -68,9 +68,12 @@ aio-shell.folder-tutorial mat-toolbar.mat-toolbar { height: 100%; margin: $hamburgerShownMargin; padding: 0; - transition-duration: .4s; - transition-property: color, margin; - transition-timing-function: cubic-bezier(.25, .8, .25, 1); + + &:not(.starting) { + transition-duration: .4s; + transition-property: color, margin; + transition-timing-function: cubic-bezier(.25, .8, .25, 1); + } @media (min-width: 992px) { // Hamburger hidden by default on large screens.