OpenVelo Agent
66 lines
6.9 KiB
Markdown
66 lines
6.9 KiB
Markdown
# Implementation Plan: Job 914 — Escape must close the New post overlay without saving
|
||
|
||
## 1. Architectural Reconnaissance
|
||
|
||
- **Codebase style & conventions:**
|
||
- Angular 17+ standalone components, `ChangeDetectionStrategy.OnPush`, signal inputs (`input()`), signal outputs (`output()`), the new `@if` control flow, and `effect()` for reactive side effects.
|
||
- Backend is NestJS with TypeORM; out of scope for this Job (no DB/API changes).
|
||
- The frontend admin blog feature lives under `frontend/src/app/features/admin/blog/`. The pure helpers live in `blog-form.pure.ts`; the modal is `blog-form-modal.component.ts`; the smart page is `blog.component.ts`.
|
||
- **Escape-to-close is the established convention for every modal in this codebase.** Every other modal uses `@HostListener('document:keydown.escape')` on the component class and emits its `cancel` output:
|
||
- `frontend/src/app/features/setup/setup-create-admin.component.ts:103`
|
||
- `frontend/src/app/features/admin/player-delete-modal.component.ts:61`
|
||
- `frontend/src/app/features/shell/change-password/change-password-modal.component.ts:156`
|
||
- `frontend/src/app/features/challenges/challenge-modal.component.ts:174`
|
||
- `frontend/src/app/features/admin/challenges/challenge-import-modal.component.ts:105`
|
||
- `frontend/src/app/features/admin/challenges/challenge-form-modal.component.ts:404`
|
||
- `frontend/src/app/features/admin/challenges/challenge-delete-modal.component.ts:56`
|
||
- `frontend/src/app/features/landing/landing.component.ts:102`
|
||
- The very similar `change-password-modal.component.ts:156–159` guards the handler with `if (this.open() && !this.submitting()) this.onCancel();`. We will mirror that exact contract for the blog form modal (swapping `submitting()` for `saving()`).
|
||
- **The blog form modal is the only modal missing this listener.** This is the bug described in the Job.
|
||
- **Data Layer:** No database changes — this is purely a frontend modal interaction fix.
|
||
- **Test Framework & Structure:**
|
||
- Jest 29 with `ts-jest`, frontend project runs in `jsdom` (`tests/jest.config.js`).
|
||
- Single test entry: `npm test` runs both backend and frontend projects; `npm run test:frontend` runs only frontend.
|
||
- Tests live under `tests/frontend/` (front-end) and `tests/backend/` (back-end). Source code lives under `frontend/src/app/` and `backend/src/`. Tests must NEVER live inside source folders.
|
||
- Existing frontend tests are predominantly pure-helper tests (no TestBed). The `challenge-card-accessibility.spec.ts` pattern uses a `readFileSync` source-text assertion against the component template, which is the safest pattern for testing that a `@HostListener` is wired into the component without standing up the full Angular runtime.
|
||
- **Required Tools & Dependencies:** None. No new packages, globals, or `setup.sh` changes are needed — `@angular/core` already exports `HostListener`, and Jest/jsdom is already configured.
|
||
|
||
## 2. Impacted Files
|
||
|
||
- **To Modify:**
|
||
- `frontend/src/app/features/admin/blog/blog-form-modal.component.ts` — add the `@HostListener('document:keydown.escape')` handler that calls the existing `onCancel()` when the modal is open and not currently saving.
|
||
- **To Create:**
|
||
- `tests/frontend/blog-form-modal-escape.spec.ts` — minimal, focused regression test that verifies the Escape listener is wired and routed correctly. Mirrors the existing source-text snapshot pattern from `tests/frontend/challenge-card-accessibility.spec.ts` so it stays free of TestBed/Angular runtime setup and runs instantly under `npm test`.
|
||
|
||
## 3. Proposed Changes
|
||
|
||
1. **Add an Escape handler to `BlogFormModalComponent`** in `frontend/src/app/features/admin/blog/blog-form-modal.component.ts`:
|
||
- Import `HostListener` from `@angular/core` alongside the existing `ChangeDetectionStrategy`, `ChangeDetectorRef`, `Component`, `DestroyRef`, `effect`, `inject`, `input`, `output`, `signal` imports.
|
||
- Add the following method on the class (mirroring `change-password-modal.component.ts:156–159`):
|
||
```ts
|
||
@HostListener('document:keydown.escape')
|
||
onEscape(): void {
|
||
if (this.open() && !this.saving()) this.onCancel();
|
||
}
|
||
```
|
||
- This reuses the existing `onCancel()` (`blog-form-modal.component.ts:210`) which already emits `cancel`. The parent `AdminBlogComponent.closeModal()` (`blog.component.ts:188`) clears `modalMode`, `editing`, `formError`, `saving`, etc., so no post is created or modified.
|
||
- Rationale for the `open() && !saving()` guard: the modal is conditionally rendered via `@if (open())` in the template, so the listener will fire when the modal is mounted regardless of `open()`'s value; we explicitly check `open()` for safety and parity with `change-password-modal.component.ts`, and we skip the handler while `saving()` is true so an in-flight create/publish cannot be cancelled mid-request (the existing `Publish`/`Save draft` buttons remain `[disabled]` while saving and the parent also blocks the toolbar via `actionInFlight`).
|
||
- No template, styles, or HTML attribute changes are needed. The `Cancel` button, backdrop click, and the new Escape keypress all flow through the same `onCancel()` → `cancel` output.
|
||
|
||
2. **No backend / API changes.** The bug is purely a missing keyboard handler on the client. The REST endpoints (`POST /api/v1/admin/blog/posts`, `PATCH /api/v1/admin/blog/posts/:id`) are already correct; the failing test scenario was a *frontend* interaction that never called the API.
|
||
|
||
3. **No `setup.sh` / dependency changes.** `HostListener` is part of `@angular/core`, which is already a dependency.
|
||
|
||
## 4. Test Strategy
|
||
|
||
- **Target Unit Test File:** `tests/frontend/blog-form-modal-escape.spec.ts` (new file, in the dedicated `tests/frontend/` folder, runnable via `npm test` from the repo root).
|
||
- **Mocking Strategy:** No runtime mocking required. The test uses the same source-text assertion pattern as `tests/frontend/challenge-card-accessibility.spec.ts`: read `frontend/src/app/features/admin/blog/blog-form-modal.component.ts` once with `fs.readFileSync` and assert that:
|
||
1. `HostListener` is imported from `@angular/core` (presence of the symbol in the import block).
|
||
2. The exact decorator `@HostListener('document:keydown.escape')` is present on the class.
|
||
3. The decorated handler method calls `this.onCancel()`.
|
||
4. The handler is guarded by `this.open()` and `this.saving()` (to prevent Escape from cancelling an in-flight save, matching the `change-password-modal` contract).
|
||
5. The existing `onCancel()` method still emits `cancel` (regression guard that the wiring remains intact).
|
||
- The test must import `'@angular/compiler'` at the top for parity with the rest of the frontend suite (see `tests/frontend/blog-admin-form.spec.ts:1`).
|
||
- Avoid standing up TestBed, ComponentFixture, or any DOM/jest-environment-jsdom behavior — keep the test pure and instant, per the `IMPORTANT RULE CONCERNING TESTS` instructions.
|
||
- All tests must pass via `npm test` (or `npm run test:frontend`) inside the container without any UI/browser interaction.
|