Skip to content

Commit 7046b74

Browse files
jr-rkclaude
andcommitted
refactor(ssr): narrow sidebar padding state type and subscribe once
Address Copilot review on PR #16: - Type sidebarPaddingState$ as the 'hidden' | 'unpinned' | 'pinned' union instead of string, for template type-safety. - Collapse the three `sidebarPaddingState$ | async` template evaluations into a single `[ngClass]` stream (outerWrapperNgClass$) so the observable and its sources are subscribed once. Seeded startWith('hidden') so the browser/OS classes never wait on the sidebar stream's first emit. Behavior-preserving: same ds-admin-sidebar-* classes, same CSS. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 9ff23d4 commit 7046b74

2 files changed

Lines changed: 22 additions & 7 deletions

File tree

src/app/root/root.component.html

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,8 @@
33
</button>
44

55
<div class="outer-wrapper" [class.d-none]="shouldShowFullscreenLoader"
6-
[ngClass]="browserOsClasses.asObservable() | async"
7-
[class.ds-admin-sidebar-animate]="gutterTransitionEnabled"
8-
[class.ds-admin-sidebar-hidden]="(sidebarPaddingState$ | async) === 'hidden'"
9-
[class.ds-admin-sidebar-unpinned]="(sidebarPaddingState$ | async) === 'unpinned'"
10-
[class.ds-admin-sidebar-pinned]="(sidebarPaddingState$ | async) === 'pinned'">
6+
[ngClass]="outerWrapperNgClass$ | async"
7+
[class.ds-admin-sidebar-animate]="gutterTransitionEnabled">
118
<ds-admin-sidebar [expandedSidebarWidth$]="expandedSidebarWidth$" [collapsedSidebarWidth$]="collapsedSidebarWidth$"></ds-admin-sidebar>
129
<div class="inner-wrapper">
1310
<ds-system-wide-alert-banner></ds-system-wide-alert-banner>

src/app/root/root.component.ts

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,9 @@ import { NotificationsBoardComponent } from '../shared/notifications/notificatio
4949
import { CSSVariableService } from '../shared/sass-helper/css-variable.service';
5050
import { SystemWideAlertBannerComponent } from '../system-wide-alert/alert-banner/system-wide-alert-banner.component';
5151

52+
/** Admin-sidebar left-gutter state; maps to the `ds-admin-sidebar-*` classes in root.component.scss. */
53+
type SidebarPaddingState = 'hidden' | 'unpinned' | 'pinned';
54+
5255
@Component({
5356
selector: 'ds-base-root',
5457
templateUrl: './root.component.html',
@@ -84,7 +87,15 @@ export class RootComponent implements OnInit, AfterViewInit {
8487
* is rendered identically on the server (the anti-flicker SSR snapshot) and the browser (the live
8588
* app) — no browser-only CSS-variable read, no hardcoded px, and it stays theme- and viewport-aware.
8689
*/
87-
sidebarPaddingState$: Observable<string>;
90+
sidebarPaddingState$: Observable<SidebarPaddingState>;
91+
92+
/**
93+
* `outer-wrapper` classes as a single stream so the template subscribes once (one `[ngClass]`)
94+
* instead of evaluating `sidebarPaddingState$ | async` per gutter class. Merges the browser/OS
95+
* classes with the current `ds-admin-sidebar-*` gutter class; seeded with the `'hidden'` gutter so
96+
* the OS classes never wait on the sidebar stream's first emit.
97+
*/
98+
outerWrapperNgClass$: Observable<string[]>;
8899

89100
/**
90101
* Enables the gutter's `transition: padding-left` only AFTER the first browser paint. The initial
@@ -155,7 +166,14 @@ export class RootComponent implements OnInit, AfterViewInit {
155166
// The CSS class resolves the gutter from `--ds-admin-sidebar-*` (see root.component.scss), identically
156167
// on server and browser — fixing the jump without any hardcoded width.
157168
this.sidebarPaddingState$ = combineLatestObservable([this.isSidebarVisible$, this.slideSidebarOver$]).pipe(
158-
map(([visible, over]) => !visible ? 'hidden' : over ? 'unpinned' : 'pinned'),
169+
map(([visible, over]): SidebarPaddingState => !visible ? 'hidden' : over ? 'unpinned' : 'pinned'),
170+
);
171+
172+
this.outerWrapperNgClass$ = combineLatestObservable([
173+
this.browserOsClasses,
174+
this.sidebarPaddingState$.pipe(startWith('hidden' as SidebarPaddingState)),
175+
]).pipe(
176+
map(([osClasses, state]) => [...osClasses, `ds-admin-sidebar-${state}`]),
159177
);
160178

161179
if (this.router.url === getPageInternalServerErrorRoute()) {

0 commit comments

Comments
 (0)