Conversation
… example Replace ~110 lines of custom CSS with Pico CSS framework, keeping only minimal custom styles for: - JS/no-JS mode indicator visibility - Completed todo item styling (strikethrough) - Empty state centering - Todo item flex layout Changes: - Add Pico CSS v2 via CDN - Use semantic HTML elements (<article>, <mark>, <ins>, <del>) - Use Pico's container class for layout - Use Pico's button classes (secondary outline, contrast outline) - Use aria-invalid for form validation errors - Use <small> for helper text and timestamps Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Updates core library to v0.8.2 which includes: - Comprehensive fuzz testing framework with TypeScript oracle - Bug fixes for reorder+update combined operations - Bug fixes for conditional branch changes in range items - Optimized benchmark workflow Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This pull request updates the livetemplate core library from v0.8.1 to v0.8.2, which includes fuzz testing improvements and bug fixes. However, it also includes a significant template refactoring for the progressive-enhancement example that is not mentioned in the PR description.
Changes:
- Updates
github.com/livetemplate/livetemplatedependency from v0.8.1 to v0.8.2 - Refactors
progressive-enhancement.tmplto use Pico CSS framework, removing ~100 lines of custom CSS - Changes HTML structure and semantic elements in the progressive-enhancement template
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| go.mod | Updates livetemplate dependency to v0.8.2 |
| go.sum | Updates checksums for livetemplate v0.8.2 |
| progressive-enhancement/progressive-enhancement.tmpl | Major refactoring: adds Pico CSS framework, restructures HTML, removes custom CSS (not mentioned in PR description) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <ins style="display:block; text-decoration:none;">{{.lvt.Flash "success"}}</ins> | ||
| {{end}} | ||
| {{if .lvt.Flash "error"}} | ||
| <del style="display:block; text-decoration:none;">{{.lvt.Flash "error"}}</del> |
There was a problem hiding this comment.
The use of <ins> and <del> elements for flash messages is semantically incorrect. These HTML elements are meant to represent editorial changes to a document (inserted or deleted text), not success or error messages.
For flash messages:
<ins>semantically means "this text was inserted into the document"<del>semantically means "this text was deleted from the document"
Consider using more appropriate semantic elements:
- For success messages: Use a
<div>with appropriate ARIA attributes likerole="status"orrole="alert" - For error messages: Use a
<div>withrole="alert"andaria-live="assertive"
This would improve accessibility and semantic correctness.
| <ins style="display:block; text-decoration:none;">{{.lvt.Flash "success"}}</ins> | |
| {{end}} | |
| {{if .lvt.Flash "error"}} | |
| <del style="display:block; text-decoration:none;">{{.lvt.Flash "error"}}</del> | |
| <div role="status" aria-live="polite" style="display:block; text-decoration:none;">{{.lvt.Flash "success"}}</div> | |
| {{end}} | |
| {{if .lvt.Flash "error"}} | |
| <div role="alert" aria-live="assertive" style="display:block; text-decoration:none;">{{.lvt.Flash "error"}}</div> |
| <link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/@picocss/pico@2/css/pico.min.css"> | ||
| <style> | ||
| * { box-sizing: border-box; } | ||
| body { | ||
| font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, sans-serif; | ||
| max-width: 600px; | ||
| margin: 40px auto; | ||
| padding: 0 20px; | ||
| background: #f5f5f5; | ||
| } | ||
| h1 { color: #333; margin-bottom: 5px; } | ||
| .subtitle { color: #666; margin-bottom: 20px; font-size: 14px; } | ||
| .mode-indicator { | ||
| background: #e8f4f8; | ||
| border: 1px solid #b8d4e3; | ||
| border-radius: 4px; | ||
| padding: 10px; | ||
| margin-bottom: 20px; | ||
| font-size: 13px; | ||
| } | ||
| .mode-indicator.js { background: #e8f8e8; border-color: #b8e3b8; } | ||
| .flash { | ||
| padding: 12px; | ||
| border-radius: 4px; | ||
| margin-bottom: 15px; | ||
| } | ||
| .flash.success { background: #d4edda; color: #155724; border: 1px solid #c3e6cb; } | ||
| .flash.error { background: #f8d7da; color: #721c24; border: 1px solid #f5c6cb; } | ||
| .add-form { | ||
| background: white; | ||
| padding: 20px; | ||
| border-radius: 8px; | ||
| box-shadow: 0 1px 3px rgba(0,0,0,0.1); | ||
| margin-bottom: 20px; | ||
| } | ||
| .add-form input[type="text"] { | ||
| width: 100%; | ||
| padding: 10px; | ||
| border: 1px solid #ddd; | ||
| border-radius: 4px; | ||
| font-size: 16px; | ||
| margin-bottom: 10px; | ||
| } | ||
| .add-form input[type="text"]:focus { | ||
| border-color: #007bff; | ||
| outline: none; | ||
| } | ||
| .add-form input.error { | ||
| border-color: #dc3545; | ||
| } | ||
| .add-form button { | ||
| background: #007bff; | ||
| color: white; | ||
| border: none; | ||
| padding: 10px 20px; | ||
| border-radius: 4px; | ||
| cursor: pointer; | ||
| font-size: 14px; | ||
| } | ||
| .add-form button:hover { background: #0056b3; } | ||
| .error-message { | ||
| color: #dc3545; | ||
| font-size: 13px; | ||
| margin-top: -5px; | ||
| margin-bottom: 10px; | ||
| } | ||
| .todo-list { | ||
| background: white; | ||
| border-radius: 8px; | ||
| box-shadow: 0 1px 3px rgba(0,0,0,0.1); | ||
| overflow: hidden; | ||
| } | ||
| .todo-item { | ||
| display: flex; | ||
| align-items: center; | ||
| padding: 15px; | ||
| border-bottom: 1px solid #eee; | ||
| } | ||
| .todo-item:last-child { border-bottom: none; } | ||
| .todo-item.completed .todo-title { | ||
| text-decoration: line-through; | ||
| color: #999; | ||
| } | ||
| .todo-title { | ||
| flex: 1; | ||
| margin-left: 10px; | ||
| } | ||
| .todo-time { | ||
| color: #999; | ||
| font-size: 12px; | ||
| margin-right: 10px; | ||
| } | ||
| .todo-actions { display: flex; gap: 5px; } | ||
| .todo-actions form { margin: 0; } | ||
| .btn-toggle, .btn-delete { | ||
| padding: 5px 10px; | ||
| border: none; | ||
| border-radius: 4px; | ||
| cursor: pointer; | ||
| font-size: 12px; | ||
| } | ||
| .btn-toggle { background: #28a745; color: white; } | ||
| .btn-toggle:hover { background: #1e7e34; } | ||
| .btn-delete { background: #dc3545; color: white; } | ||
| .btn-delete:hover { background: #bd2130; } | ||
| .empty-state { | ||
| text-align: center; | ||
| padding: 40px; | ||
| color: #999; | ||
| } | ||
| noscript .mode-indicator { display: block !important; } | ||
| /* JS/no-JS mode indicator visibility */ | ||
| .js-mode { display: none; } | ||
| .no-js-mode { display: block; } | ||
|
|
||
| /* Completed todo item */ | ||
| .completed { text-decoration: line-through; opacity: 0.6; } | ||
|
|
||
| /* Empty state centering */ | ||
| .empty-state { text-align: center; } | ||
|
|
||
| /* Todo item layout */ | ||
| .todo-item { display: flex; align-items: center; gap: var(--pico-spacing); } | ||
| .todo-item form { margin: 0; } | ||
| .todo-title { flex: 1; } | ||
| </style> |
There was a problem hiding this comment.
There is a significant discrepancy between the PR description and the code changes. The PR title and description state this is a simple library update to v0.8.2 with bug fixes and fuzz testing improvements. However, the progressive-enhancement.tmpl file contains a major UI refactoring that:
- Adds the Pico CSS framework as a new external dependency
- Removes all custom CSS styling (approximately 100 lines)
- Changes HTML structure and class names throughout the template
- Modifies flash message rendering to use
<ins>and<del>elements
This template refactoring appears to be unrelated to the v0.8.2 library update mentioned in the PR description. These are substantial changes that should either:
- Be documented in the PR description if they're intentional additional changes
- Be split into a separate PR if they're not directly related to the library update
This makes it difficult to review the PR as a "library update" when it includes significant unrelated changes.
| <mark> | ||
| <strong>No JavaScript Mode:</strong> Using traditional HTTP form submissions with page reloads. | ||
| Each action reloads the page to show updates. | ||
| </mark> | ||
| </noscript> | ||
| <mark class="js-mode" id="mode-indicator"> | ||
| <strong>JavaScript Mode:</strong> Using WebSocket for instant updates without page reloads. | ||
| Actions update the page in real-time. | ||
| </mark> |
There was a problem hiding this comment.
The use of <mark> elements for mode indicators (lines 31 and 36) is semantically questionable. The <mark> element in HTML is intended to highlight text for reference purposes (like search results), not for informational notices or status indicators.
While Pico CSS may style <mark> as an informational element, using a more semantically appropriate element would improve accessibility and standards compliance. Consider using:
<div role="status">for informational status messages<aside>for supplementary content- Or keeping the previous
<div>approach with appropriate CSS classes
This is especially important for screen readers, which may announce marked text differently than status information.
Summary
Updates core library to v0.8.2.
Changes in v0.8.2
Related PRs
🤖 Generated with Claude Code