Fix notifications and loading state #28

New issue

Closed

opened 2026-03-26 22:18:59 +00:00 by maxtkc · 1 comment

maxtkc commented

2026-03-26 22:18:59 +00:00

Owner

Summary

The current setup has two overlapping feedback systems (NotificationSystem and LoadingStateManager) both firing during feed load, producing duplicate messages and visual inconsistency. The notification system also uses emojis and hardcoded light-mode CSS classes that break in dark themes. The plan: enforce a clean ownership split — NotificationSystem handles transient bottom-left toasts exclusively (DaisyUI-styled, no emojis), LoadingStateManager is trimmed to a lean progress-only indicator for feed loads, and a later phase moves ZIP/CSV parsing to a Web Worker so large feeds don't freeze the UI. This branch should be rebased on top of 42-show-map-and-feed-info before starting, since #42 removes mapController.showLoading() calls from ui.ts that this work also touches.

Relevant Context

Files:

src/modules/notification-system.ts — NotificationSystem class; hardcoded light-mode colors (bg-red-50 etc.), emoji icons, container positioned fixed top-4 right-4
src/modules/loading-state-manager.ts — LoadingStateManager; has progress tracking (startLoading, updateProgress, finishLoading) plus abused showError/Warning/Success methods that repurpose the banner for result messages
src/modules/gtfs-parser.ts:530–681 — parseFile() drives loadingStateManager progress (10%→100%) and calls loadingStateManager.showSuccess/showError on completion; also calls loadingStateManager.showWarning for unknown files
src/modules/ui.ts:223–407 — loadGTFSFile() and loadGTFSFromURL() each call notifications.showLoading() at the start, then notifications.showSuccess/showError() at the end — duplicating what gtfs-parser already does via the loading manager
src/index.ts — constructs all modules; calls notifications.initialize() to attach the container to the DOM

Architectural invariant: loadingStateManager is used exclusively by gtfs-parser.ts for feed load progress. No other module calls it. This means we can safely narrow its scope and rename it.

#42 compatibility: Branch 42-show-map-and-feed-info removes mapController.showLoading(), hideMapOverlay(), and export button enable/disable from ui.ts. It also makes the file list always show all sections. No direct conflict with notification changes, but the map overlay is confirmed gone — don't rely on it in the new design.

Phase 1 — Notification System Visual Cleanup

Remove emojis, fix dark-mode styling, move container to bottom-left. No behavior changes.

Replace hardcoded colorMap classes (bg-red-50 border-red-200 text-red-800 etc.) with DaisyUI alert semantic classes (alert alert-error, alert alert-warning, alert alert-success, alert alert-info) in NotificationSystem.renderNotification()
Remove iconMap with emoji characters; replace with short text type labels or DaisyUI SVG alert icons
Remove the loading type emoji; the existing <span class="loading loading-spinner"> DaisyUI spinner is sufficient
Change container class from fixed top-4 right-4 to fixed bottom-4 left-4
Verify z-index: current z-50 is fine; 42-show-map-and-feed-info has map controls at z-40
Use flat DaisyUI alert child structure (icon → span → close button directly inside .alert) to avoid layout fighting DaisyUI's own flex styles; add role="alert" to match docs
Add type-appropriate SVG icons (info/success/warning/error) matching DaisyUI docs; spinner for loading type
Give showLoading a 30s fallback auto-hide so nothing can get permanently stuck

Commits:

fix(notifications): use DaisyUI semantic colors, remove emojis, move to bottom-left
fix(notifications): use DaisyUI alert structure with SVG icons, always expire
fix: use cleaner bg color — removed per-type alert-* variant classes; plain alert with DaisyUI default styling reads much better in practice

Gotcha: DaisyUI v5 alert classes use alert alert-error (two classes). Confirmed v5.5.14 installed, consistent with existing usage throughout the codebase.

Slide animation direction is translateX(-100%) (slide left) to match the bottom-left position.

Phase 2 — Clean Split: Progress Bar vs. Notification Toasts

Enforce non-overlapping ownership. LoadingStateManager = progress only. All result messages go through NotificationSystem.

Delete showError(), showWarning(), showSuccess(), and forceHideLoading() from LoadingStateManager
In gtfs-parser.ts parseFile(): remove loadingStateManager.showSuccess() call (~line 665) and loadingStateManager.showError() call (~line 676) — let callers handle result messages
In gtfs-parser.ts parseFile(): remove loadingStateManager.showWarning() for unknown files (~line 646); instead add unknownFiles to the return type and return it so ui.ts can show a warning toast
In ui.ts loadGTFSFile(): remove the notifications.showLoading() toast (~line 232) — the top progress banner is the sole loading indicator; keep the showSuccess/showError calls at the end
Do the same for ui.ts loadGTFSFromURL() (~line 339)
In ui.ts, after parseFile() resolves, check the returned unknownFiles and call notifications.showWarning() if non-empty
Remove the large-file warning toast in ui.ts (~line 246) — the progress bar status text handles this contextually
Rename LoadingStateManager → FeedProgressIndicator (class, file, export, all imports) to reflect its narrowed purpose
Run grep -r "loadingStateManager" to catch all remaining call sites

Commit: refactor(loading): clean split between progress indicator and notification toasts

Discoveries: database-fallback-manager.ts also called loadingStateManager.showSuccess/showError for the DB reset flow — not mentioned in the plan. Fixed by importing notifications there and routing those two calls through it. loading-state-manager.ts was kept as a re-export shim so the rename is non-breaking at the module level.

parseFromURL was also updated to return { unknownFiles } so the URL load path in ui.ts can show the same warning toast.

Phase 3 — Manual Testing Checkpoint

No code changes. Verify before continuing:

File load (small + large ZIP) shows top progress bar and then a bottom-left success toast
URL load works the same way
Error cases (invalid ZIP, bad URL, network failure) show error toast and dismiss progress bar
Unknown files in a ZIP show a warning toast
Undo/redo and edit notifications are unaffected (still appear bottom-left)
Dark theme looks correct (no washed-out hardcoded colors)

Phase 4 — Web Worker for Feed Parsing

Move ZIP extraction and CSV parsing off the main thread so large feeds don't freeze the UI.

Create src/workers/gtfs-parser.worker.ts; Vite handles worker bundling via new Worker(new URL('../workers/gtfs-parser.worker.ts', import.meta.url), { type: 'module' })
Define typed message protocol:
- Inbound: { type: 'parse', buffer: ArrayBuffer }
- Outbound: { type: 'progress', progress: number, status: string } | { type: 'done', data: WorkerParsedResult } | { type: 'error', message: string }
Move into the worker: JSZip.loadAsync(), Papa.parse(), type coercion (processParsedData); emit progress messages at the same percentage points where updateProgress() was called
Define WorkerParsedResult as the structured data for each file (table name → rows array + raw CSV blob), matching what parseFile() currently assembles into this.gtfsData
In GTFSParser.parseFile(): convert File | Blob to ArrayBuffer via file.arrayBuffer(), transfer to worker with worker.postMessage(buffer, [buffer]) (zero-copy transfer), await done or error
On progress message: call feedProgressIndicator.updateProgress()
On done: main thread calls setupVirtual() for each table and gtfsDatabase.saveTableBlob() — these must stay on main thread
On error: throw so the existing error path in ui.ts catches it and shows the error toast

Commit: feat(parser): move ZIP/CSV parsing to Web Worker

Discoveries: The worker imports gtfs-file-registry.ts directly (pure Zod-based module, no DOM) for ALL_GTFS_FILES, isSupportedFile, and makeHeaderOnlyCSV. GTFSDatabaseRecord is defined locally in the worker as a plain type alias to avoid importing the idb-dependent gtfs-database.ts. Worker is spawned fresh per parse call and terminated after done/error.

Phase 5 — Manual Testing Checkpoint

Large feed loads without freezing the browser tab (map remains interactive, controls respond during parse)
Progress bar still increments correctly
All success/error/warning notification cases still work
Memory profile is reasonable (ArrayBuffer transfer is zero-copy so no doubling of the ZIP in memory)

Original Issue

Lets make a niiice robust global loading and notification state thingy. Lets make it not cover up all the controls when we get a notification 😆

Maybe do a little research into the best editor notifications... Maybe powerline haha

## Summary The current setup has two overlapping feedback systems (`NotificationSystem` and `LoadingStateManager`) both firing during feed load, producing duplicate messages and visual inconsistency. The notification system also uses emojis and hardcoded light-mode CSS classes that break in dark themes. The plan: enforce a clean ownership split — `NotificationSystem` handles transient bottom-left toasts exclusively (DaisyUI-styled, no emojis), `LoadingStateManager` is trimmed to a lean progress-only indicator for feed loads, and a later phase moves ZIP/CSV parsing to a Web Worker so large feeds don't freeze the UI. This branch should be rebased on top of `42-show-map-and-feed-info` before starting, since #42 removes `mapController.showLoading()` calls from `ui.ts` that this work also touches. ## Relevant Context **Files:** - `src/modules/notification-system.ts` — `NotificationSystem` class; hardcoded light-mode colors (`bg-red-50` etc.), emoji icons, container positioned `fixed top-4 right-4` - `src/modules/loading-state-manager.ts` — `LoadingStateManager`; has progress tracking (`startLoading`, `updateProgress`, `finishLoading`) plus abused `showError/Warning/Success` methods that repurpose the banner for result messages - `src/modules/gtfs-parser.ts:530–681` — `parseFile()` drives `loadingStateManager` progress (10%→100%) and calls `loadingStateManager.showSuccess/showError` on completion; also calls `loadingStateManager.showWarning` for unknown files - `src/modules/ui.ts:223–407` — `loadGTFSFile()` and `loadGTFSFromURL()` each call `notifications.showLoading()` at the start, then `notifications.showSuccess/showError()` at the end — duplicating what gtfs-parser already does via the loading manager - `src/index.ts` — constructs all modules; calls `notifications.initialize()` to attach the container to the DOM **Architectural invariant:** `loadingStateManager` is used exclusively by `gtfs-parser.ts` for feed load progress. No other module calls it. This means we can safely narrow its scope and rename it. **#42 compatibility:** Branch `42-show-map-and-feed-info` removes `mapController.showLoading()`, `hideMapOverlay()`, and export button enable/disable from `ui.ts`. It also makes the file list always show all sections. No direct conflict with notification changes, but the map overlay is confirmed gone — don't rely on it in the new design. --- ## Phase 1 — Notification System Visual Cleanup Remove emojis, fix dark-mode styling, move container to bottom-left. No behavior changes. - [x] Replace hardcoded `colorMap` classes (`bg-red-50 border-red-200 text-red-800` etc.) with DaisyUI alert semantic classes (`alert alert-error`, `alert alert-warning`, `alert alert-success`, `alert alert-info`) in `NotificationSystem.renderNotification()` - [x] Remove `iconMap` with emoji characters; replace with short text type labels or DaisyUI SVG alert icons - [x] Remove the loading type emoji; the existing `<span class="loading loading-spinner">` DaisyUI spinner is sufficient - [x] Change container class from `fixed top-4 right-4` to `fixed bottom-4 left-4` - [x] Verify z-index: current `z-50` is fine; `42-show-map-and-feed-info` has map controls at `z-40` - [x] Use flat DaisyUI alert child structure (icon → span → close button directly inside `.alert`) to avoid layout fighting DaisyUI's own flex styles; add `role="alert"` to match docs - [x] Add type-appropriate SVG icons (info/success/warning/error) matching DaisyUI docs; spinner for loading type - [x] Give `showLoading` a 30s fallback auto-hide so nothing can get permanently stuck Commits: - `fix(notifications): use DaisyUI semantic colors, remove emojis, move to bottom-left` - `fix(notifications): use DaisyUI alert structure with SVG icons, always expire` - `fix: use cleaner bg color` — removed per-type `alert-*` variant classes; plain `alert` with DaisyUI default styling reads much better in practice Gotcha: DaisyUI v5 alert classes use `alert alert-error` (two classes). Confirmed v5.5.14 installed, consistent with existing usage throughout the codebase. Slide animation direction is `translateX(-100%)` (slide left) to match the bottom-left position. --- ## Phase 2 — Clean Split: Progress Bar vs. Notification Toasts Enforce non-overlapping ownership. `LoadingStateManager` = progress only. All result messages go through `NotificationSystem`. - [x] Delete `showError()`, `showWarning()`, `showSuccess()`, and `forceHideLoading()` from `LoadingStateManager` - [x] In `gtfs-parser.ts` `parseFile()`: remove `loadingStateManager.showSuccess()` call (~line 665) and `loadingStateManager.showError()` call (~line 676) — let callers handle result messages - [x] In `gtfs-parser.ts` `parseFile()`: remove `loadingStateManager.showWarning()` for unknown files (~line 646); instead add `unknownFiles` to the return type and return it so `ui.ts` can show a warning toast - [x] In `ui.ts` `loadGTFSFile()`: remove the `notifications.showLoading()` toast (~line 232) — the top progress banner is the sole loading indicator; keep the `showSuccess/showError` calls at the end - [x] Do the same for `ui.ts` `loadGTFSFromURL()` (~line 339) - [x] In `ui.ts`, after `parseFile()` resolves, check the returned `unknownFiles` and call `notifications.showWarning()` if non-empty - [x] Remove the large-file warning toast in `ui.ts` (~line 246) — the progress bar status text handles this contextually - [x] Rename `LoadingStateManager` → `FeedProgressIndicator` (class, file, export, all imports) to reflect its narrowed purpose - [x] Run `grep -r "loadingStateManager"` to catch all remaining call sites Commit: `refactor(loading): clean split between progress indicator and notification toasts` Discoveries: `database-fallback-manager.ts` also called `loadingStateManager.showSuccess/showError` for the DB reset flow — not mentioned in the plan. Fixed by importing `notifications` there and routing those two calls through it. `loading-state-manager.ts` was kept as a re-export shim so the rename is non-breaking at the module level. `parseFromURL` was also updated to return `{ unknownFiles }` so the URL load path in `ui.ts` can show the same warning toast. --- ## Phase 3 — Manual Testing Checkpoint No code changes. Verify before continuing: - File load (small + large ZIP) shows top progress bar and then a bottom-left success toast - URL load works the same way - Error cases (invalid ZIP, bad URL, network failure) show error toast and dismiss progress bar - Unknown files in a ZIP show a warning toast - Undo/redo and edit notifications are unaffected (still appear bottom-left) - Dark theme looks correct (no washed-out hardcoded colors) --- ## Phase 4 — Web Worker for Feed Parsing Move ZIP extraction and CSV parsing off the main thread so large feeds don't freeze the UI. - [x] Create `src/workers/gtfs-parser.worker.ts`; Vite handles worker bundling via `new Worker(new URL('../workers/gtfs-parser.worker.ts', import.meta.url), { type: 'module' })` - [x] Define typed message protocol: - Inbound: `{ type: 'parse', buffer: ArrayBuffer }` - Outbound: `{ type: 'progress', progress: number, status: string }` | `{ type: 'done', data: WorkerParsedResult }` | `{ type: 'error', message: string }` - [x] Move into the worker: `JSZip.loadAsync()`, `Papa.parse()`, type coercion (`processParsedData`); emit `progress` messages at the same percentage points where `updateProgress()` was called - [x] Define `WorkerParsedResult` as the structured data for each file (table name → rows array + raw CSV blob), matching what `parseFile()` currently assembles into `this.gtfsData` - [x] In `GTFSParser.parseFile()`: convert `File | Blob` to `ArrayBuffer` via `file.arrayBuffer()`, transfer to worker with `worker.postMessage(buffer, [buffer])` (zero-copy transfer), await `done` or `error` - [x] On `progress` message: call `feedProgressIndicator.updateProgress()` - [x] On `done`: main thread calls `setupVirtual()` for each table and `gtfsDatabase.saveTableBlob()` — these must stay on main thread - [x] On `error`: throw so the existing error path in `ui.ts` catches it and shows the error toast Commit: `feat(parser): move ZIP/CSV parsing to Web Worker` Discoveries: The worker imports `gtfs-file-registry.ts` directly (pure Zod-based module, no DOM) for `ALL_GTFS_FILES`, `isSupportedFile`, and `makeHeaderOnlyCSV`. `GTFSDatabaseRecord` is defined locally in the worker as a plain type alias to avoid importing the idb-dependent `gtfs-database.ts`. Worker is spawned fresh per parse call and terminated after `done`/`error`. --- ## Phase 5 — Manual Testing Checkpoint - Large feed loads without freezing the browser tab (map remains interactive, controls respond during parse) - Progress bar still increments correctly - All success/error/warning notification cases still work - Memory profile is reasonable (ArrayBuffer transfer is zero-copy so no doubling of the ZIP in memory) --- ## Original Issue Lets make a niiice robust global loading and notification state thingy. Lets make it not cover up all the controls when we get a notification 😆 Maybe do a little research into the best editor notifications... Maybe powerline haha