Skip to content

Commit feff1bd

Browse files
bloveclaude
andauthored
fix(ag-ui): gate <google-map> on Maps API load (blank-page-on-reload) (#736)
* fix(ag-ui): gate <google-map> on Maps API load (blank-page-on-reload crash) Reloading the demo with App mode persisted on rendered a blank page (only the toolbar). Root cause (confirmed against @angular/google-maps google-maps.mjs:136): the GoogleMap constructor throws "Namespace google not found" when window.google is absent — and the Maps script loads async, so on a fresh load it loses the race, the map throws during the shell's initial render, and the whole shell render aborts (dev-mode only, ngDevMode-gated, which is why #732 CI/smoke missed it; the P2 smoke only toggled App mode at runtime, never reloaded with it persisted on). Fix: a GoogleMapsLoader service exposing a `loaded` signal (owns the script injection, flips on script.onload); gate <google-map> with @if (loader.loaded()) so it never constructs before the API is present. app.config loads eagerly through the same service so the GeocodingService keeps working. markerOptions uses the numeric SymbolPath literal (0) as defense-in-depth against eager google.* reads. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(ag-ui): hide sidebar launcher hint when App mode is on In App mode the Google Map is the full-bleed background and the chat sidebar floats over it, so the placeholder "Use the launcher…" hint (projected into chat-sidebar's default content slot) floated unreadably over the map. Gate it on appMode !== 'on' so it only shows in plain sidebar mode where the area behind the chat is genuinely empty. Verified via DOM probe: /sidebar appMode off → hint present (1); /sidebar?appmode=on with the map mounted → hint absent (0). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(ag-ui): itinerary panel is App-mode-only (drop the sidebar panel) The frontend-owned itinerary used to render twice: as a fixed 300px panel beside the chat in plain embed/sidebar mode, and as the floating overlay in App mode. Remove the former — the itinerary now appears only in App mode, as the map overlay. Plain mode is just the chat; the App-mode toggle reveals the map + itinerary. Drops <app-itinerary-panel class="ag-ui-shell__itinerary"> from the non-App-mode body and its now-dead two-column / responsive CSS. The ItineraryPanelComponent import stays (still used by the overlay). The itinerary-client-tools e2e suite drove the panel in default mode, so point it at ?appmode=on — App mode is URL-driven regardless of the key-gated toolbar toggle, and the overlay panel renders independent of the map, so the same panel DOM is present even without GOOGLE_MAPS_API_KEY in CI. Verified via DOM probe: clean default → 0 itinerary panels (just chat); ?appmode=on → overlay panel with the Louvre/Eiffel/Orsay seed + map mounted. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(ag-ui): restore the App-mode cockpit route on reload App mode persists via localStorage, but on a fresh load with it persisted on the app rendered the itinerary overlay yet showed EmbedMode's full-bleed chat over the map instead of the sidebar cockpit. Root cause: the knob persist effect navigated with a route-relative `navigate([])`, which at bootstrap resolves against the not-yet-settled initial navigation and lands on the default /embed — so the router-outlet mounted EmbedMode (z-index above the map) while appMode-gated chrome (overlay, map-canvas) still rendered. Navigate to the absolute `/sidebar` whenever App mode is on (the layout the cockpit requires), keeping the route-relative `[]` only when it's off. Also drop the now-redundant navigate in onAppModeChange — the effect is the single source of routing truth for both the toggle and reload paths. Verified: toggle on → /sidebar; reload from / → stays /sidebar with the sidebar chat + map + overlay (was bouncing to /embed); toggle off keeps the current route with plain chat; clean default still lands on /embed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 01ed35e commit feff1bd

8 files changed

Lines changed: 142 additions & 80 deletions

examples/ag-ui/angular/e2e/itinerary-client-tools.spec.ts

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,13 @@ import { attachBrowserHygiene, messageInput, openDemo, sendButton } from './test
88
// the ToolMessage re-runs the graph, and the continuation streams back. Each
99
// test starts from the store seed (Day 1: Louvre + Eiffel Tower, Day 2: Musée
1010
// d'Orsay) by clearing the persisted key before the page hydrates.
11+
//
12+
// The itinerary panel only renders in App mode (it's the floating map overlay;
13+
// plain embed/sidebar no longer shows it), so these tests open the demo with
14+
// `?appmode=on`. App mode is driven from the URL regardless of the key-gated
15+
// toolbar toggle, and the overlay panel renders independent of the map — so the
16+
// same panel DOM (region "Trip itinerary", section.itin__day, .itin__handle …)
17+
// is present even without a GOOGLE_MAPS_API_KEY in CI.
1118
const STORAGE_KEY = 'ag-ui-demo:itinerary';
1219

1320
test.beforeEach(async ({ page }) => {
@@ -19,7 +26,7 @@ test.beforeEach(async ({ page }) => {
1926
});
2027

2128
test('panel renders the seeded itinerary', async ({ page }) => {
22-
await openDemo(page);
29+
await openDemo(page, '/?appmode=on');
2330

2431
const panel = page.getByRole('region', { name: 'Trip itinerary' });
2532
await expect(panel).toBeVisible();
@@ -31,7 +38,7 @@ test('panel renders the seeded itinerary', async ({ page }) => {
3138
test('read round-trip: get_itinerary executes in the browser and the run continues', async ({
3239
page,
3340
}) => {
34-
await openDemo(page);
41+
await openDemo(page, '/?appmode=on');
3542
const hygiene = attachBrowserHygiene(page);
3643

3744
await messageInput(page).fill("What's on my itinerary?");
@@ -46,7 +53,7 @@ test('read round-trip: get_itinerary executes in the browser and the run continu
4653
});
4754

4855
test('ask chain: clear_day confirm mutates the panel and resumes the run', async ({ page }) => {
49-
await openDemo(page);
56+
await openDemo(page, '/?appmode=on');
5057

5158
await messageInput(page).fill('Clear my day 2 plans');
5259
await sendButton(page).click();
@@ -79,7 +86,7 @@ test('ask chain: clear_day confirm mutates the panel and resumes the run', async
7986
test('move_stop action: panel mutates — Eiffel Tower moves from day 1 to day 2', async ({
8087
page,
8188
}) => {
82-
await openDemo(page);
89+
await openDemo(page, '/?appmode=on');
8390

8491
// Seed has Eiffel Tower under Day 1 — verify it's there before sending.
8592
const panel = page.getByRole('region', { name: 'Trip itinerary' });
@@ -112,7 +119,7 @@ test('move_stop action: panel mutates — Eiffel Tower moves from day 1 to day 2
112119
test('day_card view: component renders with model-filled props and run auto-continues', async ({
113120
page,
114121
}) => {
115-
await openDemo(page);
122+
await openDemo(page, '/?appmode=on');
116123

117124
await messageInput(page).fill('Show me a recap card for day 1');
118125
await sendButton(page).click();
@@ -130,7 +137,7 @@ test('day_card view: component renders with model-filled props and run auto-cont
130137
});
131138

132139
test('user can drag-reorder stops within a day', async ({ page }) => {
133-
await openDemo(page);
140+
await openDemo(page, '/?appmode=on');
134141

135142
const panel = page.getByRole('region', { name: 'Trip itinerary' });
136143

@@ -170,7 +177,7 @@ test('user can drag-reorder stops within a day', async ({ page }) => {
170177
});
171178

172179
test('reorder_stop: agent puts Louvre last on day 1', async ({ page }) => {
173-
await openDemo(page);
180+
await openDemo(page, '/?appmode=on');
174181
const hygiene = attachBrowserHygiene(page);
175182

176183
await messageInput(page).fill('Put Louvre last on day 1.');

examples/ag-ui/angular/src/app/app.config.ts

Lines changed: 8 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// SPDX-License-Identifier: MIT
22
import {
33
ApplicationConfig,
4+
inject,
45
provideBrowserGlobalErrorListeners,
56
provideEnvironmentInitializer,
67
provideZonelessChangeDetection,
@@ -13,6 +14,7 @@ import { environment } from '../environments/environment';
1314
import { routes } from './app.routes';
1415
import { ItineraryStore } from './itinerary-store';
1516
import { ITINERARY_AGENT } from './client-tools';
17+
import { GoogleMapsLoader } from './google-maps-loader';
1618

1719
export const appConfig: ApplicationConfig = {
1820
providers: [
@@ -35,21 +37,12 @@ export const appConfig: ApplicationConfig = {
3537
// Typed agent provider: flows ItineraryState through DI so every
3638
// injectAgent(ITINERARY_AGENT) call returns AgUiAgent<ItineraryState>.
3739
provideAgent(ITINERARY_AGENT, { url: environment.agentUrl }),
38-
// Load the Google Maps JS API once at bootstrap so the map canvas and the
39-
// GeocodingService both run against the same loaded script. Skips cleanly
40-
// when no key is configured (the googleMapsApiKey is '' in that case).
41-
provideEnvironmentInitializer(() => {
42-
const key = (environment.googleMapsApiKey as string) ?? '';
43-
if (!key) return;
44-
const g = globalThis as { google?: unknown };
45-
if (g.google) return;
46-
if (document.querySelector('script[data-google-maps]')) return;
47-
const script = document.createElement('script');
48-
script.src = `https://maps.googleapis.com/maps/api/js?key=${encodeURIComponent(key)}&libraries=geocoding`;
49-
script.async = true;
50-
script.setAttribute('data-google-maps', '');
51-
document.head.appendChild(script);
52-
}),
40+
// Eagerly load the Google Maps JS API at bootstrap via the loader service,
41+
// which owns the single `loaded` signal the map canvas gates its
42+
// <google-map> on. Loading early lets the GeocodingService work before App
43+
// mode is opened; the map still waits for `loaded()` so its component never
44+
// constructs before the API is present. Skips cleanly with no key.
45+
provideEnvironmentInitializer(() => inject(GoogleMapsLoader).ensureLoaded()),
5346
provideChat({ license: environment.license }),
5447
// The frontend-owned itinerary is a single shared instance: the panel,
5548
// the App component, and the client-tool ask component all inject it, so
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
// SPDX-License-Identifier: MIT
2+
import { Injectable, signal } from '@angular/core';
3+
import { environment } from '../environments/environment';
4+
5+
/**
6+
* Loads the Google Maps JS API on demand and exposes a `loaded` signal.
7+
*
8+
* Why this exists: `<google-map>` (from `@angular/google-maps`) THROWS in its
9+
* constructor when `window.google` is absent ("Namespace google not found…",
10+
* dev-mode only). The Maps script loads asynchronously, so rendering the map
11+
* before it resolves aborts the host component's change-detection pass — which
12+
* (on a fresh load with App mode persisted on) blanks the whole shell. The fix
13+
* is the documented `@angular/google-maps` contract: only render `<google-map>`
14+
* once the API is present. Consumers gate their template on `loaded()` and call
15+
* `ensureLoaded()` once.
16+
*/
17+
@Injectable({ providedIn: 'root' })
18+
export class GoogleMapsLoader {
19+
/** Becomes true once `window.google.maps` is available. */
20+
readonly loaded = signal(false);
21+
private started = false;
22+
23+
/** Idempotent: injects the Maps script once (if a key is present) and flips
24+
* `loaded` when ready. Safe to call from multiple components. */
25+
ensureLoaded(): void {
26+
if (this.loaded() || this.started) return;
27+
const w = globalThis as { google?: { maps?: unknown }; document?: Document };
28+
if (w.google?.maps) {
29+
this.loaded.set(true);
30+
return;
31+
}
32+
this.started = true;
33+
34+
const key = (environment.googleMapsApiKey as string) ?? '';
35+
if (!key) return; // No key → map stays gated off (the toolbar toggle is also disabled).
36+
37+
const doc = w.document;
38+
if (!doc) return;
39+
40+
const existing = doc.querySelector('script[data-google-maps]');
41+
if (existing) {
42+
// A load is already in flight (e.g. a prior instance). Poll for readiness.
43+
const poll = setInterval(() => {
44+
if ((globalThis as { google?: { maps?: unknown } }).google?.maps) {
45+
clearInterval(poll);
46+
this.loaded.set(true);
47+
}
48+
}, 100);
49+
return;
50+
}
51+
52+
const script = doc.createElement('script');
53+
script.src = `https://maps.googleapis.com/maps/api/js?key=${encodeURIComponent(key)}&libraries=geocoding`;
54+
script.async = true;
55+
script.setAttribute('data-google-maps', '');
56+
script.addEventListener('load', () => this.loaded.set(true));
57+
doc.head.appendChild(script);
58+
}
59+
}

examples/ag-ui/angular/src/app/map-canvas.component.ts

Lines changed: 41 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
} from '@angular/core';
1313
import { GoogleMap, MapInfoWindow, MapMarker, MapPolyline } from '@angular/google-maps';
1414
import { ItineraryStop, ItineraryStore } from './itinerary-store';
15+
import { GoogleMapsLoader } from './google-maps-loader';
1516

1617
const DARK_STYLE: google.maps.MapTypeStyle[] = [
1718
{ elementType: 'geometry', stylers: [{ color: '#1d2c4d' }] },
@@ -37,36 +38,41 @@ const PARIS_CENTER: google.maps.LatLngLiteral = { lat: 48.8566, lng: 2.3522 };
3738
imports: [GoogleMap, MapInfoWindow, MapMarker, MapPolyline],
3839
changeDetection: ChangeDetectionStrategy.OnPush,
3940
template: `
40-
<google-map
41-
width="100%"
42-
height="100%"
43-
[center]="center()"
44-
[zoom]="zoom()"
45-
[options]="mapOptions"
46-
>
47-
@for (s of stopsWithCoords(); track s.id) {
48-
<map-marker
49-
#marker
50-
[position]="{ lat: s.lat!, lng: s.lng! }"
51-
[options]="markerOptions(s)"
52-
(mapClick)="onMarkerClick(s)"
53-
/>
54-
}
55-
@for (line of polylines(); track line.day) {
56-
<map-polyline [path]="line.path" [options]="polylineOptions(line.day)" />
57-
}
58-
<map-info-window>
59-
@if (focused(); as f) {
60-
<div class="info">
61-
<strong>{{ f.place }}</strong>
62-
@if (f.note) {
63-
<div class="info__note">{{ f.note }}</div>
64-
}
65-
<button type="button" class="info__remove" (click)="removeFocused()">Remove</button>
66-
</div>
41+
<!-- Render <google-map> ONLY after the Maps API has loaded. Its constructor
42+
throws "Namespace google not found" when window.google is absent, which
43+
would abort the host shell's render on a fresh load with App mode on. -->
44+
@if (loader.loaded()) {
45+
<google-map
46+
width="100%"
47+
height="100%"
48+
[center]="center()"
49+
[zoom]="zoom()"
50+
[options]="mapOptions"
51+
>
52+
@for (s of stopsWithCoords(); track s.id) {
53+
<map-marker
54+
#marker
55+
[position]="{ lat: s.lat!, lng: s.lng! }"
56+
[options]="markerOptions(s)"
57+
(mapClick)="onMarkerClick(s)"
58+
/>
6759
}
68-
</map-info-window>
69-
</google-map>
60+
@for (line of polylines(); track line.day) {
61+
<map-polyline [path]="line.path" [options]="polylineOptions(line.day)" />
62+
}
63+
<map-info-window>
64+
@if (focused(); as f) {
65+
<div class="info">
66+
<strong>{{ f.place }}</strong>
67+
@if (f.note) {
68+
<div class="info__note">{{ f.note }}</div>
69+
}
70+
<button type="button" class="info__remove" (click)="removeFocused()">Remove</button>
71+
</div>
72+
}
73+
</map-info-window>
74+
</google-map>
75+
}
7076
`,
7177
styles: [
7278
`
@@ -83,6 +89,7 @@ const PARIS_CENTER: google.maps.LatLngLiteral = { lat: 48.8566, lng: 2.3522 };
8389
})
8490
export class MapCanvasComponent {
8591
protected readonly store = inject(ItineraryStore);
92+
protected readonly loader = inject(GoogleMapsLoader);
8693
protected readonly center = signal<google.maps.LatLngLiteral>(PARIS_CENTER);
8794
protected readonly zoom = signal<number>(12);
8895
protected readonly mapOptions: google.maps.MapOptions = {
@@ -104,6 +111,8 @@ export class MapCanvasComponent {
104111
);
105112

106113
constructor() {
114+
this.loader.ensureLoaded();
115+
107116
effect(() => {
108117
const f = this.focused();
109118
if (!f || f.lat == null || f.lng == null) return;
@@ -140,7 +149,9 @@ export class MapCanvasComponent {
140149
protected markerOptions(s: ItineraryStop): google.maps.MarkerOptions {
141150
return {
142151
icon: {
143-
path: google.maps.SymbolPath.CIRCLE,
152+
// Numeric literal for google.maps.SymbolPath.CIRCLE (=0) — avoids an
153+
// eager google.* value read (defense-in-depth alongside the loader gate).
154+
path: 0,
144155
fillColor: DAY_COLORS[(s.day - 1) % DAY_COLORS.length],
145156
fillOpacity: 1,
146157
strokeColor: '#fff',

examples/ag-ui/angular/src/app/modes/sidebar-mode.component.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,13 @@ import { WelcomeSuggestionsComponent } from './welcome-suggestions.component';
2222
(selectedModelChange)="shell.onModelChange($event)"
2323
>
2424
<div class="sidebar-mode__background">
25-
<p class="sidebar-mode__hint">
26-
Use the launcher (right edge) to dismiss or re-open the chat panel.
27-
</p>
25+
<!-- In App mode the map is the background; this placeholder hint would
26+
float unreadably over it, so only show it in plain sidebar mode. -->
27+
@if (shell.appMode() !== 'on') {
28+
<p class="sidebar-mode__hint">
29+
Use the launcher (right edge) to dismiss or re-open the chat panel.
30+
</p>
31+
}
2832
</div>
2933
<welcome-suggestions chatWelcomeSuggestions (selected)="send($event)" />
3034
</chat-sidebar>

examples/ag-ui/angular/src/app/shell/ag-ui-shell.component.css

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -195,29 +195,11 @@
195195
}
196196
.ag-ui-shell__theme-toggle--toolbar { margin-left: auto; }
197197

198-
/* Two-column body: frontend-owned itinerary panel beside the routed chat
199-
* (ported from the pre-shell layout that #655 introduced). */
198+
/* Non-App-mode body: the routed chat fills the area below the toolbar.
199+
* The itinerary lives only in App mode now, as the floating map overlay. */
200200
.ag-ui-shell__body { flex: 1 1 auto; min-height: 0; display: flex; }
201-
.ag-ui-shell__itinerary {
202-
flex: 0 0 300px;
203-
min-height: 0;
204-
overflow-y: auto;
205-
border-right: 1px solid var(--ngaf-chat-separator);
206-
}
207201
.ag-ui-shell__body > .ag-ui-shell__main { flex: 1 1 auto; min-width: 0; min-height: 0; display: flex; flex-direction: column; }
208202

209-
/* Stack to a single column on narrow viewports; the panel goes full-width
210-
* above the chat. */
211-
@media (max-width: 900px) {
212-
.ag-ui-shell__body { flex-direction: column; }
213-
.ag-ui-shell__itinerary {
214-
flex: 0 0 auto;
215-
max-height: 40dvh;
216-
border-right: none;
217-
border-bottom: 1px solid var(--ngaf-chat-separator);
218-
}
219-
}
220-
221203
.ag-ui-shell__app-toggle {
222204
display: inline-flex;
223205
align-items: center;

examples/ag-ui/angular/src/app/shell/ag-ui-shell.component.html

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,6 @@
7070
</div>
7171
} @else {
7272
<div class="ag-ui-shell__body">
73-
<app-itinerary-panel class="ag-ui-shell__itinerary" />
7473
<div class="ag-ui-shell__main">
7574
<router-outlet />
7675
@if (agent.interrupt && agent.interrupt()) {

examples/ag-ui/angular/src/app/shell/ag-ui-shell.component.ts

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,15 @@ export class AgUiShell {
169169
scheme: this.colorScheme() === DEFAULTS.scheme ? null : this.colorScheme(),
170170
appmode: this.appMode() === 'off' ? null : this.appMode(),
171171
};
172-
void this.router.navigate([], { queryParams: q, queryParamsHandling: 'merge', replaceUrl: true });
172+
// App mode's layout requires the sidebar route (chat as the right rail
173+
// beside the map). On a fresh load with App mode persisted on, a
174+
// route-relative navigate([]) resolves against the not-yet-settled
175+
// initial navigation and lands on the default /embed — mounting
176+
// EmbedMode over the map. Navigate to the absolute /sidebar whenever
177+
// App mode is on so a reload restores the cockpit; otherwise keep the
178+
// current route ([]) and just sync query params.
179+
const commands = this.appMode() === 'on' ? ['/', 'sidebar'] : [];
180+
void this.router.navigate(commands, { queryParams: q, queryParamsHandling: 'merge', replaceUrl: true });
173181
});
174182
}
175183

@@ -180,9 +188,8 @@ export class AgUiShell {
180188
onAppModeChange(v: 'on' | 'off'): void {
181189
this.appMode.set(v);
182190
this.persistence.write('appMode', v);
183-
if (v === 'on' && this.mode() !== 'sidebar') {
184-
void this.router.navigate(['/', 'sidebar'], { queryParamsHandling: 'preserve' });
185-
}
191+
// Routing is handled by the persist effect, which forces /sidebar whenever
192+
// App mode is on (toggle and reload alike) and keeps the current route off.
186193
}
187194
onModelChange(v: string): void { this.model.set(v); this.persistence.write('model', v); }
188195
protected onEffortChange(v: string): void { this.effort.set(v); this.persistence.write('effort', v); }

0 commit comments

Comments
 (0)