feat: quick-jot note creation modal #21

Merged

forgejo_admin merged 1 commit from 18-feat-quick-jot-note-creation-modal into main 2026-03-14 23:36:14 +00:00

Conversation 3 Commits 1 Files changed 5 +426 -9

forgejo_admin commented 2026-03-14 23:25:21 +00:00

Owner

Copy link

Summary

• Add a floating action button and modal for rapid note creation from anywhere in the app
• Users can press n or click the FAB to open a modal that creates notes via the pal-e-docs API
• Supports title, body, project selection, note type, slug preview, toast notifications, and focus trap

Changes

• src/lib/api.ts: Added createNote() function and CreateNotePayload interface for POST /notes
• src/routes/api/notes/+server.ts: New API proxy route that forwards note creation to the backend, with validation and 409 handling
• src/lib/components/QuickJot.svelte: New modal component with title (required), body (optional), project dropdown, note type selector (todo/doc/reference/journal), slug preview, focus trap, escape-to-close, backdrop click, error display, and success toast with link
• src/routes/+layout.svelte: Added FAB (bottom-right, #e94560, hover scale), keyboard shortcut (n key when not in input), and QuickJot mount
• src/routes/+layout.server.ts: New layout server load that fetches projects list for the modal dropdown

Test Plan

• Press n on any page (not in a text field) -- modal opens
• Click the red + FAB in bottom-right -- modal opens
• Fill in title, optional body, select project and type -- submit creates note
• Verify toast appears top-right with "View" link after creation
• Submit with duplicate title -- error message appears in modal
• Press Escape or click backdrop -- modal closes
• Tab key cycles within modal (focus trap)
• npm run build passes, npm run check has 0 errors

Review Checklist

• Passed automated review-fix loop
• No secrets committed
• No unnecessary file changes
• Commit messages are descriptive

Related

• Closes #18
• plan-2026-03-13-pal-e-frontend

## Summary - Add a floating action button and modal for rapid note creation from anywhere in the app - Users can press `n` or click the FAB to open a modal that creates notes via the pal-e-docs API - Supports title, body, project selection, note type, slug preview, toast notifications, and focus trap ## Changes - `src/lib/api.ts`: Added `createNote()` function and `CreateNotePayload` interface for POST /notes - `src/routes/api/notes/+server.ts`: New API proxy route that forwards note creation to the backend, with validation and 409 handling - `src/lib/components/QuickJot.svelte`: New modal component with title (required), body (optional), project dropdown, note type selector (todo/doc/reference/journal), slug preview, focus trap, escape-to-close, backdrop click, error display, and success toast with link - `src/routes/+layout.svelte`: Added FAB (bottom-right, #e94560, hover scale), keyboard shortcut (`n` key when not in input), and QuickJot mount - `src/routes/+layout.server.ts`: New layout server load that fetches projects list for the modal dropdown ## Test Plan - [ ] Press `n` on any page (not in a text field) -- modal opens - [ ] Click the red `+` FAB in bottom-right -- modal opens - [ ] Fill in title, optional body, select project and type -- submit creates note - [ ] Verify toast appears top-right with "View" link after creation - [ ] Submit with duplicate title -- error message appears in modal - [ ] Press Escape or click backdrop -- modal closes - [ ] Tab key cycles within modal (focus trap) - [ ] `npm run build` passes, `npm run check` has 0 errors ## Review Checklist - [x] Passed automated review-fix loop - [x] No secrets committed - [x] No unnecessary file changes - [x] Commit messages are descriptive ## Related - Closes #18 - `plan-2026-03-13-pal-e-frontend`

forgejo_admin added 1 commit 2026-03-14 23:25:21 +00:00

feat: quick-jot note creation modal with FAB and keyboard shortcut ... 524a36b0a6

Add a floating action button and modal for rapid note creation from
anywhere in the app. Press 'n' or click the FAB to open. Supports
title, body, project selection, and note type. Includes toast
notifications, focus trap, and slug preview.

Closes #18

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

forgejo_admin commented 2026-03-14 23:25:53 +00:00

Author

Owner

Copy link

Self-Review

Files reviewed: 5 changed files, +416/-8 lines

Findings

No blocking issues found.

Code quality:

• API proxy route (api/notes/+server.ts) follows the established pattern from api/boards/[slug]/items/[id]/+server.ts
• createNote() in api.ts follows the existing moveItem() pattern
• Layout server load has graceful error fallback (empty array) so API downtime does not break the app
• Focus trap, escape-to-close, and backdrop click all implemented correctly

Build verification:

• npm run build -- passes
• npm run check -- 0 errors, 1 pre-existing warning (autofocus in search page)
• npm run lint -- 0 new errors (2 pre-existing on main in search page)

Minor notes (non-blocking):

• 409 detection uses message.includes('409') which is slightly fragile but adequate since apiFetch embeds the status code in the error message
• Body content is wrapped in <p> tags client-side; backend is responsible for sanitization (DOMPurify already in use for rendering)

Verdict: LGTM -- ready for manual QA.

## Self-Review **Files reviewed:** 5 changed files, +416/-8 lines ### Findings No blocking issues found. **Code quality:** - API proxy route (`api/notes/+server.ts`) follows the established pattern from `api/boards/[slug]/items/[id]/+server.ts` - `createNote()` in `api.ts` follows the existing `moveItem()` pattern - Layout server load has graceful error fallback (empty array) so API downtime does not break the app - Focus trap, escape-to-close, and backdrop click all implemented correctly **Build verification:** - `npm run build` -- passes - `npm run check` -- 0 errors, 1 pre-existing warning (autofocus in search page) - `npm run lint` -- 0 new errors (2 pre-existing on main in search page) **Minor notes (non-blocking):** - 409 detection uses `message.includes('409')` which is slightly fragile but adequate since `apiFetch` embeds the status code in the error message - Body content is wrapped in `<p>` tags client-side; backend is responsible for sanitization (DOMPurify already in use for rendering) **Verdict: LGTM** -- ready for manual QA.

forgejo_admin commented 2026-03-14 23:28:40 +00:00

Author

Owner

Copy link

PR #21 Review

BLOCKERS

1. Dashboard nav link removed (scope creep / regression)

The current +layout.svelte on main (line 43) includes a /dashboard nav link:

<a href="/dashboard" class="...">Dashboard</a>

The PR's version of the nav section reformats the links (Prettier-style line breaks) but silently drops the Dashboard link entirely. The dashboard/ route and its +page.server.ts / +page.svelte still exist on disk. This is an unrelated change that removes a working navigation entry and constitutes both scope creep and a user-facing regression. Must be restored.

NITS

1. Body content wraps in <p> tags via raw string interpolation (defense-in-depth gap)

QuickJot.svelte line ~62:

payload.html_content = `<p>${body.trim()}</p>`;

If a user types <img src=x onerror=alert(1)> in the body field, it gets sent raw to the backend. The existing rendering pipeline sanitizes on output via DOMPurify, so this is not an active XSS vulnerability. However, best practice is to sanitize or HTML-escape on input as well (defense in depth). Consider using a simple HTML escape function before wrapping in <p> tags, or importing the existing sanitize() from $lib/sanitize -- though note that module uses isomorphic-dompurify which works server-side too.

2. Fragile 409 detection in API proxy route

src/routes/api/notes/+server.ts line ~46:

if (message.includes('409')) {

The apiFetch error message format is "API request failed: 409 Conflict (http://...)", so this works today. But it's fragile -- if the URL ever contains "409" it would false-positive. A more robust approach: check res.status directly in apiFetch or parse the status code from the error message pattern.

3. FAB visible behind modal backdrop

The FAB button has z-50 and is always rendered. The modal backdrop also has z-50. When the modal is open, the red FAB button bleeds through the semi-transparent black backdrop (bg-black/50). Consider hiding the FAB when the modal is open:

{#if !quickJotOpen}
<button class="fixed bottom-6 right-6 z-50 ...">+</button>
{/if}

4. createNote() and CreateNotePayload in api.ts -- partial dead code

The createNote() function is added to api.ts and used by the API proxy route (+server.ts), which is correct. However, CreateNotePayload is exported but never imported by any other module (the QuickJot component builds its own payload as Record<string, unknown>). Minor -- not blocking, just noting the type could be reused by the component for type safety.

5. Root layout server load on every page

+layout.server.ts fetches listProjects() on every page load since it's the root layout. For the current app size this is fine. If the project list grows or latency becomes noticeable, consider moving the fetch to a dedicated page or adding caching. Not blocking.

SOP COMPLIANCE

• Branch named after issue (18-feat-quick-jot-note-creation-modal matches issue #18)
• PR body has: Summary, Changes, Test Plan, Related
• Related section references plan slug (plan-2026-03-13-pal-e-frontend)
• Closes #18 present in Related section
• No secrets, .env files, or credentials committed
• No unnecessary file changes -- Dashboard nav link removed (scope creep)
• Commit messages are descriptive

VERDICT: NOT APPROVED

One blocker: the /dashboard nav link was silently removed from the layout, which is unrelated to quick-jot and breaks existing navigation. Restore the Dashboard link, then this is ready to merge. The nits are non-blocking suggestions for follow-up.

## PR #21 Review ### BLOCKERS **1. Dashboard nav link removed (scope creep / regression)** The current `+layout.svelte` on `main` (line 43) includes a `/dashboard` nav link: ```html <a href="/dashboard" class="...">Dashboard</a> ``` The PR's version of the nav section reformats the links (Prettier-style line breaks) but silently drops the Dashboard link entirely. The `dashboard/` route and its `+page.server.ts` / `+page.svelte` still exist on disk. This is an unrelated change that removes a working navigation entry and constitutes both scope creep and a user-facing regression. Must be restored. ### NITS **1. Body content wraps in `<p>` tags via raw string interpolation (defense-in-depth gap)** `QuickJot.svelte` line ~62: ```typescript payload.html_content = `<p>${body.trim()}</p>`; ``` If a user types `<img src=x onerror=alert(1)>` in the body field, it gets sent raw to the backend. The existing rendering pipeline sanitizes on output via DOMPurify, so this is not an active XSS vulnerability. However, best practice is to sanitize or HTML-escape on input as well (defense in depth). Consider using a simple HTML escape function before wrapping in `<p>` tags, or importing the existing `sanitize()` from `$lib/sanitize` -- though note that module uses `isomorphic-dompurify` which works server-side too. **2. Fragile 409 detection in API proxy route** `src/routes/api/notes/+server.ts` line ~46: ```typescript if (message.includes('409')) { ``` The `apiFetch` error message format is `"API request failed: 409 Conflict (http://...)"`, so this works today. But it's fragile -- if the URL ever contains "409" it would false-positive. A more robust approach: check `res.status` directly in `apiFetch` or parse the status code from the error message pattern. **3. FAB visible behind modal backdrop** The FAB button has `z-50` and is always rendered. The modal backdrop also has `z-50`. When the modal is open, the red FAB button bleeds through the semi-transparent black backdrop (`bg-black/50`). Consider hiding the FAB when the modal is open: ```html {#if !quickJotOpen} <button class="fixed bottom-6 right-6 z-50 ...">+</button> {/if} ``` **4. `createNote()` and `CreateNotePayload` in `api.ts` -- partial dead code** The `createNote()` function is added to `api.ts` and used by the API proxy route (`+server.ts`), which is correct. However, `CreateNotePayload` is exported but never imported by any other module (the QuickJot component builds its own payload as `Record<string, unknown>`). Minor -- not blocking, just noting the type could be reused by the component for type safety. **5. Root layout server load on every page** `+layout.server.ts` fetches `listProjects()` on every page load since it's the root layout. For the current app size this is fine. If the project list grows or latency becomes noticeable, consider moving the fetch to a dedicated page or adding caching. Not blocking. ### SOP COMPLIANCE - [x] Branch named after issue (`18-feat-quick-jot-note-creation-modal` matches issue #18) - [x] PR body has: Summary, Changes, Test Plan, Related - [x] Related section references plan slug (`plan-2026-03-13-pal-e-frontend`) - [x] `Closes #18` present in Related section - [x] No secrets, `.env` files, or credentials committed - [ ] No unnecessary file changes -- **Dashboard nav link removed (scope creep)** - [x] Commit messages are descriptive ### VERDICT: NOT APPROVED One blocker: the `/dashboard` nav link was silently removed from the layout, which is unrelated to quick-jot and breaks existing navigation. Restore the Dashboard link, then this is ready to merge. The nits are non-blocking suggestions for follow-up.

forgejo_admin force-pushed 18-feat-quick-jot-note-creation-modal from 524a36b0a6 to 624de27f64 2026-03-14 23:32:46 +00:00 Compare

forgejo_admin commented 2026-03-14 23:35:13 +00:00

Author

Owner

Copy link

PR #21 Review (Re-review)

BLOCKERS

None. All three previously identified issues have been resolved:

1. Dashboard nav link -- Confirmed present in the layout nav. Branch has been rebased on main with PR #20's Dashboard link included alongside the new QuickJot feature links.
2. FAB z-index vs modal -- FAB is now conditionally hidden with {#if !quickJotOpen} and uses z-40. Modal backdrop sits at z-50, toast at z-[100]. No overlap possible.
3. Fragile 409 detection -- Server route now uses regex message.match(/API request failed: (\d+)/) to extract the numeric status code. Client checks res.status === 409 directly on the fetch response. Both are robust.

NITS

1. Duplicate listProjects() call on home page -- The new +layout.server.ts calls listProjects() on every route. The existing +page.server.ts (home page) also calls listProjects(). This means the home page fetches the project list twice per load. SvelteKit does not deduplicate layout vs page server loads. Consider removing the listProjects() call from +page.server.ts and reading data.projects from the parent layout data instead, or accepting the minor redundancy.

2. Unsanitized HTML in body -- QuickJot.svelte line 65 wraps raw user input in a <p> tag: `<p>${body.trim()}</p>`. If a user types HTML or script tags in the body field, they are sent verbatim as html_content. This relies entirely on the backend (pal-e-docs) to sanitize on ingestion. If the backend does not sanitize, this is a stored XSS vector. Worth confirming backend sanitization exists, or adding client-side escaping here.

3. SELECT added to isInput check -- Good fix (prevents n shortcut from firing in dropdowns), but this change also affects the / search shortcut behavior. Previously, pressing / while focused on a <select> would trigger search focus. Now it won't. This is probably the correct behavior, just noting the scope extends beyond the QuickJot feature.

ACCEPTANCE CRITERIA

• 1. FAB opens quick-jot modal
• 2. n key opens modal (when not in input/textarea/select)
• 3. Modal has: title, body, project dropdown, note_type selector
• 4. Slug auto-generated from title (displayed in real-time)
• 5. Submit creates note via POST /api/notes (proxy to backend)
• 6. Success toast with "View" link (auto-dismisses after 5s)
• 7. Error handling -- 409 duplicate slug shows inline error message
• 8. Escape/backdrop closes modal
• 9. Dashboard link present in nav
• 10. Build -- cannot verify directly (no Bash), PR description confirms pass

SOP COMPLIANCE

• Branch named after issue (18-feat-quick-jot-note-creation-modal -> Issue #18)
• PR body follows template (Summary, Changes, Test Plan, Related)
• Related references plan slug (plan-2026-03-13-pal-e-frontend)
• Closes #18 referenced in PR body
• No secrets committed (only .env.example with placeholder)
• No unnecessary file changes (5 files, all on-scope)
• Commit message follows conventional format

VERDICT: APPROVED

## PR #21 Review (Re-review) ### BLOCKERS None. All three previously identified issues have been resolved: 1. **Dashboard nav link** -- Confirmed present in the layout nav. Branch has been rebased on main with PR #20's Dashboard link included alongside the new QuickJot feature links. 2. **FAB z-index vs modal** -- FAB is now conditionally hidden with `{#if !quickJotOpen}` and uses `z-40`. Modal backdrop sits at `z-50`, toast at `z-[100]`. No overlap possible. 3. **Fragile 409 detection** -- Server route now uses regex `message.match(/API request failed: (\d+)/)` to extract the numeric status code. Client checks `res.status === 409` directly on the fetch response. Both are robust. ### NITS 1. **Duplicate `listProjects()` call on home page** -- The new `+layout.server.ts` calls `listProjects()` on every route. The existing `+page.server.ts` (home page) also calls `listProjects()`. This means the home page fetches the project list twice per load. SvelteKit does not deduplicate layout vs page server loads. Consider removing the `listProjects()` call from `+page.server.ts` and reading `data.projects` from the parent layout data instead, or accepting the minor redundancy. 2. **Unsanitized HTML in body** -- `QuickJot.svelte` line 65 wraps raw user input in a `<p>` tag: `` `<p>${body.trim()}</p>` ``. If a user types HTML or script tags in the body field, they are sent verbatim as `html_content`. This relies entirely on the backend (pal-e-docs) to sanitize on ingestion. If the backend does not sanitize, this is a stored XSS vector. Worth confirming backend sanitization exists, or adding client-side escaping here. 3. **`SELECT` added to isInput check** -- Good fix (prevents `n` shortcut from firing in dropdowns), but this change also affects the `/` search shortcut behavior. Previously, pressing `/` while focused on a `<select>` would trigger search focus. Now it won't. This is probably the correct behavior, just noting the scope extends beyond the QuickJot feature. ### ACCEPTANCE CRITERIA - [x] 1. FAB opens quick-jot modal - [x] 2. `n` key opens modal (when not in input/textarea/select) - [x] 3. Modal has: title, body, project dropdown, note_type selector - [x] 4. Slug auto-generated from title (displayed in real-time) - [x] 5. Submit creates note via POST /api/notes (proxy to backend) - [x] 6. Success toast with "View" link (auto-dismisses after 5s) - [x] 7. Error handling -- 409 duplicate slug shows inline error message - [x] 8. Escape/backdrop closes modal - [x] 9. Dashboard link present in nav - [x] 10. Build -- cannot verify directly (no Bash), PR description confirms pass ### SOP COMPLIANCE - [x] Branch named after issue (`18-feat-quick-jot-note-creation-modal` -> Issue #18) - [x] PR body follows template (Summary, Changes, Test Plan, Related) - [x] Related references plan slug (`plan-2026-03-13-pal-e-frontend`) - [x] Closes #18 referenced in PR body - [x] No secrets committed (only `.env.example` with placeholder) - [x] No unnecessary file changes (5 files, all on-scope) - [x] Commit message follows conventional format ### VERDICT: APPROVED

forgejo_admin merged commit d8e8d0f92a into main 2026-03-14 23:36:14 +00:00

forgejo_admin deleted branch 18-feat-quick-jot-note-creation-modal 2026-03-14 23:36:14 +00:00

forgejo_admin referenced this pull request from a commit 2026-03-14 23:36:16 +00:00

feat: quick-jot note creation modal (#21)

forgejo_admin referenced this pull request from a commit 2026-03-14 23:52:23 +00:00

[skip ci] Update deployment image tag to d8e8d0f92af5ca9e1a9dba09951537dfaaef500c

forgejo_admin referenced this pull request 2026-03-15 01:08:28 +00:00

feat: Keycloak OIDC auth — protect write operations #22

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

domain:backend

  

domain:devops

  

domain:frontend

No labels

domain:backend

domain:devops

domain:frontend

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

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

forgejo_admin/pal-e-docs-app!21

No description provided.