Skip to content

Commit b51bedc

Browse files
author
jm
committed
Address Copilot review
- angular.json: tighten budget back to 5.5MB warn / 6MB error (was 8MB) - index.html: re-entrancy guard on __dspaceRemoveSsrOverlay (null the pointer up-front so the isStable + 15s safety fallback can't double-fade) - index.html: drop aria-hidden from overlay so screen-reader users get the SSR snapshot during boot (ds-app underneath has visibility:hidden which already excludes it from a11y tree) - index.html: console.warn on the overlay-script catch so a silently broken flicker fix is at least diagnosable in DevTools - typings.d.ts: typed Window.__dspaceRemoveSsrOverlay augmentation; drop the `as any` cast in AppComponent.removeSsrOverlayWhenStable - app.component.spec.ts: cover removeSsrOverlayWhenStable (calls the global once on isStable=true; no-op when global absent) - Drop scripts/dspace-deploy.bat + .claude/skills/dspace-deploy/SKILL.md from this PR per request (local dev tooling, will live elsewhere)
1 parent 95d6e6b commit b51bedc

7 files changed

Lines changed: 95 additions & 278 deletions

File tree

.claude/skills/dspace-deploy/SKILL.md

Lines changed: 0 additions & 75 deletions
This file was deleted.

angular.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,8 @@
9999
"budgets": [
100100
{
101101
"type": "initial",
102-
"maximumWarning": "6mb",
103-
"maximumError": "8mb"
102+
"maximumWarning": "5.5mb",
103+
"maximumError": "6mb"
104104
},
105105
{
106106
"type": "anyComponentStyle",

scripts/dspace-deploy.bat

Lines changed: 0 additions & 191 deletions
This file was deleted.

src/app/app.component.spec.ts

Lines changed: 61 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
import { Store, StoreModule } from '@ngrx/store';
2-
import { ComponentFixture, inject, TestBed, waitForAsync } from '@angular/core/testing';
3-
import { CUSTOM_ELEMENTS_SCHEMA } from '@angular/core';
2+
import { ComponentFixture, fakeAsync, flush, inject, TestBed, tick, waitForAsync } from '@angular/core/testing';
3+
import { ApplicationRef, CUSTOM_ELEMENTS_SCHEMA } from '@angular/core';
44
import { CommonModule } from '@angular/common';
55
import { ActivatedRoute, Router } from '@angular/router';
66
import { TranslateLoader, TranslateModule } from '@ngx-translate/core';
7+
import { BehaviorSubject } from 'rxjs';
78

89
// Load the implementations that should be tested
910
import { AppComponent } from './app.component';
@@ -127,4 +128,62 @@ describe('App component', () => {
127128
});
128129

129130
});
131+
132+
describe('removeSsrOverlayWhenStable', () => {
133+
// The inline bootstrap script in src/index.html injects window.__dspaceRemoveSsrOverlay
134+
// and AppComponent must call it exactly once when ApplicationRef.isStable first emits true.
135+
let appRef: ApplicationRef;
136+
let isStable$: BehaviorSubject<boolean>;
137+
let originalRaF: typeof window.requestAnimationFrame;
138+
139+
beforeEach(() => {
140+
appRef = TestBed.inject(ApplicationRef);
141+
isStable$ = new BehaviorSubject<boolean>(false);
142+
// Patch isStable to our controllable subject for this test only
143+
Object.defineProperty(appRef, 'isStable', { value: isStable$.asObservable() });
144+
145+
// Force rAF to a synchronous shim so we can flush() through the chain deterministically.
146+
originalRaF = window.requestAnimationFrame;
147+
(window as any).requestAnimationFrame = (cb: FrameRequestCallback) => {
148+
cb(0);
149+
return 0 as any;
150+
};
151+
});
152+
153+
afterEach(() => {
154+
(window as any).requestAnimationFrame = originalRaF;
155+
delete (window as any).__dspaceRemoveSsrOverlay;
156+
});
157+
158+
it('removes the overlay once isStable emits true', fakeAsync(() => {
159+
const spy = jasmine.createSpy('__dspaceRemoveSsrOverlay');
160+
window.__dspaceRemoveSsrOverlay = spy;
161+
162+
// Re-construct so the constructor-time subscription picks up our patched isStable + global.
163+
const f = TestBed.createComponent(AppComponent);
164+
f.detectChanges();
165+
166+
expect(spy).not.toHaveBeenCalled();
167+
168+
isStable$.next(true);
169+
tick(50); // matches the 50ms pad after rAF in removeSsrOverlayWhenStable
170+
flush();
171+
172+
expect(spy).toHaveBeenCalledTimes(1);
173+
}));
174+
175+
it('is a no-op when the global is not injected (e.g. CSR-only route, SSR skipped)', fakeAsync(() => {
176+
// Global intentionally absent; constructor should not throw and should not break later.
177+
delete (window as any).__dspaceRemoveSsrOverlay;
178+
179+
const f = TestBed.createComponent(AppComponent);
180+
expect(() => f.detectChanges()).not.toThrow();
181+
182+
isStable$.next(true);
183+
tick(50);
184+
flush();
185+
186+
expect(window.__dspaceRemoveSsrOverlay).toBeUndefined();
187+
}));
188+
});
130189
});

src/app/app.component.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ export class AppComponent implements OnInit, AfterViewInit {
102102
* is also a 15s hard fallback inside the script itself in case isStable never fires.
103103
*/
104104
private removeSsrOverlayWhenStable(): void {
105-
const w = this._window?.nativeWindow as any;
105+
const w: Window | undefined = this._window?.nativeWindow;
106106
if (!w || typeof w.__dspaceRemoveSsrOverlay !== 'function') {
107107
return;
108108
}

0 commit comments

Comments
 (0)