fix(aio): reduce flicker and reflows for initial rendering (#21695)

For the initial rendering, where there is no transition from a previous
visual state to a new one, animations make little sense. The page should
load with as few reflows as possible.
Similarly, while we typically want to defer updating the SideNav state
(e.g. opened/closed) until the "leaving" document is animated out of the
page, on the initial rendering (where there is no "leaving" document)
this leads to the SideNav flashing (from closed to open).

These worked as expected before, but several parts (mostly related to
documents with a SideNav) have been accidentally broken in recent
commits (e.g. when upgraded to latest material, or enabled animations
for DocViewer transitions, etc.).

This commit restores the previous behavior by ensuring that (on the
initial rendering) the SideNav state is updated as soon as possible and
that there will be no animations when:

1. The hamburger button appears.
2. The SideNav is opened.
3. The main section's width is adjusted to make room for the SideNav.

PR Close #21695
This commit is contained in:
George Kalpakas 2018-01-21 14:41:52 +02:00 committed by Miško Hevery
parent bec188506c
commit 66191e8a37
5 changed files with 91 additions and 50 deletions

View File

@ -18,7 +18,7 @@
</aio-notification>
</mat-toolbar-row>
<mat-toolbar-row>
<button mat-button class="hamburger" (click)="sidenav.toggle()" title="Docs menu">
<button mat-button class="hamburger" [class.starting]="isStarting" (click)="sidenav.toggle()" title="Docs menu">
<mat-icon svgIcon="menu"></mat-icon>
</button>
<a class="nav-link home" href="/" [ngSwitch]="isSideBySide">

View File

@ -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);

View File

@ -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

View File

@ -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 {

View File

@ -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.