Skip to content

Commit 8ac588e

Browse files
Port #1350 to dtq-dev-9-base: UFAL/Backport #1333: admin-sidebar gutter via CSS var (no logged-in reload shift) (#1512)
For an authenticated user a hard reload shifted the whole page right by the admin-sidebar width. The `.outer-wrapper` left gutter was produced by the `@slideSidebarPadding` animation, whose width comes from `CSSVariableService`, a browser-only store fed from `document.styleSheets`. On the server that store is empty, so SSR emitted `style="padding-left: 0;"` and the browser resolved the real width afterwards - hence the jump. Measured on dev-6 before this change: <div _ngcontent-... class="outer-wrapper ng-tns-c4194216939-0 ng-trigger ng-trigger-slideSidebarPadding" style="padding-left: 0;"> The gutter now comes from a CSS class - `ds-admin-sidebar-{hidden,unpinned,pinned}`, derived from a small `sidebarPaddingState$` - whose `padding-left` resolves from the `--ds-admin-sidebar-*` custom properties. Those are defined in the render-blocking theme stylesheet (`--ds-admin-sidebar-fixed-element-width: 55px`, `--ds-admin-sidebar-total-width: 305px` on dev-6), so CSS resolves the gutter identically on the server and in the browser, with no hardcoded width and no browser-only variable read. The pin/unpin slide is preserved by `transition: padding-left 300ms`, gated behind `ds-admin-sidebar-animate`, which `ngAfterViewInit` enables only after the first paint so the initial SSR->CSR resolution never animates. Adaptations to the v9 base (the source commit targets the 7.x root component): * `.outer-wrapper` on 9-base already carries `[ngClass]="browserOsClasses…"`, a vanilla-9 feature 7.x does not have. Angular allows one `[ngClass]` per element, and overwriting it would silently drop `browser-firefox` / `browser-firefox-windows`, which `_custom_variables.scss` uses. The two are therefore merged into a single `outerWrapperClasses$` stream instead of pasting the source's `[ngClass]="'ds-admin-sidebar-' + (…)"`. A unit test guards the merge. * the sidebar element is `<ds-admin-sidebar>` on v9, not `<ds-themed-admin-sidebar>`; that context line is left as it is. * the source's trailing-comma reformat of the `windowService` constructor parameter is already present on 9-base, and `AfterViewInit` joins a multi-line `@angular/core` import, so those two hunks are no-ops here. `slideSidebarPadding` itself is deliberately left in `src/app/shared/animations/slide.ts`: after this change it still has one consumer, the `custom` theme's `file-section.component.ts`. Two unit tests are added beyond the source commit, which ships none. The v9 `root.component.spec.ts` had a single `it('should create')` that cannot detect this regression. Both new tests were proven load-bearing with negative controls (see the PR description). Source: 1321b8a (dtq-dev PR #1350) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 0300412 commit 8ac588e

5 files changed

Lines changed: 132 additions & 9 deletions

File tree

src/app/root/root.component.html

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,9 @@
22
{{ 'root.skip-to-content' | translate }}
33
</button>
44

5-
<div class="outer-wrapper" [class.d-none]="shouldShowFullscreenLoader" [ngClass]="browserOsClasses.asObservable() | async" [@slideSidebarPadding]="{
6-
value: ((isSidebarVisible$ | async) !== true ? 'hidden' : (slideSidebarOver$ | async) ? 'unpinned' : 'pinned'),
7-
params: { collapsedWidth: (collapsedSidebarWidth$ | async), expandedWidth: (expandedSidebarWidth$ | async) }
8-
}">
5+
<div class="outer-wrapper" [class.d-none]="shouldShowFullscreenLoader"
6+
[class.ds-admin-sidebar-animate]="gutterTransitionEnabled"
7+
[ngClass]="outerWrapperClasses$ | async">
98
<ds-admin-sidebar [expandedSidebarWidth$]="expandedSidebarWidth$" [collapsedSidebarWidth$]="collapsedSidebarWidth$"></ds-admin-sidebar>
109
<div class="inner-wrapper">
1110
<ds-system-wide-alert-banner></ds-system-wide-alert-banner>

src/app/root/root.component.scss

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,3 +14,31 @@
1414
top: 0;
1515
}
1616
}
17+
18+
// Admin-sidebar left gutter. Driven by the `ds-admin-sidebar-*` class set in root.component.html (from
19+
// sidebarPaddingState$) rather than the @slideSidebarPadding Angular animation. The animation needed a
20+
// concrete width from the browser-only CSS-variable store, so on the server it rendered padding-left:0
21+
// and the authenticated page jumped right when the anti-flicker SSR snapshot was removed. Resolving the
22+
// gutter from the `--ds-admin-sidebar-*` custom properties in CSS instead renders identically on the
23+
// server (snapshot) and the browser (live app) - no hardcoded px, theme- and viewport-aware - and the
24+
// transition keeps the pin/unpin slide. 'hidden' (no admin sidebar) keeps the default padding-left: 0.
25+
.outer-wrapper {
26+
// padding-left:0 (no admin sidebar); explicit for self-documentation.
27+
&.ds-admin-sidebar-hidden {
28+
padding-left: 0;
29+
}
30+
31+
&.ds-admin-sidebar-unpinned {
32+
padding-left: var(--ds-admin-sidebar-fixed-element-width);
33+
}
34+
35+
&.ds-admin-sidebar-pinned {
36+
padding-left: var(--ds-admin-sidebar-total-width);
37+
}
38+
39+
// Slide only genuine pin/unpin toggles. The class is added after first paint (gutterTransitionEnabled)
40+
// so the initial SSR->CSR gutter resolution behind the anti-flicker overlay never animates.
41+
&.ds-admin-sidebar-animate {
42+
transition: padding-left 300ms ease-in-out;
43+
}
44+
}

src/app/root/root.component.spec.ts

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
import { NoopAnimationsModule } from '@angular/platform-browser/animations';
88
import { Router } from '@angular/router';
99
import { TranslateModule } from '@ngx-translate/core';
10+
import { of } from 'rxjs';
1011

1112
import { AccessibilitySettingsService } from '../accessibility/accessibility-settings.service';
1213
import { AccessibilitySettingsServiceStub } from '../accessibility/accessibility-settings.service.stub';
@@ -72,4 +73,50 @@ describe('RootComponent', () => {
7273
it('should create', () => {
7374
expect(component).toBeTruthy();
7475
});
76+
77+
it('should emit the hidden gutter state when the admin sidebar is not visible', () => {
78+
spyOn(TestBed.inject(MenuService), 'isMenuVisibleWithVisibleSections').and.returnValue(of(false));
79+
component.ngOnInit();
80+
component.browserOsClasses.next(['browser-firefox', 'browser-firefox-windows']);
81+
fixture.detectChanges();
82+
83+
let state: string;
84+
component.sidebarPaddingState$.subscribe((value: string) => state = value);
85+
expect(state).toEqual('hidden');
86+
87+
// the gutter class must be merged with the browser/OS classes, not replace them
88+
let classes: string[];
89+
component.outerWrapperClasses$.subscribe((value: string[]) => classes = value);
90+
expect(classes).toEqual(['browser-firefox', 'browser-firefox-windows', 'ds-admin-sidebar-hidden']);
91+
92+
const wrapper: HTMLElement = fixture.nativeElement.querySelector('.outer-wrapper');
93+
expect(Array.from(wrapper.classList)).toContain('browser-firefox');
94+
expect(Array.from(wrapper.classList)).toContain('browser-firefox-windows');
95+
expect(Array.from(wrapper.classList)).toContain('ds-admin-sidebar-hidden');
96+
});
97+
98+
it('should enable the gutter transition only after the first paint', () => {
99+
const paints: FrameRequestCallback[] = [];
100+
spyOn(window, 'requestAnimationFrame').and.callFake((callback: FrameRequestCallback) => {
101+
paints.push(callback);
102+
return 0;
103+
});
104+
105+
const gutterFixture = TestBed.createComponent(RootComponent);
106+
gutterFixture.detectChanges();
107+
const wrapper: HTMLElement = gutterFixture.nativeElement.querySelector('.outer-wrapper');
108+
109+
// the gutter itself is rendered right away (this is what removes the SSR -> CSR jump) ...
110+
expect(Array.from(wrapper.classList)).toContain('ds-admin-sidebar-pinned');
111+
// ... but it must not animate before the first paint
112+
expect(gutterFixture.componentInstance.gutterTransitionEnabled).toBeFalse();
113+
expect(Array.from(wrapper.classList)).not.toContain('ds-admin-sidebar-animate');
114+
115+
expect(paints.length).toBeGreaterThan(0);
116+
paints.forEach((callback: FrameRequestCallback) => callback(0));
117+
gutterFixture.detectChanges();
118+
119+
expect(gutterFixture.componentInstance.gutterTransitionEnabled).toBeTrue();
120+
expect(Array.from(wrapper.classList)).toContain('ds-admin-sidebar-animate');
121+
});
75122
});

src/app/root/root.component.ts

Lines changed: 54 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import {
33
NgClass,
44
} from '@angular/common';
55
import {
6+
AfterViewInit,
67
Component,
78
Inject,
89
Input,
@@ -38,7 +39,6 @@ import {
3839
} from '../core/services/window.service';
3940
import { ThemedFooterComponent } from '../footer/themed-footer.component';
4041
import { ThemedHeaderNavbarWrapperComponent } from '../header-nav-wrapper/themed-header-navbar-wrapper.component';
41-
import { slideSidebarPadding } from '../shared/animations/slide';
4242
import { HostWindowService } from '../shared/host-window.service';
4343
import { LiveRegionComponent } from '../shared/live-region/live-region.component';
4444
import { ThemedLoadingComponent } from '../shared/loading/themed-loading.component';
@@ -52,7 +52,6 @@ import { SystemWideAlertBannerComponent } from '../system-wide-alert/alert-banne
5252
selector: 'ds-base-root',
5353
templateUrl: './root.component.html',
5454
styleUrls: ['./root.component.scss'],
55-
animations: [slideSidebarPadding],
5655
imports: [
5756
AsyncPipe,
5857
LiveRegionComponent,
@@ -68,12 +67,37 @@ import { SystemWideAlertBannerComponent } from '../system-wide-alert/alert-banne
6867
TranslateModule,
6968
],
7069
})
71-
export class RootComponent implements OnInit {
70+
export class RootComponent implements OnInit, AfterViewInit {
7271
theme: Observable<ThemeConfig> = of({} as any);
7372
isSidebarVisible$: Observable<boolean>;
7473
slideSidebarOver$: Observable<boolean>;
7574
collapsedSidebarWidth$: Observable<string>;
7675
expandedSidebarWidth$: Observable<string>;
76+
77+
/**
78+
* The admin-sidebar padding state ('hidden' | 'unpinned' | 'pinned') used to drive the
79+
* outer-wrapper's left gutter via CSS classes (see root.component.scss) instead of an Angular
80+
* animation. CSS resolves the gutter width from the `--ds-admin-sidebar-*` custom properties, so it
81+
* is rendered identically on the server (the anti-flicker SSR snapshot) and the browser (the live
82+
* app) - no browser-only CSS-variable read, no hardcoded px, and it stays theme- and viewport-aware.
83+
*/
84+
sidebarPaddingState$: Observable<string>;
85+
86+
/**
87+
* The classes on the outer wrapper: the browser/OS classes this branch already carries, plus the
88+
* admin-sidebar gutter class. Merged into a single stream because Angular allows one [ngClass]
89+
* binding per element and the wrapper already has one.
90+
*/
91+
outerWrapperClasses$: Observable<string[]>;
92+
93+
/**
94+
* Enables the gutter's `transition: padding-left` only AFTER the first browser paint. The initial
95+
* SSR->CSR gutter resolution happens behind the anti-flicker overlay; without this gate a plain CSS
96+
* transition would animate that initial 0->gutter change (the overlay settle detector only watches
97+
* DOM mutations, not style changes), which could leak a 300ms slide right as the overlay is removed.
98+
* Off on the server and on first render, so only genuine pin/unpin toggles animate.
99+
*/
100+
gutterTransitionEnabled = false;
77101
notificationOptions: INotificationBoardOptions;
78102
models: any;
79103

@@ -113,6 +137,8 @@ export class RootComponent implements OnInit {
113137

114138
this.isSidebarVisible$ = this.menuService.isMenuVisibleWithVisibleSections(MenuID.ADMIN);
115139

140+
// Still provided to <ds-admin-sidebar>; the sidebar element itself sizes from CSS vars, so a
141+
// null value on the server (the store is browser-only) is harmless there.
116142
this.expandedSidebarWidth$ = this.cssService.getVariable('--ds-admin-sidebar-total-width').pipe(
117143
skipWhile((val) => !val),
118144
first(),
@@ -129,11 +155,36 @@ export class RootComponent implements OnInit {
129155
startWith(true),
130156
);
131157

158+
// Drive the outer-wrapper gutter via a CSS class instead of the @slideSidebarPadding animation: the
159+
// animation needs a concrete width from the browser-only CSS-variable store, so on the server it
160+
// rendered padding-left:0 and the authenticated page jumped right when the SSR snapshot was removed.
161+
// The CSS class resolves the gutter from `--ds-admin-sidebar-*` (see root.component.scss), identically
162+
// on server and browser - fixing the jump without any hardcoded width.
163+
this.sidebarPaddingState$ = combineLatestObservable([this.isSidebarVisible$, this.slideSidebarOver$]).pipe(
164+
map(([visible, over]) => !visible ? 'hidden' : over ? 'unpinned' : 'pinned'),
165+
);
166+
this.outerWrapperClasses$ = combineLatestObservable([
167+
this.browserOsClasses.asObservable(),
168+
this.sidebarPaddingState$,
169+
]).pipe(
170+
map(([osClasses, state]) => [...osClasses, `ds-admin-sidebar-${state}`]),
171+
);
172+
132173
if (this.router.url === getPageInternalServerErrorRoute()) {
133174
this.shouldShowRouteLoader = false;
134175
}
135176
}
136177

178+
ngAfterViewInit(): void {
179+
// Enable the gutter slide only after the first paint (browser only; requestAnimationFrame is not
180+
// defined under SSR), so the initial padding resolution never animates - see gutterTransitionEnabled.
181+
if (typeof requestAnimationFrame === 'function') {
182+
requestAnimationFrame(() => {
183+
this.gutterTransitionEnabled = true;
184+
});
185+
}
186+
}
187+
137188
skipToMainContent() {
138189
const mainContent = document.getElementById('main-content');
139190
if (mainContent) {

src/themes/custom/app/root/root.component.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import { ThemedBreadcrumbsComponent } from '../../../../app/breadcrumbs/themed-b
1111
import { ThemedFooterComponent } from '../../../../app/footer/themed-footer.component';
1212
import { ThemedHeaderNavbarWrapperComponent } from '../../../../app/header-nav-wrapper/themed-header-navbar-wrapper.component';
1313
import { RootComponent as BaseComponent } from '../../../../app/root/root.component';
14-
import { slideSidebarPadding } from '../../../../app/shared/animations/slide';
1514
import { LiveRegionComponent } from '../../../../app/shared/live-region/live-region.component';
1615
import { ThemedLoadingComponent } from '../../../../app/shared/loading/themed-loading.component';
1716
import { NotificationsBoardComponent } from '../../../../app/shared/notifications/notifications-board/notifications-board.component';
@@ -23,7 +22,6 @@ import { SystemWideAlertBannerComponent } from '../../../../app/system-wide-aler
2322
styleUrls: ['../../../../app/root/root.component.scss'],
2423
// templateUrl: './root.component.html',
2524
templateUrl: '../../../../app/root/root.component.html',
26-
animations: [slideSidebarPadding],
2725
imports: [
2826
AsyncPipe,
2927
LiveRegionComponent,

0 commit comments

Comments
 (0)