feat!: attribute reduction — migrate all templates and Go code#225
feat!: attribute reduction — migrate all templates and Go code#225
Conversation
- auto_tables.go: lvt-click → name, lvt-data-id → data-id, lvt-confirm → data-confirm, lvt-submit → name, lvt-reset-on:success → lvt-el:reset:on:success - auto_tasks.go: lvt-click → lvt-on:click (checkbox), lvt-data-id → data-id, lvt-submit → name, lvt-reset-on:success → lvt-el:reset:on:success - page.go: lvt-click → name, lvt-data-id → data-id - interactive-block.ts: Remove checkLvtConfirm/extractLvtData imports, handle button[name] + lvt-on:click routing, form name/submitter routing, add local checkConfirm (data-confirm) and extractData (data-*) helpers - Bump livetemplate v0.8.4 → v0.8.16, @livetemplate/client ^0.8.4 → latest
…butes BREAKING CHANGE: All Go generators, templates, examples, docs, and tests updated to use new attribute syntax: - lvt-click → button name (buttons) or lvt-on:click (non-buttons) - lvt-submit → form name - lvt-data-* → data-* - lvt-confirm → data-confirm (CSP-safe) - lvt-reset-on:success → lvt-el:reset:on:success - lvt-change → lvt-on:change TypeScript client updated: - Removed checkLvtConfirm/extractLvtData imports - Button name routing + lvt-on:click for non-buttons - Form name/submitter routing - Local data-confirm and data-* extraction helpers Dependencies bumped: - livetemplate v0.8.4 → v0.8.16 - @livetemplate/client ^0.8.4 → ^0.8.19 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
PR Review: feat! attribute reduction (Phase 3A+3B)This is a well-scoped migration with consistent application across Go generators, TypeScript, and all template/doc files. The changes are coherent and the attribute table in the PR description makes the intent clear. Correctness issues
Stale Design observations
Submit button No deprecation bridge -- Old Minor
private checkConfirm(element: HTMLElement): boolean {
const message = element.dataset.confirm;
return !message || window.confirm(message);
}Coverage gapE2e tests are updated but explicitly not run per the test plan. Manual verification before merge is necessary, especially for the Overall the migration is consistent and the direction (using standard HTML attributes) is the right call. The main things to resolve before merging: the |
There was a problem hiding this comment.
Pull request overview
This PR implements the “attribute reduction” breaking change across Tinkerdown’s generators, client event handling, docs/examples, and test selectors, aligning the repo with the updated livetemplate / @livetemplate/client APIs.
Changes:
- Replace
lvt-click/lvt-submit/lvt-data-*/lvt-confirmwithname(button/form),lvt-on:click(non-button),data-*, anddata-confirmacross templates/docs/examples. - Update Go auto-generators and golden/unit/e2e tests to emit/target the new attribute syntax.
- Update the TypeScript interactive block routing and bump Go/TS dependencies to newer
livetemplateversions.
Reviewed changes
Copilot reviewed 100 out of 104 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| skills/tinkerdown/SKILL.md | Updates tutorial examples + attribute reference to new syntax. |
| skills/tinkerdown/reference.md | Updates full reference examples from lvt-* to name/data-*/lvt-on:*. |
| skills/tinkerdown/examples/01-todo-app.md | Migrates example to name + lvt-on:click + data-id. |
| skills/tinkerdown/examples/02-dashboard.md | Migrates refresh button to name="Refresh". |
| skills/tinkerdown/examples/03-contact-form.md | Migrates form submit + delete buttons to new syntax. |
| skills/tinkerdown/examples/04-blog.md | Migrates blog CRUD actions to new syntax. |
| skills/tinkerdown/examples/05-inventory.md | Migrates inventory actions + refresh to new syntax. |
| skills/tinkerdown/examples/06-survey.md | Migrates survey form and delete action to new syntax. |
| skills/tinkerdown/examples/07-booking.md | Migrates booking form and cancel action to new syntax. |
| skills/tinkerdown/examples/08-expense-tracker.md | Migrates expense tracker form + delete action. |
| skills/tinkerdown/examples/09-faq.md | Migrates FAQ form + delete action. |
| skills/tinkerdown/examples/10-status-page.md | Migrates status page actions + docs wording. |
| skill_examples_test.go | Adjusts expectations for updated “Key Attributes” content. |
| README.md | Updates attribute table to reflect new routing/confirm/data attributes. |
| parser.go | Updates comment to reflect button-name routing / lvt-on:click. |
| parser_test.go | Updates parser test fixtures/assertions to new attributes. |
| page.go | Updates generated templates to emit name + data-id for actions. |
| page_test.go | Updates page fixture to use name-based action buttons. |
| new_command_e2e_test.go | Updates scaffold verification for new form routing attribute. |
| lvtsource_sqlite_e2e_test.go | Updates chromedp selectors for new name/data-id attributes. |
| lvtsource_rest_e2e_test.go | Updates REST e2e template + selectors for name="Refresh". |
| lvtsource_pg_e2e_test.go | Updates PG e2e selectors for name="Refresh". |
| lvtsource_markdown_e2e_test.go | Migrates markdown e2e templates + selectors for new attributes. |
| lvtsource_e2e_test.go | Updates exec-source e2e selectors for name="Refresh". |
| execargs_e2e_test.go | Migrates exec args e2e selectors and messages to form[name="Run"]. |
| auto_tables_e2e_test.go | Updates auto-table e2e selectors for new routing/data attributes. |
| action_buttons_e2e_test.go | Updates action button selectors to button[name="..."]. |
| internal/server/playground.html | Updates embedded playground template examples. |
| docs/playground.html | Updates docs playground template examples. |
| docs/sources/sqlite.md | Updates sqlite source docs examples to new action/data/submit syntax. |
| docs/reference/lvt-attributes.md | Updates attribute reference (event handling, data attrs, confirm, lifecycle). |
| docs/reference/frontmatter.md | Updates frontmatter example form routing to name. |
| docs/guides/auto-rendering.md | Updates auto-rendering examples + note from lvt-data-id to data-id. |
| docs/guides/go-templates.md | Updates Go template examples to new action/data syntax. |
| docs/guides/debugging.md | Updates debugging snippet to name/data-id. |
| docs/guides/ai-generation.md | Updates AI-generation guide snippets to name on forms. |
| docs/getting-started/quickstart.md | Updates quickstart button example to name="SayHello". |
| docs/llms.txt | Updates LLM-facing examples/table to new attribute syntax. |
| docs/llm-system-prompt.md | Updates system prompt examples to new attribute syntax. |
| docs/caching.md | Updates cache refresh example to name + data-source. |
| docs/archive/ROADMAP.md | Updates archived roadmap references to new attribute names. |
| docs/archive/launch-page.md | Updates archived launch page examples and attribute descriptions. |
| docs/research/examples/llm-generated-apps/README.md | Updates research examples to new form/button routing. |
| docs/research/examples/llm-generated-apps/01-job-tracker/index.md | Migrates research example to form name="add". |
| docs/research/examples/llm-generated-apps/04-feedback-form/PROMPT.md | Updates prompt guidance to reflect name on forms. |
| docs/research/examples/llm-generated-apps/04-feedback-form/index.md | Migrates research example to form name="add". |
| docs/research/examples/llm-generated-apps/05-expense-tracker/index.md | Migrates research example to form name="add". |
| docs/research/examples/llm-generated-apps/06-meeting-notes/index.md | Migrates research example to form name="add". |
| docs/research/examples/llm-generated-apps/07-reading-list/index.md | Migrates research example to form name="add". |
| docs/plans/tinkerdown-unified-design.md | Updates plan examples to new attribute syntax. |
| docs/plans/livemdtools-enhancements.md | Updates plan wording for button action routing. |
| docs/plans/2026-01-03-action-buttons-design.md | Updates plan wording/examples for custom actions routing. |
| docs/plans/2026-01-01-cli-scaffolding-implementation.md | Updates plan examples for new action routing. |
| docs/plans/2025-12-31-productivity-tools-research.md | Updates plan examples to new action routing. |
| docs/plans/2025-12-31-missing-features-deep-dive.md | Updates plan snippets to new name/data-* syntax. |
| docs/plans/2025-12-31-markdown-native-research.md | Updates plan examples to new action routing. |
| docs/plans/2025-12-31-e2e-testing-strategy.md | Updates plan wording for e2e attribute selectors. |
| docs/plans/2025-12-30-runbook-instances-design.md | Updates plan examples for new attribute syntax. |
| docs/plans/2025-12-29-auto-rendering-lists-design.md | Updates plan snippets to new action/data attributes. |
| docs/plans/2025-12-21-exec-args-form-design.md | Updates plan docs from lvt-submit to form name="Run". |
| docs/plans/2025-12-15-web-components-design.md | Updates plan wording for new event handling attributes. |
| docs/plans/2025-12-14-multi-interface-design.md | Updates plan examples and inference rules to name/data-*. |
| docs/plans/2025-11-12-livemdtools-design.md | Updates plan examples and test selector references. |
| docs/plans/2024-12-14-self-contained-markdown-apps-design.md | Updates plan examples to name/data-* and data-confirm. |
| docs/plans/2024-12-14-phase2-validation-design.md | Updates plan docs to new attribute names. |
| component_library_e2e_test.go | Updates generated HTML assertions to name and data-id. |
| examples/two-line-todo/index.md | Migrates checkbox action to lvt-on:click + data-id. |
| examples/team-tasks/app.md | Migrates team-tasks app to new action routing + reset lifecycle syntax. |
| examples/markdown-data-todo/index.md | Migrates markdown-data-todo app to new routing + reset lifecycle syntax. |
| examples/markdown-data-dashboard/index.md | Migrates markdown-data-dashboard form + delete button syntax. |
| examples/markdown-data-bookmarks/index.md | Migrates bookmarks example to new routing/data syntax. |
| examples/lvt-source-wasm-test/index.md | Migrates refresh button to name="Refresh". |
| examples/lvt-source-sqlite-test/index.md | Migrates sqlite test app controls to new syntax. |
| examples/lvt-source-rest-test/index.md | Migrates rest test app refresh button to name="Refresh". |
| examples/lvt-source-pg-test/index.md | Migrates pg test app refresh button to name="Refresh". |
| examples/lvt-source-file-test/index.md | Migrates file source test refresh buttons to name="Refresh". |
| examples/lvt-source-exec-test/index.md | Migrates exec source test refresh button to name="Refresh". |
| examples/expense-tracker/README.md | Updates README to reference name="..." actions. |
| examples/expense-tracker/index.md | Migrates expense tracker app to new routing/data syntax. |
| examples/exec-args-test/index.md | Migrates exec-args form to form name="Run". |
| examples/action-buttons/index.md | Migrates custom action button example to new routing/data syntax. |
| cmd/tinkerdown/commands/templates/basic/index.md | Updates CLI template app to new routing syntax. |
| cmd/tinkerdown/commands/templates/dashboard/index.md | Updates CLI dashboard template to new routing syntax. |
| cmd/tinkerdown/commands/templates/api-explorer/index.md | Updates CLI API explorer template form routing to name. |
| cmd/tinkerdown/commands/templates/form/index.md | Updates CLI form template to new routing/data syntax. |
| cmd/tinkerdown/commands/templates/markdown-notes/index.md | Updates CLI markdown-notes template routing + reset lifecycle attribute. |
| cmd/tinkerdown/commands/templates/todo/index.md | Updates CLI todo template to new routing/data syntax. |
| cmd/tinkerdown/commands/templates/tutorial/index.md | Updates CLI tutorial template routing and lvt-on:change usage. |
| cmd/tinkerdown/commands/templates/wasm-source/test-app/index.md | Updates WASM template refresh button to name="Refresh". |
| cmd/tinkerdown/commands/new_test.go | Updates scaffold test to look for name=" routing. |
| client/src/tinkerdown-client.ts | Updates lifecycle listener comment to new lvt-el:* syntax. |
| client/src/blocks/interactive-block.ts | Implements name/lvt-on:* routing + data-confirm/data-* extraction. |
| auto_tasks.go | Updates auto task block generation to new action routing/lifecycle attrs. |
| auto_tasks_test.go | Updates auto task tests for new attribute strings. |
| auto_tables.go | Updates auto table generation (refresh/edit/delete/add/update/cancel) to new syntax. |
| auto_tables_test.go | Updates auto table tests for new attribute strings and confirm attribute. |
| auto_tables_integration_test.go | Updates integration test comment to reflect new routing. |
| go.mod | Bumps github.com/livetemplate/livetemplate and updates indirect deps. |
| go.sum | Updates module sums consistent with Go module changes. |
| client/package.json | Bumps @livetemplate/client dependency. |
| client/package-lock.json | Locks @livetemplate/client@0.8.19. |
Files not reviewed (1)
- client/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private handleClick(e: Event): void { | ||
| const target = e.target as HTMLElement; | ||
| const action = target.getAttribute("lvt-click"); | ||
|
|
||
| // Standard HTML: button name routing | ||
| if (target instanceof HTMLButtonElement && target.name) { |
There was a problem hiding this comment.
handleClick relies on e.target being the actionable element. With delegated listeners, clicks on nested elements (e.g. <button><span>…</span></button> or children inside an element with lvt-on:click) will set target to the child and the action won’t be detected. Consider resolving the action element via closest('button[name]') and/or closest('[lvt-on\\:click]') before reading attributes.
| // Standard HTML: button name routing | ||
| if (target instanceof HTMLButtonElement && target.name) { | ||
| e.preventDefault(); | ||
|
|
||
| if (!this.checkConfirm(target)) { | ||
| this.log("Click action cancelled by user:", target.name); | ||
| return; | ||
| } | ||
|
|
||
| const data = this.extractData(target); | ||
| this.sendAction(target.name, data); | ||
| this.log("Click action (button name):", target.name, data); | ||
| return; |
There was a problem hiding this comment.
The button[name] branch always calls e.preventDefault() and returns, which will prevent form submission when the clicked button is a submit button. That makes the handleSubmit logic (including “submitter button name > form name” routing and FormData collection) unreachable for named submit buttons. Consider skipping click-handling for button[type="submit"] (or buttons with button.form != null) and letting the submit handler drive form actions.
1. handleSubmit: only intercept forms with explicit routing signal (submitter name or form name) — plain forms now behave natively 2. handleClick: add orphan button guard (target.form === null) to match @livetemplate/client semantics — buttons inside forms are handled by the submit path 3. Fix event listener leak: store bound handlers as class properties and remove them in destroy() 4. Clear pendingForm/pendingAction in destroy() to prevent phantom lifecycle events after disconnect 5. Use for...in on DOMStringMap instead of Object.entries to avoid intermediate tuple array allocation in extractData() Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Code ReviewOverall this is a clean, well-scoped migration. The attribute table in the PR description makes the changes easy to follow, and the Go/template/test coverage is thorough. A few issues worth addressing before merge. |
|
test single line body |
|
|
Part 1/6: Code Review - Event delegation bug. The handleClick handler checks e.target directly. With event delegation, if a button contains a child node (e.g., a span or icon), clicking the child sets target to the child, not the button, and the handler silently does nothing. Fix: use closest("button[name]") and closest("[lvt-on:click]") to walk up to the nearest matching ancestor. |
|
Part 2/6: FormData bug. In handleSubmit, when a named submit button triggers the form, the browser includes that buttons name=value pair in the FormData. The current code collects all entries, so the action name becomes a spurious key. Fix: exclude the submitters own entry when iterating formData (check if submitter instanceof HTMLButtonElement and key === submitter.name, then return). |
|
Retracting test comments - full review posted below. |
TinkerdownBuild data-driven apps with markdown
Tinkerdown is a CLI tool for creating interactive, data-driven applications using markdown files. Connect to databases, APIs, and files with zero boilerplate. Built on LiveTemplate. Why Tinkerdown?Tinkerdown replaces typical app scaffolding with a single markdown file — data sources in YAML, layout in markdown, interactions via HTML attributes.
Quick Start# Install
go install github.com/livetemplate/tinkerdown/cmd/tinkerdown@latest
# Create a new app
tinkerdown new myapp
cd myapp
# Run the app
tinkerdown serve
# Open http://localhost:8080What You Can BuildWrite a markdown file with a YAML source definition and a standard markdown table. Tinkerdown infers that the "Tasks" heading matches the "tasks" source and auto-generates an interactive table with add, edit, and delete: ---
title: Task Manager
sources:
tasks:
type: sqlite
db: ./tasks.db
table: tasks
readonly: false
---
# Task Manager
## Tasks
| Title | Status | Due Date |
|-------|--------|----------|Run
Need more control? Use HTML attributes for explicit binding: <table lvt-source="tasks" lvt-columns="title,status" lvt-datatable lvt-actions="Complete,Delete">
</table>Key Features
Data SourcesDefine sources in your page's frontmatter: ---
sources:
tasks:
type: sqlite
path: ./tasks.db
query: SELECT * FROM tasks
users:
type: rest
from: https://api.example.com/users
config:
type: json
path: ./_data/config.json
---
Auto-RenderingGenerate HTML automatically from data sources: <!-- Table with actions -->
<table lvt-source="tasks" lvt-columns="title,status" lvt-actions="Edit,Delete">
</table>
<!-- Select dropdown -->
<select lvt-source="categories" lvt-value="id" lvt-label="name">
</select>
<!-- List -->
<ul lvt-source="items" lvt-field="name">
</ul>See Auto-Rendering Guide for full details. Interactive Attributes
See lvt-* Attributes Reference for the complete list. ConfigurationRecommended: Configure in frontmatter (single-file apps): ---
title: My App
sources:
tasks:
type: sqlite
path: ./tasks.db
query: SELECT * FROM tasks
styling:
theme: clean
---For complex apps: Use # tinkerdown.yaml - for multi-page apps with shared sources
server:
port: 3000
sources:
shared_data:
type: rest
from: ${API_URL}
cache:
ttl: 5mSee Configuration Reference for when to use each approach. AI-Assisted DevelopmentTinkerdown works great with AI assistants. Describe what you want: See AI Generation Guide for tips on using Claude Code and other AI tools. DocumentationGetting Started: Guides: Reference: Planning: Developmentgit clone https://github.com/livetemplate/tinkerdown.git
cd tinkerdown
go mod download
go test ./...
go build -o tinkerdown ./cmd/tinkerdownLicenseMIT ContributingContributions welcome! See ROADMAP.md for planned features and current priorities. |
|
1 |
|
Sun Apr 5 18:58:31 UTC 2026 |
|
CLAUDE.md |
|
NOTE: Previous test comments above can be disregarded. Full code review follows in the next comment. |
|
FULL CODE REVIEW (posted in sections due to environment constraints): SECTION 1 - OVERALL: Overall this is a clean, well-scoped migration. The attribute table in the PR description makes the changes easy to follow, and the Go/template/test coverage is thorough. A few issues worth addressing before merge. |
|
SECTION 2 - BUG: Event delegation does not walk the DOM tree. The handleClick handler checks e.target directly. With event delegation (capture phase), e.target is the actual clicked element. If a button contains a child node (e.g., button > span), clicking the span sets target to the span, not the button, and the handler silently does nothing. The same problem applies to the lvt-on:click branch. FIX: use closest("button[name]") and closest("[lvt-on:click]") to walk up to the nearest matching ancestor instead of checking e.target directly. |
|
SECTION 3 - BUG: Submitter button name leaks into FormData. In handleSubmit, when a named submit button triggers the form, the browser includes that buttons name=value pair in the FormData. The current code collects all entries and sends them as the data payload, so the action name becomes a spurious key in the data. FIX: when iterating formData, skip the entry where key === submitter.name (when submitter instanceof HTMLButtonElement). |
|
SECTION 4 - MINORS AND NITS: (a) for...in on dataset in extractData can iterate inherited properties - prefer Object.keys(element.dataset) as the idiomatic safe form. (b) data-confirm namespace collision risk: this attribute is used by Rails UJS and others. The old lvt-confirm was unambiguous - worth noting potential conflicts in attribute docs. (c) Nit: handlers registered as (e: Event) but cast inside body - declare method signatures with specific event types (MouseEvent, SubmitEvent) to remove casts and clarify intent. |
|
SECTION 5 - POSITIVE NOTES: Storing bound handlers as class fields and removing them in destroy() is a real improvement - the previous anonymous arrow functions leaked listeners on every re-mount. Inlining checkConfirm and extractData rather than importing from @livetemplate/client is the right call since the attribute names changed. Test coverage across Go unit tests, golden tests, and e2e selectors is consistent and thorough. The lvt-el:reset:on:success comment update in handleMessage keeps code and docs in sync. The event delegation bug is the most impactful item - any button whose label is wrapped in a span or icon element will silently fail after this change. Recommend fixing before merge. |
|
Generated with Claude Code |
1. Use closest() in handleClick to walk up from e.target — fixes child elements inside buttons not being detected (e.g., <button><span>) 2. Add containment check (this.element.contains) to prevent matching ancestors outside the block 3. Skip submitter button's name=value entry in FormData iteration to prevent the action name from leaking as a spurious data key 4. Switch extractData from for...in to Object.keys() to avoid inherited property iteration on DOMStringMap Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Code ReviewOverall this is a clean, well-scoped migration. The attribute reduction is a genuine improvement — using native HTML semantics ( Bug: In <input
type="text"
value="{{.Message}}"
lvt-on:change="update-message"
lvt-change-data='{"message": "$value"}'
/>The Documentation inconsistency: lifecycle attribute syntax
The PR migrates Test quality regression: The updated assertion is too broad — any element with a Minor: The HTML Nit: return !message || window.confirm(message);What works well
🤖 Generated with Claude Code |
The lvt-change-data attribute was a legacy mechanism for passing data with change events. With lvt-on:change, the input value is sent automatically. Use name="message" so the value flows through FormData. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
PR Review — feat!: attribute reduction (Phase 3A+3B)Overall this is a well-scoped migration. The attribute table in the summary is clear, the Go generator and template changes are mechanical and consistent, and the TypeScript refactor includes a real improvement (event handler cleanup in Bugs / Correctness1.
<input type="checkbox" ... lvt-on:click="Toggle" data-id="{{.Id}}">In 2. Stale lifecycle attribute syntax in The "Processed by - - Lifecycle: `lvt-{action}-on:{event}`
+ - Lifecycle: `lvt-el:{method}:on:{state}`This conflicts with the corrected example just above it ( CLAUDE.md concern
Behaviour change worth documenting
Minor nitREADME table is missing The README attribute table now lists Positives
🤖 Generated with Claude Code |
Summary
Attribute Changes
lvt-click="X"on buttonsname="X"lvt-click="X"on non-buttonslvt-on:click="X"lvt-submit="X"name="X"lvt-data-*="V"data-*="V"lvt-confirm="msg"data-confirm="msg"(CSP-safe)lvt-reset-on:successlvt-el:reset:on:successlvt-change="X"lvt-on:change="X"Test plan
tinkerdown serve🤖 Generated with Claude Code