This repository was archived by the owner on May 23, 2026. It is now read-only.
fix: use lvt/testing client library in chat example - #17
Merged
Conversation
Replace custom serveClientLibrary function with e2etest.ServeClientLibrary from lvt/testing package. This ensures the chat example uses the same embedded client library as other examples, fixing CI test failures where the client couldn't be found. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR replaces the custom serveClientLibrary function in the chat example with the standardized e2etest.ServeClientLibrary function from the lvt/testing package. This change ensures consistency with other examples and fixes CI test failures where the client library couldn't be found at the custom paths.
Key Changes:
- Removed custom client library serving logic that searched multiple local paths
- Added dependency on github.com/livetemplate/lvt/testing package
- Updated to use the embedded client library via
e2etest.ServeClientLibrary
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| chat/main.go | Replaced custom serveClientLibrary function with standardized e2etest.ServeClientLibrary import and usage |
| chat/go.mod | Added github.com/livetemplate/lvt dependency to support the new client library serving approach |
| chat/go.sum | Updated checksums to include the new lvt/testing dependency |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| require ( | ||
| github.com/chromedp/chromedp v0.14.2 | ||
| github.com/livetemplate/livetemplate v0.7.0 | ||
| github.com/livetemplate/lvt v0.0.0-20251213063335-769e058d10cd |
There was a problem hiding this comment.
The version of github.com/livetemplate/lvt being added (v0.0.0-20251213063335-769e058d10cd) is newer than the versions used in most other examples. For consistency, consider using the same version that other examples use (v0.0.0-20251130141940-9b94cde94e9d as used in counter, avatar-upload, and todos examples), unless there's a specific reason this example requires a newer version. Version inconsistencies across examples can make maintenance more difficult and may lead to unexpected behavior differences.
Suggested change
| github.com/livetemplate/lvt v0.0.0-20251213063335-769e058d10cd | |
| github.com/livetemplate/lvt v0.0.0-20251130141940-9b94cde94e9d |
7 tasks
adnaan
added a commit
that referenced
this pull request
Apr 25, 2026
…navigation (#74) * feat: patterns example Session 4 (patterns #17-21) — dialogs, tabs & navigation Adds five new patterns under a new "Dialogs, Tabs & Navigation" category in the patterns example app, covering the Tier-1/Tier-2 navigation primitives that LiveTemplate exposes on top of native browser features. #17 Modal Dialog — native <dialog> + Invoker Commands (command/commandfor) + #hash deep linking; field-level validation inside the open dialog exercises the client v0.8.33 morphdom fix #18 Confirm Dialog — per-item <dialog id="confirm-{{.ID}}"> for destructive-action confirmation, button value attribute (no hidden inputs), per-item hash linking #19 Tabs (HATEOAS) — <a href="?tab=…"> driven by the in-band __navigate__ WebSocket action (server v0.8.19+, client v0.8.26+); Mount-only controller with validTabs allowlist #20 SPA Navigation — explainer page covering same-pathname __navigate__, cross-pathname reconnect, and the lvt-nav:no-intercept opt-out #21 Keyboard Shortcuts — Tier-2 lvt-on:window:keydown + lvt-key for a command-palette-style overlay, plus a Tier-1 form fallback Library bumped to v0.8.21 (from a stale v0.8.19 pre-release pseudo-version) to align go.sum with the latest tagged release. Examples/.gitignore now includes patterns/patterns so the compiled binary doesn't reappear after local `go run` cycles. E2E coverage: - TestModalDialog — Open_Via_Button, Open_Via_Hash_Link, Submit_Invalid_…, Submit_Valid_…, Browser_Back_Closes_Dialog, UI_Standards, Visual_Check - TestConfirmDialog — Open_Specific_Item_Confirm, Cancel_…, Confirm_Deletes Per_Item_Hash_Link_…, UI_Standards, Visual_Check - TestTabs — Default_Tab_…, Click_Settings_…, Tab_Switch_Uses_WebSocket_… Direct_URL_Load_…, Invalid_Tab_Falls_Back_…, UI_Standards, Visual_Check - TestSPANavigation — Initial_Load, Same_Pathname_Step_Update_No_HTTP, External_Link_Has_No_Intercept_Attribute, UI_Standards, Visual_Check - TestKeyboardShortcuts — Slash_Key_Opens_Panel, Escape_Closes_Panel, Tier1_Form_Fallback_Works, UI_Standards, Visual_Check - cross_handler_nav_test.go — Index_To_Modal_Dialog_No_Stale_Dom, Tabs_Same_Pathname_Uses_WebSocket, SPA_Navigation_Cross_Pathname_Reconnects All five Category 5 entries in data.go flipped to Implemented:true. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: trim Session 4 comment noise Drops version-specific references (v0.8.33, v0.8.19+, etc.) and "Session 4"-style task references from controller doc-comments and test inline comments. Also removes comments that restated test mechanics already evident from the test code, and pares the controller doc-comments to the load-bearing WHY (e.g. "errors must render inside still-open dialog", "button value attribute, not hidden input"). No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: address Claude/Copilot bot review comments Tabs: - Mount() now resets to "overview" for any unknown ?tab= value regardless of current state, matching the template's "fall back silently to Overview" claim. Previously a user already on Settings who hit ?tab=garbage would silently keep Settings. - Reword tabs.tmpl: validTabs is a package-level allowlist, not controller instance state. - Use {{else if}} for the mutually-exclusive tab content blocks. Modal Dialog: - Add explicit dialog-close assertion after Submit_Valid_Form. Catches a hypothetical bug where the success flash appears but the dialog fails to close. - Document why we use BindAndValidate (rather than ctx.ValidateForm used by Sessions 1–3 form patterns): ExtractFormSchema does not currently surface the form schema for forms inside <dialog>, so the explicit struct + validator tags is the same shape used by examples/dialog-patterns and works reliably. Confirm Dialog: - Reword "Refresh to reseed the list" to also acknowledge that navigating away and back reseeds (via Mount). SPA Navigation: - Move the period outside <strong> in the Step indicator (cosmetic). Tests: - Restore window.fetch via t.Cleanup so a mid-test failure cannot leak the override into later subtests (3 places: TestTabs, TestSPANavigation, and the cross-handler version which is removed below). - Drop the duplicate Tabs_Same_Pathname_Uses_WebSocket subtest from cross_handler_nav_test.go — the same assertion lives in TestTabs/Tab_Switch_Uses_WebSocket_Not_HTTP. - Capture and check the chromedp.Run error around the leak-detection Evaluate in Index_To_Modal_Dialog_No_Stale_Dom; treat "no-wrapper" as a hard failure. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: address Claude bot round-2 review Comment style: - Trim controller doc-comments to one short line each, per CLAUDE.md ("never write multi-line comment blocks"). - Drop state_navigation.go type and field comments (they were restating identifier names). - Compress validateNav comment to one line. Tabs: - Rewrite Mount() as a switch with one-line comments on each branch so the "explicit unknown → reset" vs "first load → default" intent is legible. Confirm Dialog: - Rename the per-row form `confirmDelete` → `delete` to match the submit button's name attribute. - Add a {{/* ... */}} comment explaining why the dialogs render outside the table (so future maintainers don't try to nest them back into <tbody>). Tests: - Rename `hasDeleted` → `rowExists` (the variable holds the existence result, not a deletion flag). - Rename `__origFetch2` → `__origFetchSPA` (less collision-prone if more fetch-mocking tests get added). - Add Open_Button_Click_Opens_Panel subtest to TestKeyboardShortcuts; the keyboard path was covered but the Tier-1 form-button click through name="open" wasn't. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs: address Claude bot round-3 nits - Inline comment near ctx.GetString("value") in ConfirmDialog.Delete documenting that "value" is the framework's key for the clicked submit button's value attribute (independent of the button's name). Same idiom as examples/dialog-patterns. - Inline comment in SPANavController.Mount noting that out-of-range ?step= values fall through to the default — symmetric with the TabsController handling. - Template comment in keyboard-shortcuts.tmpl marking the search input as intentionally decorative (wiring to Change()/Submit() is out of scope for the keyboard-shortcut demo). No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: add method="POST" to forms + verify form state after Save The four Session 4 forms (modal-dialog #save, confirm-dialog #delete, keyboard-shortcuts #closePanel + #openPanel) were missing method="POST" on the <form> tag. CLAUDE.md requires it for the Tier-1 HTTP POST fallback path; without it the no-JS user flow breaks. All four forms now match the convention used by Sessions 1–3. Also adds the form-state-after-mutation assertion to Submit_Valid_Form_Closes_Dialog_And_Updates_State per the project's E2E checklist: re-opens the dialog after save and verifies the name input now holds the saved value (the value="{{.Name}}" template expression should have rerendered). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test: cover ?step=3 + out-of-range step in TestSPANavigation Adds Step_3_Direct_URL_Activates and Out_Of_Range_Step_Falls_Back_To_Default subtests so the spaNavMaxStep clamp is exercised end-to-end (mirroring the TestTabs/Invalid_Tab_Falls_Back_To_Default precedent). Also adds a one-line comment in ConfirmDialogController.Delete noting that unknown ids are an intentional no-op (next render reconciles drift between client and server item lists). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: tidy form names in keyboard-shortcuts and modal-dialog Per the action resolution order (button name → form name → "submit"), the form names "closePanel"/"openPanel" in keyboard-shortcuts.tmpl were shadowed by the button names "close"/"open" and never reached. Renamed to match the buttons so the form-name encoding is honest if a future contributor reads it for routing semantics. modal-dialog.tmpl had a duplicate name="save" on both the form and the submit button — the button wins, so the form attribute was redundant. Dropped it. No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs: clarify confirm-dialog empty-state reseed conditions Mount re-seeds whenever state.Items is empty AND ctx.Action() == "" — which includes WebSocket reconnects (network blip, tab backgrounded), not just explicit reload/navigation. Updated the empty-state message to mention all three triggers so it doesn't contradict the code. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: address Claude bot round-8 nits - spa-navigation.tmpl: rel="noopener" → "noopener noreferrer" on the external link (idiomatic for target="_blank"; readers will copy this pattern). - keyboard-shortcuts.tmpl: extended the decorative-search-input comment to note the autofocus + Escape-handling interaction caveat. Production use should defer focus to a wrapper or drop autofocus. - handlers_navigation.go: brief comment in ConfirmDialogController.Delete noting that id is a server-rendered Item.ID echoed back via the form (not free-form user input), so no allowlist check is needed. No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: address Claude bot round-9 nits - keyboard-shortcuts.tmpl: drop autofocus from the decorative search input. The input is wired to nothing; autofocus only created an Escape-key interaction caveat. Updated the template comment to document the trade-off explicitly. - confirm-dialog.tmpl: soften the empty-state message — drop the "wait for the WebSocket to reconnect" claim. Reseed-on-reconnect is contingent on framework state-clone semantics that could change; the safer "reload or navigate away and back" claim is unambiguous. No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix: scope keyboard-shortcut listeners by panel state Previously the outer <article> bound lvt-on:window:keydown="open" unconditionally. While the panel was open, "/" still routed to Open() which bailed out via the re-entrancy guard — but the WebSocket round- trip happened regardless. Now the outer article binds either the Open or the Close listener based on .PanelOpen, so stray keypresses can't trigger no-op round- trips. The inner panel article keeps its aria-label. Also replaces the decorative search input's lack of autofocus with an explicit `disabled` attribute so the lack-of-affordance is visible to the user (they can see the field is non-functional, and it can't grab focus to interfere with the Escape handler). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore: defensive ActiveTab invariant guard in TabsController.Mount Add a belt-and-suspenders fallback after the if-block: regardless of which branch ran (or if the if-block was skipped because Action() != ""), state.ActiveTab is guaranteed to be in validTabs by the time Mount returns. Closes a hypothetical edge case where a malformed message could route through Mount with the action set but ActiveTab unset. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced Apr 25, 2026
adnaan
added a commit
that referenced
this pull request
Apr 27, 2026
* fix(patterns): close Session 1 follow-ups #62-#65 — title + a11y - #62: index page rendered "LiveTemplate Patterns — LiveTemplate Patterns" due to layout suffix on a Title that already named the brand. Drop the index Title; layout now conditionally prepends "{Title} — " only when Title is non-empty, leaving the home page tab as the bare brand name. - #63: aria-label on edit-row name/email inputs. - #64: aria-label on reset-input message field. - #65: aria-label="Active for {{.Name}}" on per-row bulk-update checkboxes so screen readers announce row identity for each checkbox. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(patterns): SetFlash audit — auto-expire success in #6, #7, #15, #16 Under the v0.8.19+ persistent-flash model, success flashes without FlashExpiry stay sticky indefinitely until the next action. That contradicts the demo's UX intent (a transient "Saved" toast). Add livetemplate.FlashExpiry(flashSuccessExpiry) to: - #6 File Upload (Uploaded: <name>) - #7 Preserving File Inputs (Saved: <name>) - #15 Progress Bar (Job complete) - #16 Async Operations (Fetch complete) Errors stay sticky — they require user acknowledgment. Pattern #4 (Bulk Update) keeps its mutual-clear pattern (success/info ClearFlash of the opposite key). This brings the four outliers in line with the canonical model from #25 (Flash Messages) and #17 (Modal Dialog). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(patterns): add README Quick start, 31-pattern catalog by category, architecture pointer (state_*.go / handlers_*.go / templates/*), testing instructions including the multi-tab peer-tab pattern, and references to the proposal and project conventions. The in-app index page remains the canonical pattern catalog; the README is the entry point and pointer. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(patterns): #15 disconnect resilience + render-nudge for FlashExpiry Two concerns from manual review on iPhone: 1) Success flashes (#6, #7, #15, #16) didn't actually fade after FlashExpiry. The expiry is render-driven — the entry is only pruned on the next render — so without a follow-up render at the deadline, the flash sits visually until the user's next interaction. Pattern #25 had this nudge inline; extracted as nudgeFlashExpiry() and applied to the four sticky-success patterns. Each controller gets a no-op Refresh handler as the action target. 2) Pattern #15 progress bar hung on iPhone background-then-return. The WebSocket dies (often as a "zombie" socket reporting OPEN but with dead TCP), the goroutine errors on next TriggerAction, exits, and the UI is stuck. Fixes: - Goroutine retries TriggerAction for ~5s (50 × 100ms) when the session group has no live connections — covers brief iOS app- switches under the client's 3s visibility-reconnect threshold. - state.Progress and state.Done are lvt:"persist" so completed runs survive a reconnect (user keeps the "Run Again" button rather than snapping back to "Start Job"). state.Running is intentionally NOT persisted: the same trade-off Pattern #31 makes — a stale Running=true with no goroutine is worse UX than a clean Start button after a long disconnect. - UpdateProgress guards on state.Done already being true: if any race produces a stale tick after Done is set, the trailing tick is silently dropped instead of overwriting Progress with a mid-flight value (the screenshot reproducer for this bug showed Run Again next to a 70% bar). - No Mount-driven revival. An earlier attempt that spawned a fresh goroutine on reconnect raced with the original goroutine's retry loop and was the source of the impossible state. New chromedp tests cover four scenarios: - Brief disconnect within retry window completes - Long disconnect beyond retry window settles without impossible state - Multiple disconnect cycles never produce impossible state - Done state survives reconnect via persist Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(patterns): address review feedback on Session 7 - Multiple_Disconnect_Cycles test: replace fixed 300/400ms sleeps with state-based waits (capture pre-cycle progress, wait for it to advance after each disconnect+connect). - Done_State_Survives_Reconnect test: collapse the 500ms sleep — fire disconnect+connect back-to-back, sync on WaitForWebSocketReady. - Constants: rename progressRetryBudget (50) to derived form. Now progressRetryWindow (5s) and progressRetryDelay (100ms) are the source of truth; progressRetryAttempts is computed from them. - ProgressBarController doc: explicit "no OnConnect" rationale — restorePersistedState already hydrates Progress/Done on reconnect. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * docs(patterns): clarify Pattern #15 retry lifetime + sleep rationale Two doc-only clarifications from review: - ProgressBarController doc: the 5s retry window is per-tick, not for the whole run. A goroutine that never reconnects can live up to ~50s before all 10 ticks exhaust their budgets and exit. The earlier prose read like a 5s ceiling on the whole run. - TestProgressBar Brief_Disconnect_Within_Retry_Window: the inner chromedp.Sleep(1s) keeps the connection down during the retry window on purpose — there's no client-observable signal during the goroutine's retry loop, so a condition-based wait wouldn't substitute here. Comment parallels the existing rationale on the Long_Disconnect 7s sleep. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(patterns): skip retry sleep on first attempt of tickWithRetry The previous loop slept progressRetryDelay AFTER every attempt, including the 50th (final) one — which is wasted work when the goroutine is about to give up and exit. Sleep only between attempts. Net effect: each tick that exhausts its retry budget terminates ~100ms sooner. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * style(patterns): const progressRetryAttempts + idiomatic err handling + accurate worst-case Round-4 review nits: - progressRetryAttempts: var → const. Both Duration operands are typed constants, int(typedConst) is a constant expression, so the trio stays immutable as intended. - tickWithRetry: drop the else-after-return pattern in favour of the more idiomatic "early return on success" shape. - ProgressBarController doc: worst-case lifetime now correctly includes progressTickRate per tick — (500ms + 5s) × 10 = ~55s, not ~50s. No behaviour change. TestProgressBar (all 7 subtests) still green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
serveClientLibraryfunction withe2etest.ServeClientLibraryfrom lvt/testing packageRoot Cause
The chat example had a custom
serveClientLibraryfunction that looked for the client in local paths:livetemplate-client.js../client/dist/livetemplate-client.browser.js../../client/dist/livetemplate-client.browser.jsBut the CI workflow copies the client to
lvt/testing/livetemplate-client.browser.js, which is then embedded viago:embedin the lvt/testing package.Test plan
🤖 Generated with Claude Code