feat: remove redundant examples, merge component patterns into todos#46
feat: remove redundant examples, merge component patterns into todos#46
Conversation
…42) - Delete todos-progressive (Tier 1 CRUD patterns covered by todos) - Delete profile-progressive (form/validation patterns covered by todos-progressive) - Delete todos-components (standalone); merge modal + toast into todos - todos now demonstrates: CRUD, auth, pagination, delete confirmation modal, toast notifications - Add delete-via-modal E2E test to todos_test.go - Update test-all.sh, README.md, CLAUDE.md to reflect 8 remaining examples Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Consolidates the example set by removing redundant “progressive/components” demos and making todos/ the single, comprehensive reference example (now including modal-confirm delete + toast notifications).
Changes:
- Deleted redundant examples (
todos-progressive/,profile-progressive/,todos-components/) and removed them fromtest-all.sh. - Integrated modal confirmation + toast component patterns into
todos/(template + controller + component template registration). - Added an E2E test covering delete via confirmation modal in
todos/todos_test.go.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
todos/todos.tmpl |
Routes delete through a confirmation action and renders modal/toast component templates; adds positioning CSS for components. |
todos/controller.go |
Adds modal-driven delete flow and toast notifications on add/toggle/clear/delete; initializes component state in lifecycle methods. |
todos/state.go |
Extends state with toast + modal component state and persisted pending-delete ID. |
todos/main.go |
Registers modal/toast component template sets with LiveTemplate; adds convertTemplateSet helper. |
todos/todos_test.go |
Adds E2E coverage for delete via confirmation modal flow. |
README.md |
Updates examples table to reflect consolidation into todos/. |
CLAUDE.md |
Updates “Reference Examples” section to point to todos/ as the canonical example. |
test-all.sh |
Removes deleted examples from WORKING_EXAMPLES. |
todos-progressive/* |
Deleted redundant Tier 1 CRUD progressive example. |
profile-progressive/* |
Deleted redundant Tier 1 validation form example. |
todos-components/* |
Deleted standalone components example after merging patterns into todos/. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| // ConfirmDeleteConfirm executes the deletion after the user confirms the modal. | ||
| func (c *TodoController) ConfirmDeleteConfirm(state TodoState, ctx *livetemplate.Context) (TodoState, error) { | ||
| if state.DeleteID == "" { |
There was a problem hiding this comment.
ConfirmDeleteConfirm returns early when DeleteID is empty, which can leave the confirmation modal open with no way to proceed (and provides no feedback). Consider hiding the modal and/or adding an error/warning toast when there is no pending DeleteID (and optionally clearing DeleteID defensively).
| if state.DeleteID == "" { | |
| if state.DeleteID == "" { | |
| // No pending delete ID: hide the modal and notify the user instead of leaving the UI stuck. | |
| state.DeleteConfirm.Hide() | |
| state.Toasts.AddWarning("Nothing to delete", "No todo is pending deletion.") | |
| state.DeleteID = "" |
todos/todos.tmpl
Outdated
| /* Toast container: fixed top-right overlay (required for component positioning) */ | ||
| [data-toast-container] { position: fixed; top: 1rem; right: 1rem; z-index: 50; display: flex; flex-direction: column; gap: 0.5rem; width: 360px; } | ||
| [data-toast-container] [role="alert"] { background: var(--pico-card-background-color); border: 1px solid var(--pico-muted-border-color); border-radius: var(--pico-border-radius); padding: 0.75rem 1rem; box-shadow: 0 4px 12px rgba(0,0,0,0.15); display: flex; align-items: flex-start; gap: 0.5rem; } | ||
| [data-toast-container] [role="alert"] > div { flex: 1; } | ||
| [data-toast-container] [role="alert"] button { width: auto; min-width: auto; margin: 0; padding: 0.25rem; --pico-background-color: transparent; --pico-border-color: transparent; color: var(--pico-muted-color); line-height: 1; } | ||
| /* Modal overlay (required for component positioning) */ | ||
| [data-modal] { position: fixed; inset: 0; z-index: 40; display: flex; align-items: center; justify-content: center; background: rgba(0,0,0,0.5); } |
There was a problem hiding this comment.
This adds custom CSS for toast/modal positioning, but the repo guidance explicitly says "Do NOT write custom CSS" (see CLAUDE.md:98). If this styling is required for components, consider enabling the components' styled mode (so the component library provides its own CSS) or updating the documented convention to allow the minimal CSS needed for component positioning.
| /* Toast container: fixed top-right overlay (required for component positioning) */ | |
| [data-toast-container] { position: fixed; top: 1rem; right: 1rem; z-index: 50; display: flex; flex-direction: column; gap: 0.5rem; width: 360px; } | |
| [data-toast-container] [role="alert"] { background: var(--pico-card-background-color); border: 1px solid var(--pico-muted-border-color); border-radius: var(--pico-border-radius); padding: 0.75rem 1rem; box-shadow: 0 4px 12px rgba(0,0,0,0.15); display: flex; align-items: flex-start; gap: 0.5rem; } | |
| [data-toast-container] [role="alert"] > div { flex: 1; } | |
| [data-toast-container] [role="alert"] button { width: auto; min-width: auto; margin: 0; padding: 0.25rem; --pico-background-color: transparent; --pico-border-color: transparent; color: var(--pico-muted-color); line-height: 1; } | |
| /* Modal overlay (required for component positioning) */ | |
| [data-modal] { position: fixed; inset: 0; z-index: 40; display: flex; align-items: center; justify-content: center; background: rgba(0,0,0,0.5); } |
todos/todos_test.go
Outdated
| // Step 3: verify a toast appeared | ||
| var hasToast bool | ||
| if err := chromedp.Run(ctx, chromedp.Evaluate(`document.querySelector('[data-toast-container]') !== null`, &hasToast)); err != nil { | ||
| t.Fatalf("Failed to check toast: %v", err) | ||
| } | ||
| if !hasToast { | ||
| t.Error("Expected toast notification after delete") | ||
| } |
There was a problem hiding this comment.
The toast assertion only checks that [data-toast-container] exists, but the container may be rendered even when there are no toasts; this makes the test pass without proving a delete toast was shown. Consider asserting that at least one toast element exists (e.g., a toast item/[role="alert"]/[data-toast]) and/or that expected toast text is present.
With SetStyled(false), all class="" attributes are empty — component layout depends entirely on structural CSS selectors. Toast fixes: - Apply display:flex at ToastInner and ToastLayout levels so the dismiss button sits at the right edge (not stacked below) - Set explicit width/height on SVG so the × dismiss icon is visible - Scope button selector to [data-toast-container] to avoid conflicts Modal fixes: - Style the panel div with background, border-radius, padding, shadow so the dialog has a proper card container on the overlay - Right-align action buttons with flex + gap + margin-top Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Uses a MutationObserver to watch [data-toast-container] for new alert elements and schedules setTimeout based on data-auto-dismiss attribute value. A document click handler dismisses all visible toasts when clicking outside the container. Both send dismiss_toast_notifications action to the server to remove the toast from state, keeping client and server in sync. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- controller.go: remove DismissToastNotifications() (client handles dismissal) - todos.tmpl: remove server-rendered toast HTML and custom modal/toast CSS; component templates now own their own <style> blocks; keep .visually-hidden - todos_test.go: assert on [data-lvt-toast-item] (client DOM) not server HTML - test-all.sh: use local go.work when present (dev mode with local module overrides) - CLAUDE.md: update reference examples + add ephemeral-components guide link Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add computed-style assertion to Delete_via_confirmation_modal subtest: checks that [data-lvt-toast-stack] has position=fixed, plus dumps all <style> elements, computed CSS, and stack HTML for debugging. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…odal on empty DeleteID - test.yml: remove todos-progressive, profile-progressive, todos-components from CI matrix (directories deleted in this PR) - controller.go: hide confirmation modal when DeleteID is empty so user isn't stuck with an open modal Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
CI Status
The toast E2E test ( All tests pass locally with |
Pick up trigger-attribute toast template + component CSS + TakePendingJSON from merged livetemplate/lvt#290. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
Closes #42
todos-progressive/— Tier 1 CRUD patterns are already demonstrated bytodos/profile-progressive/— form + validation patterns are a subset oftodos-progressivetodos-components/(standalone); merged its modal + toast component patterns intotodos/todos/to be the single comprehensive reference example: SQLite CRUD, BasicAuth, pagination, search, sort, delete confirmation modal, toast notificationsDelete via confirmation modalE2E test totodos/todos_test.gotest-all.shWORKING_EXAMPLES (11 → 8)README.mdtable andCLAUDE.mdReference ExamplesTest plan
go build ./...passes cleanlytodosE2E tests all pass including newDelete_via_confirmation_modalsub-test./test-all.shpasses for all 8 remaining examples🤖 Generated with Claude Code