Duplicate notifications on many creates #89

Closed
opened 2026-04-11 07:43:53 +00:00 by maxtkc · 0 comments
Owner

Summary

Both issues are about the toast/notification system, and they share one root cause: there is no single source of truth for change notifications. Entity changes are announced from two places at once, and the wording is inconsistent and unstyled for long values.

  • #89 — "Duplicate notifications on many creates." Creating an agency/service/route fires two toasts: Created agency / agency (from the patch system's label, shown on the patch change event in index.ts) and Agency "agency" created (an explicit .show() call in src/utils/inline-entity-creator.ts). The issue author's own suggested fix: "disable notifications from anywhere but the patch spot."
  • #135 — "Handle long words in notification." A long unbreakable token (e.g. STOP-97cb7d7f-7f85-41e1-9fd3-308e23da2d92) overflows the toast because the message has no word-break/hyphenation CSS. The issue also asks to revamp the messages to be consistent, include the entity name (not just the id), and synchronize the wording with the history (Changes) panel so the two don't drift.

Approach (chosen with the user):

  1. The patch system is the single source of truth for entity-change notifications. All create/update/delete toasts come from the patch change/undo/redo handlers via one label function; remove the redundant explicit .show() success calls scattered in feature modules. This is the dedup that closes #89 and makes the double-notification bug structurally impossible.
  2. One label function feeds both the toast and the history panel. humanLabel() in src/utils/patch-label.ts becomes the single canonical text generator, consumed by both index.ts (toasts) and history-controller.ts (Changes panel). No drift (#135).
  3. Text convention: Type "name" verbStop "Main St" created, Service "weekday" updated, Route "Blue Line" deleted — using the entity's display name (via src/utils/entity-display.ts), not the raw id.
  4. One notification API: an imported notify singleton (notify.success/error/warning/info), replacing the notifications.showSuccess(...) surface, the per-module injected notificationSystem, and map-controller's private showNotification.
  5. Fix overflow with word-break/hyphenation CSS on the message element (#135).

Net effect: fewer call sites, one wording source, consistent UX — "less code overall" and mistakes much harder to make.


Relevant context

Notification module — src/modules/notification-system.ts

  • class NotificationSystem with show(message, type, options) plus showError/showWarning/ showSuccess/showInfo/showLoading, removeNotification, removeAllNotifications, updateNotification, renderNotification, escapeHtml, initialize.
  • Singleton exported at the bottom: export const notifications = new NotificationSystem();
  • renderNotification() builds the message as <div class="flex-1 min-w-0"><span class="text-sm">${escapeHtml(message)}</span>...</div> inside an element with max-w-sm; the container is max-w-xs. No word-break#135 overflow.

The two notification emitters (the #89 duplicate)

  • Patch path (KEEP — the canonical "patch spot"): src/index.ts (~line 237–259) listens to the patch change event and calls notifications.showInfo(humanLabel(r?.patch), { duration: 3000 }); undo/redo show Undone: ${humanLabel(...)} / Redone: ${humanLabel(...)}. humanLabel comes from src/utils/patch-label.ts. page-content-renderer.ts already relies on this model — see its comment // Record as one atomic batch patch → one 'change' event, one notification (~line 1349).
  • Explicit-success path (REMOVE the duplicates):
    • src/utils/inline-entity-creator.ts: on success calls .show('Agency "${id}" created','success') (~81), Service (~124), Route (~176) — these duplicate the patch toast. It also has validation/error .show(...,'error') calls (~35, 42, 65, 86, 108, 129, 151, 181) — those are NOT patch-driven and must be kept (re-routed through notify.error). Constructor takes notificationSystem: NotificationSystem (~25) and onEntityCreated: () => void (~26); wired from src/modules/browse-navigation.ts:357 (onEntityCreated: () => this.render()).
    • src/modules/schedule-controller.ts:1294: notifications.showSuccess('Trip "${id}" created successfully') — duplicate of patch toast; also notifications.show(...) at ~618 (audit).
    • src/modules/map-controller.ts: a private showNotification() (~1183) used at ~1143, plus console.log for created stop/pathway (~1157, ~1173).
    • src/modules/ui.ts:1191: notifications.showSuccess('New empty GTFS feed created.') — this is a bulk/import action, not a single patch; keep as a deliberate one-off.

Label generator — src/utils/patch-label.ts

  • humanLabel(patch) returns the human string. Insert case (~line 20): return \Created ${source.table} / ${source.id}`;(this is theCreated agency / agencytoast). Update case producesUpdated in / (the #135 overflow example). Delete case similar. **Rewrite this to theType "name" verb` convention** and resolve the display name.

    Entity display — src/utils/entity-display.ts

    • getStopDisplay() (child stops → Name (stop_id); stations/standalone → name), renderOptionLabel, renderCardLabel. Reuse for stop names; add/extend a generic getEntityDisplay(table, row) for route/agency/trip/service/pathway name resolution.

    History / Changes panel — src/modules/history-controller.ts

    • Renders the patch log UI. Must consume the same humanLabel() so panel wording == toast wording (#135 "synchronize ... so we don't have drift").

    Invariants to preserve

    • All user edits go through the patch system — every create/edit/delete already records a patch; we are only changing who announces it, not the recording. The patch is therefore guaranteed to exist and is the correct single emit point.
    • Stop labels go through getStopDisplay() — the rewritten label must use it for stops.
    • Fail loudly / logging — keep console.* breadcrumbs; only the user-facing duplicate toast is removed.

    Phases

    Phase 1 — Collapse to one notify API + fix overflow (#135 mechanical)

    Goal: Establish a single, hard-to-misuse notification entry point and fix the long-word overflow, without yet changing which events fire. Foundation for Phases 2–3.

    • In notification-system.ts, export the singleton as notify with concise methods success/error/warning/info/loading (thin wrappers over the existing show*). Optionally keep notifications + showSuccess/etc. as deprecated aliases during the transition to limit churn, then remove them at the end of the phase.
    • Fix overflow in renderNotification(): add break-words [overflow-wrap:anywhere] hyphens-auto to the message <span> (and verify min-w-0 on its flex parent so it can shrink). Re-check max-w-xs/max-w-sm widths read well with wrapped ids.
    • Replace map-controller.ts's private showNotification() (~1183) and its call (~1143) with notify.*; delete the private method.
    • Migrate every remaining caller to notify.*: index.ts, ui.ts, schedule-controller.ts, service-days-controller.ts, interaction-handler.ts, keyboard-shortcuts.ts, database-fallback-manager.ts, timetable-database.ts, page-content-renderer.ts, inline-entity-creator.ts.
    • pnpm typecheck + pnpm lint clean.

    Gotchas:

    • inline-entity-creator.ts receives notificationSystem via constructor — switching it to import notify directly lets us drop that constructor param and simplify browse-navigation.ts wiring (do the param removal in Phase 3 when its success calls are also removed).
    • Don't change toast durations/animations — only the message CSS and the call surface.

    Phase 1 outcome / discoveries:

    • Renamed the verbose show* methods to concise success/error/warning/info/loading (kept the generic show, initialize, removeNotification, updateNotification, removeAllNotifications, escapeHtml). No transitional notifications/show* aliases were kept — every caller was migrated in one pass, so the old names are gone.
    • Bulk caller migration was a mechanical notifications.showX(notify.x( sed across the 9 listed modules, plus the { notifications } import/dynamic-import rename and the notify, arg in page-content-renderer.ts.
    • inline-entity-creator.ts needed no edits in Phase 1. Its calls go through the injected instance via the generic this.notificationSystem.show(msg, 'success'|'error'), and show is unchanged. page-content-renderer.ts now passes the notify singleton into its constructor. Switching it to import notify directly + dropping the constructor param is deferred to Phase 3 (as the gotcha notes), where its duplicate success .show() calls are also removed.
    • CSS fix applied to the message <span>; the flex parent already had min-w-0. Durations/animations untouched.

    Phase 2 — One canonical label for toast + history (#135 consistency / no drift)

    Goal: Make humanLabel() the single source of change wording, in the Type "name" verb convention, including the entity's display name; wire the history panel to it.

    • Rewrite patch-label.ts::humanLabel(patch):
      • insert → ${TypeLabel} "${name}" created
      • update → ${TypeLabel} "${name}" updated (optionally append a concise changed-field summary, e.g. Stop "Main St" updated (lat, lon) — keep it short and breakable)
      • delete → ${TypeLabel} "${name}" deleted
    • Resolve name via entity-display.ts: getStopDisplay() for stops; name fields for route/agency/trip; id fallback for service (calendar) and pathway. Add a small getEntityDisplay(table, row) dispatcher if one doesn't exist.
    • Verify the patch object carries the row data needed for the name (insert → new row; update → before/after; delete → removed row). If a patch only carries table+id, fall back to id (and note where a lookup would be needed) — do not introduce async DB lookups into label rendering; keep humanLabel synchronous.
    • Update history-controller.ts to render entries via humanLabel() so the Changes panel text matches the toast exactly.
    • Confirm index.ts undo/redo still read Undone: ${humanLabel(...)} / Redone: ... correctly with the new wording.
    • pnpm typecheck + pnpm lint clean.

    Gotchas:

    • TypeLabel must be human ("Stop", "Service", "Route", "Agency", "Trip", "Pathway"), not the raw table name (stops, calendar). Centralize this map in patch-label.ts.
    • Keep the changed-field summary breakable (no giant ids) so it cooperates with the Phase 1 CSS fix.

    Phase 2 outcome / discoveries:

    • history-controller.ts already consumed humanLabel(patch) (line 168) and index.ts already wrapped it for undo/redo (Undone:/Redone:/Undo:/Redo:) — so no edits were needed in either; rewriting humanLabel propagated the new wording to toast, Changes panel, and the undo/redo menu labels for free. Confirms the "single source of wording" design.
    • Added getEntityDisplay(table, row) dispatcher + getTripDisplay() to entity-display.ts. The dispatcher routes agency/stops/routes/calendar/trips to their dedicated helpers and has a generic fallback (<singular>_name / <singular>_id) for everything else (pathways, transfers, levels, etc.). Callers read .primary for the single quoted name.
    • Centralized human TYPE_LABELS map in patch-label.ts (Agency/Stop/Route/Trip/Service/Pathway/…), with typeLabel() falling back to the raw table name for unmapped tables.
    • Update patches only carry changed fields (forward.changes), not the full row — so the name resolves from changes only if the name field itself was edited; otherwise it falls back to source.id (the documented behavior). Concretely, moving a stop still shows the long id, but now as Stop "STOP-…" updated (stop_lat, stop_lon), and the Phase 1 CSS wraps the id instead of overflowing. Insert/delete carry the full record so they get proper names. A future improvement would be to thread the row name into update patches at record time, but that's out of scope (no async lookups in humanLabel).
    • Field summary keeps raw field names (stop_lat, stop_lon) rather than stripping the table prefix — unambiguous, breakable, and avoids a premature helper.

    Phase 3 — Remove duplicate emitters; patch spot is the only source (closes #89)

    Goal: Ensure exactly one notification per entity change by deleting the redundant explicit success toasts; keep only non-patch notifications (validation errors, bulk import, undo/redo).

    • Delete the success .show()/showSuccess() calls that duplicate the patch toast: inline-entity-creator.ts (agency, service, route) and schedule-controller.ts (trip created, was line 1290).
    • Keep validation/error notifications in inline-entity-creator.ts but route them through notify.error/notify.warning; drop the now-unused notificationSystem constructor param and update the page-content-renderer.ts construction site.
    • Audit schedule-controller.ts:618, service-days-controller.ts, and any other entity CRUD paths for additional explicit success toasts that duplicate a recorded patch; remove them.
    • Confirm intentional non-patch toasts remain: ui.ts (new feed / import / export), database-fallback-manager.ts (reset), pending-stop and timetable-database.ts toasts.
    • Manual verification (user runs the app — no Playwright):
      • Create an agency/service/route → exactly one toast, reading e.g. Agency "agency" created.
      • Move a stop → one toast whose long id wraps instead of overflowing.
      • Open the Changes panel → entry text matches the toast verbatim.
    • pnpm typecheck + pnpm lint clean; net line count reduced.

    Gotchas:

    • Some "create" actions may currently record the patch and show a toast in the same method — verify the patch is actually recorded before deleting the toast, so removing it doesn't leave the action silent.
    • page-content-renderer.ts already follows the patch-only model; use it as the reference pattern and don't regress it.

    Phase 3 outcome / discoveries:

    • The InlineEntityCreator is constructed in page-content-renderer.ts (not browse-navigation.ts as the plan guessed); the onEntityCreated callback flows through PageContentRenderer's dependencies (browse-navigation.ts:357 only wires onEntityCreated: () => this.render(), which is unchanged). Dropping the notificationSystem param meant editing the new InlineEntityCreator(...) call in page-content-renderer.ts and removing its now-unused notify import.
    • Removed three duplicate success toasts in inline-entity-creator.ts (agency/service/route) and one in schedule-controller.ts (trip). All four record a patch immediately before, so the patch change event still emits exactly one toast — the action is not left silent.
    • Audit kept these as legitimate non-patch toasts: schedule-controller.ts:618 (trip update failure error), the two "Stop added. Enter a time…" toasts (~1196/1382 — pending stop is UI-only state, no patch recorded), timetable-database.ts ×2 (Added stop to trip — these write via database.replaceRows directly, bypassing the patch system, so they're the only signal for that action), database-fallback-manager.ts (DB reset), and ui.ts (load file / load URL / new feed / export — all bulk/non-patch). service-days-controller.ts only has two notify.error calls.
    • Net −21 lines. typecheck + lint clean.

    Original Issues

    #89 — Duplicate notifications on many creates

    Lets see if we can disable notifications from anywhere but the patch spot so we stop having this issue?

    #135 — Handle long words in notification

    Long words go off the end of the notification. For instance, moving a stop results in this notification:

     Updated stop_lat, stop_lon in stops / STOP-97cb7d7f-7f85-41e1-9fd3-308e23da2d92
    

    The stop id here is too long and overflows off the edge of the notification. We should use hyphen based line wrapping. While we're at it, we should revamp the notification messages so they're all consistent. This should at least include the stop name. We should synchronize this naming with the history so we don't have drift in functionality.

## Summary Both issues are about the toast/notification system, and they share one root cause: **there is no single source of truth for change notifications.** Entity changes are announced from two places at once, and the wording is inconsistent and unstyled for long values. - **#89 — "Duplicate notifications on many creates."** Creating an agency/service/route fires **two** toasts: `Created agency / agency` (from the patch system's label, shown on the patch `change` event in `index.ts`) **and** `Agency "agency" created` (an explicit `.show()` call in `src/utils/inline-entity-creator.ts`). The issue author's own suggested fix: *"disable notifications from anywhere but the patch spot."* - **#135 — "Handle long words in notification."** A long unbreakable token (e.g. `STOP-97cb7d7f-7f85-41e1-9fd3-308e23da2d92`) overflows the toast because the message has no word-break/hyphenation CSS. The issue also asks to **revamp the messages to be consistent**, **include the entity name** (not just the id), and **synchronize the wording with the history (Changes) panel so the two don't drift.** **Approach (chosen with the user):** 1. **The patch system is the single source of truth for entity-change notifications.** All create/update/delete toasts come from the patch `change`/undo/redo handlers via one label function; remove the redundant explicit `.show()` success calls scattered in feature modules. This is the dedup that closes #89 and makes the double-notification bug structurally impossible. 2. **One label function feeds both the toast and the history panel.** `humanLabel()` in `src/utils/patch-label.ts` becomes the single canonical text generator, consumed by both `index.ts` (toasts) and `history-controller.ts` (Changes panel). No drift (#135). 3. **Text convention: `Type "name" verb`** — `Stop "Main St" created`, `Service "weekday" updated`, `Route "Blue Line" deleted` — using the entity's display **name** (via `src/utils/entity-display.ts`), not the raw id. 4. **One notification API: an imported `notify` singleton** (`notify.success/error/warning/info`), replacing the `notifications.showSuccess(...)` surface, the per-module injected `notificationSystem`, and `map-controller`'s private `showNotification`. 5. **Fix overflow** with word-break/hyphenation CSS on the message element (#135). Net effect: fewer call sites, one wording source, consistent UX — "less code overall" and mistakes much harder to make. --- ## Relevant context ### Notification module — `src/modules/notification-system.ts` - `class NotificationSystem` with `show(message, type, options)` plus `showError/showWarning/ showSuccess/showInfo/showLoading`, `removeNotification`, `removeAllNotifications`, `updateNotification`, `renderNotification`, `escapeHtml`, `initialize`. - Singleton exported at the bottom: `export const notifications = new NotificationSystem();` - `renderNotification()` builds the message as `<div class="flex-1 min-w-0"><span class="text-sm">${escapeHtml(message)}</span>...</div>` inside an element with `max-w-sm`; the container is `max-w-xs`. **No word-break** → #135 overflow. ### The two notification emitters (the #89 duplicate) - **Patch path (KEEP — the canonical "patch spot"):** `src/index.ts` (~line 237–259) listens to the patch `change` event and calls `notifications.showInfo(humanLabel(r?.patch), { duration: 3000 })`; undo/redo show `Undone: ${humanLabel(...)}` / `Redone: ${humanLabel(...)}`. `humanLabel` comes from `src/utils/patch-label.ts`. `page-content-renderer.ts` already relies on this model — see its comment `// Record as one atomic batch patch → one 'change' event, one notification` (~line 1349). - **Explicit-success path (REMOVE the duplicates):** - `src/utils/inline-entity-creator.ts`: on success calls `.show('Agency "${id}" created','success')` (~81), Service (~124), Route (~176) — these duplicate the patch toast. It also has validation/error `.show(...,'error')` calls (~35, 42, 65, 86, 108, 129, 151, 181) — those are NOT patch-driven and must be kept (re-routed through `notify.error`). Constructor takes `notificationSystem: NotificationSystem` (~25) and `onEntityCreated: () => void` (~26); wired from `src/modules/browse-navigation.ts:357` (`onEntityCreated: () => this.render()`). - `src/modules/schedule-controller.ts:1294`: `notifications.showSuccess('Trip "${id}" created successfully')` — duplicate of patch toast; also `notifications.show(...)` at ~618 (audit). - `src/modules/map-controller.ts`: a **private** `showNotification()` (~1183) used at ~1143, plus `console.log` for created stop/pathway (~1157, ~1173). - `src/modules/ui.ts:1191`: `notifications.showSuccess('New empty GTFS feed created.')` — this is a bulk/import action, **not** a single patch; keep as a deliberate one-off. ### Label generator — `src/utils/patch-label.ts` - `humanLabel(patch)` returns the human string. Insert case (~line 20): `return \`Created ${source.table} / ${source.id}\`;` (this is the `Created agency / agency` toast). Update case produces `Updated <fields> in <table> / <id>` (the #135 overflow example). Delete case similar. **Rewrite this to the `Type "name" verb` convention** and resolve the display name. ### Entity display — `src/utils/entity-display.ts` - `getStopDisplay()` (child stops → `Name (stop_id)`; stations/standalone → name), `renderOptionLabel`, `renderCardLabel`. Reuse for stop names; add/extend a generic `getEntityDisplay(table, row)` for route/agency/trip/service/pathway name resolution. ### History / Changes panel — `src/modules/history-controller.ts` - Renders the patch log UI. Must consume the same `humanLabel()` so panel wording == toast wording (#135 "synchronize ... so we don't have drift"). ### Invariants to preserve - **All user edits go through the patch system** — every create/edit/delete already records a patch; we are only changing *who announces it*, not the recording. The patch is therefore guaranteed to exist and is the correct single emit point. - **Stop labels go through `getStopDisplay()`** — the rewritten label must use it for stops. - **Fail loudly / logging** — keep `console.*` breadcrumbs; only the user-facing duplicate toast is removed. --- ## Phases ### Phase 1 — Collapse to one `notify` API + fix overflow (#135 mechanical) **Goal:** Establish a single, hard-to-misuse notification entry point and fix the long-word overflow, without yet changing *which* events fire. Foundation for Phases 2–3. - [x] In `notification-system.ts`, export the singleton as `notify` with concise methods `success/error/warning/info/loading` (thin wrappers over the existing `show*`). Optionally keep `notifications` + `showSuccess`/etc. as deprecated aliases during the transition to limit churn, then remove them at the end of the phase. - [x] Fix overflow in `renderNotification()`: add `break-words [overflow-wrap:anywhere] hyphens-auto` to the message `<span>` (and verify `min-w-0` on its flex parent so it can shrink). Re-check `max-w-xs`/`max-w-sm` widths read well with wrapped ids. - [x] Replace `map-controller.ts`'s private `showNotification()` (~1183) and its call (~1143) with `notify.*`; delete the private method. - [x] Migrate every remaining caller to `notify.*`: `index.ts`, `ui.ts`, `schedule-controller.ts`, `service-days-controller.ts`, `interaction-handler.ts`, `keyboard-shortcuts.ts`, `database-fallback-manager.ts`, `timetable-database.ts`, `page-content-renderer.ts`, `inline-entity-creator.ts`. - [x] `pnpm typecheck` + `pnpm lint` clean. **Gotchas:** - `inline-entity-creator.ts` receives `notificationSystem` via constructor — switching it to import `notify` directly lets us drop that constructor param and simplify `browse-navigation.ts` wiring (do the param removal in Phase 3 when its success calls are also removed). - Don't change toast durations/animations — only the message CSS and the call surface. **Phase 1 outcome / discoveries:** - Renamed the verbose `show*` methods to concise `success/error/warning/info/loading` (kept the generic `show`, `initialize`, `removeNotification`, `updateNotification`, `removeAllNotifications`, `escapeHtml`). No transitional `notifications`/`show*` aliases were kept — every caller was migrated in one pass, so the old names are gone. - Bulk caller migration was a mechanical `notifications.showX(` → `notify.x(` sed across the 9 listed modules, plus the `{ notifications }` import/dynamic-import rename and the `notify,` arg in `page-content-renderer.ts`. - **`inline-entity-creator.ts` needed no edits in Phase 1.** Its calls go through the *injected* instance via the generic `this.notificationSystem.show(msg, 'success'|'error')`, and `show` is unchanged. `page-content-renderer.ts` now passes the `notify` singleton into its constructor. Switching it to import `notify` directly + dropping the constructor param is deferred to Phase 3 (as the gotcha notes), where its duplicate success `.show()` calls are also removed. - CSS fix applied to the message `<span>`; the flex parent already had `min-w-0`. Durations/animations untouched. ### Phase 2 — One canonical label for toast + history (#135 consistency / no drift) **Goal:** Make `humanLabel()` the single source of change wording, in the `Type "name" verb` convention, including the entity's display name; wire the history panel to it. - [x] Rewrite `patch-label.ts::humanLabel(patch)`: - insert → `${TypeLabel} "${name}" created` - update → `${TypeLabel} "${name}" updated` (optionally append a concise changed-field summary, e.g. `Stop "Main St" updated (lat, lon)` — keep it short and breakable) - delete → `${TypeLabel} "${name}" deleted` - [x] Resolve `name` via `entity-display.ts`: `getStopDisplay()` for stops; name fields for route/agency/trip; id fallback for service (calendar) and pathway. Add a small `getEntityDisplay(table, row)` dispatcher if one doesn't exist. - [x] Verify the patch object carries the row data needed for the name (insert → new row; update → before/after; delete → removed row). If a patch only carries `table`+`id`, fall back to id (and note where a lookup would be needed) — **do not** introduce async DB lookups into label rendering; keep `humanLabel` synchronous. - [x] Update `history-controller.ts` to render entries via `humanLabel()` so the Changes panel text matches the toast exactly. - [x] Confirm `index.ts` undo/redo still read `Undone: ${humanLabel(...)}` / `Redone: ...` correctly with the new wording. - [x] `pnpm typecheck` + `pnpm lint` clean. **Gotchas:** - `TypeLabel` must be human ("Stop", "Service", "Route", "Agency", "Trip", "Pathway"), not the raw table name (`stops`, `calendar`). Centralize this map in `patch-label.ts`. - Keep the changed-field summary breakable (no giant ids) so it cooperates with the Phase 1 CSS fix. **Phase 2 outcome / discoveries:** - `history-controller.ts` already consumed `humanLabel(patch)` (line 168) and `index.ts` already wrapped it for undo/redo (`Undone:`/`Redone:`/`Undo:`/`Redo:`) — so **no edits were needed in either**; rewriting `humanLabel` propagated the new wording to toast, Changes panel, and the undo/redo menu labels for free. Confirms the "single source of wording" design. - Added `getEntityDisplay(table, row)` dispatcher + `getTripDisplay()` to `entity-display.ts`. The dispatcher routes agency/stops/routes/calendar/trips to their dedicated helpers and has a generic fallback (`<singular>_name` / `<singular>_id`) for everything else (pathways, transfers, levels, etc.). Callers read `.primary` for the single quoted name. - Centralized human `TYPE_LABELS` map in `patch-label.ts` (Agency/Stop/Route/Trip/Service/Pathway/…), with `typeLabel()` falling back to the raw table name for unmapped tables. - **Update patches only carry changed fields** (`forward.changes`), not the full row — so the name resolves from `changes` *only if the name field itself was edited*; otherwise it falls back to `source.id` (the documented behavior). Concretely, moving a stop still shows the long id, but now as `Stop "STOP-…" updated (stop_lat, stop_lon)`, and the Phase 1 CSS wraps the id instead of overflowing. Insert/delete carry the full record so they get proper names. A future improvement would be to thread the row name into update patches at record time, but that's out of scope (no async lookups in `humanLabel`). - Field summary keeps raw field names (`stop_lat, stop_lon`) rather than stripping the table prefix — unambiguous, breakable, and avoids a premature helper. ### Phase 3 — Remove duplicate emitters; patch spot is the only source (closes #89) **Goal:** Ensure exactly one notification per entity change by deleting the redundant explicit success toasts; keep only non-patch notifications (validation errors, bulk import, undo/redo). - [x] Delete the success `.show()/showSuccess()` calls that duplicate the patch toast: `inline-entity-creator.ts` (agency, service, route) and `schedule-controller.ts` (trip created, was line 1290). - [x] Keep validation/error notifications in `inline-entity-creator.ts` but route them through `notify.error`/`notify.warning`; drop the now-unused `notificationSystem` constructor param and update the `page-content-renderer.ts` construction site. - [x] Audit `schedule-controller.ts:618`, `service-days-controller.ts`, and any other entity CRUD paths for additional explicit success toasts that duplicate a recorded patch; remove them. - [x] Confirm intentional non-patch toasts remain: `ui.ts` (new feed / import / export), `database-fallback-manager.ts` (reset), pending-stop and `timetable-database.ts` toasts. - [ ] Manual verification (user runs the app — no Playwright): - Create an agency/service/route → **exactly one** toast, reading e.g. `Agency "agency" created`. - Move a stop → one toast whose long id wraps instead of overflowing. - Open the Changes panel → entry text matches the toast verbatim. - [x] `pnpm typecheck` + `pnpm lint` clean; net line count reduced. **Gotchas:** - Some "create" actions may currently record the patch *and* show a toast in the same method — verify the patch is actually recorded before deleting the toast, so removing it doesn't leave the action silent. - `page-content-renderer.ts` already follows the patch-only model; use it as the reference pattern and don't regress it. **Phase 3 outcome / discoveries:** - The `InlineEntityCreator` is constructed in `page-content-renderer.ts` (not `browse-navigation.ts` as the plan guessed); the `onEntityCreated` callback flows through `PageContentRenderer`'s dependencies (`browse-navigation.ts:357` only wires `onEntityCreated: () => this.render()`, which is unchanged). Dropping the `notificationSystem` param meant editing the `new InlineEntityCreator(...)` call in `page-content-renderer.ts` and removing its now-unused `notify` import. - Removed three duplicate success toasts in `inline-entity-creator.ts` (agency/service/route) and one in `schedule-controller.ts` (trip). All four record a patch immediately before, so the patch `change` event still emits exactly one toast — the action is not left silent. - **Audit kept these as legitimate non-patch toasts:** `schedule-controller.ts:618` (trip update *failure* error), the two `"Stop added. Enter a time…"` toasts (~1196/1382 — pending stop is UI-only state, **no patch recorded**), `timetable-database.ts` ×2 (`Added stop to trip` — these write via `database.replaceRows` directly, bypassing the patch system, so they're the only signal for that action), `database-fallback-manager.ts` (DB reset), and `ui.ts` (load file / load URL / new feed / export — all bulk/non-patch). `service-days-controller.ts` only has two `notify.error` calls. - Net −21 lines. typecheck + lint clean. --- ## Original Issues ### #89 — Duplicate notifications on many creates > Lets see if we can disable notifications from anywhere but the patch spot so we stop having this issue? ### #135 — Handle long words in notification > Long words go off the end of the notification. For instance, moving a stop results in this notification: > ``` > Updated stop_lat, stop_lon in stops / STOP-97cb7d7f-7f85-41e1-9fd3-308e23da2d92 > ``` > The stop id here is too long and overflows off the edge of the notification. We should use > hyphen based line wrapping. While we're at it, we should revamp the notification messages so > they're all consistent. This should at least include the stop name. We should synchronize this > naming with the history so we don't have drift in functionality.
maxtkc self-assigned this 2026-04-11 07:43:53 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
gtfs.zone/coloring-book#89
No description provided.