Skip to content

feat: migrate all examples to Pico CSS#38

Closed
adnaan wants to merge 5 commits intomainfrom
feat/pico-css-migration
Closed

feat: migrate all examples to Pico CSS#38
adnaan wants to merge 5 commits intomainfrom
feat/pico-css-migration

Conversation

@adnaan
Copy link
Copy Markdown
Contributor

@adnaan adnaan commented Mar 28, 2026

Summary

  • Replace all custom CSS with Pico CSS semantic markup across all 16 examples
  • Remove ~660 lines of custom CSS (gradients, buttons, forms, layouts)
  • Add Pico CSS CDN link to all templates
  • Use semantic HTML: <article> for cards, <ins>/<del> for status messages, <mark> for badges, <progress> for progress bars
  • Use aria-invalid + <small> for form validation (Pico auto-colors error states)
  • Only functional CSS remains: .visually-hidden (accessibility), .js-mode (JS toggle), chat message styling (uses --pico-* variables)
  • Update CLAUDE.md with Pico CSS guidelines for future examples
  • Update test selectors for new semantic HTML structure

Test plan

  • ./test-all.sh passes all 11 examples
  • No decorative <style> blocks remain (only functional: visually-hidden, js-mode, chat messages)
  • All templates include Pico CSS CDN link
  • go build ./... passes

🤖 Generated with Claude Code

Replace all custom CSS with Pico CSS semantic markup across all 16
examples. Templates now use Pico's classless styling (article, hgroup,
mark, ins/del, progress, fieldset role=group, etc.) instead of custom
style blocks.

- Remove ~660 lines of custom CSS (gradients, buttons, forms, layouts)
- Add Pico CSS CDN link to all templates
- Use <ins>/<del> for flash/status messages (Pico styles natively)
- Use <article> for cards, <mark> for badges, <progress> for bars
- Use aria-invalid + <small> for form validation (Pico auto-colors)
- Use .grid, .secondary, .contrast, .outline Pico classes
- Only functional CSS remains: .visually-hidden, .js-mode, chat messages
- Update CLAUDE.md with Pico CSS guidelines
- Update test selectors for new semantic HTML structure

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 28, 2026 03:57
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Migrates the example templates to Pico CSS semantic markup, removing most custom styling while keeping behavior intact (including updated E2E/test selectors and authoring guidance).

Changes:

  • Replace custom CSS-heavy layouts with Pico CSS CDN + semantic HTML across examples.
  • Update templates to use Pico-friendly elements (<main class="container">, <article>, <table>, <ins>/<del>, <small>, etc.).
  • Adjust Go tests/E2E selectors to match the new DOM structure and update authoring guidance in CLAUDE.md.

Reviewed changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
todos-progressive/todos_progressive_test.go Updates E2E selectors from <ul>/<li> to table-based markup.
todos-progressive/todos.tmpl Replaces custom CSS + list layout with Pico CSS + table/fieldset markup.
todos-components/todos-components.tmpl Removes remaining minimal custom CSS and shifts todo list to a table layout.
testing/01_basic/welcome.tmpl Adds Pico CSS and wraps content in a container.
progressive-enhancement/progressive_enhancement_test.go Updates selectors to match new table + <s> completion markup.
progressive-enhancement/progressive-enhancement.tmpl Removes custom todo styling and moves list to Pico-styled table semantics.
profile-progressive/profile_progressive_test.go Updates selectors for new success/preview markup.
profile-progressive/profile.tmpl Replaces custom CSS with Pico CSS + semantic status/validation markup.
production/single-host/app.tmpl Removes large custom CSS and reworks layout using Pico components.
observability/counter.tmpl Adds Pico CSS and uses container/article/grid structure.
login/templates/auth.html Removes custom styling and reworks login/dashboard using Pico semantics.
live-preview/preview.tmpl Adds Pico CSS, wraps content in container/article, uses <blockquote> for preview.
graceful-shutdown/counter.tmpl Adds Pico CSS and uses container/article/grid structure.
flash-messages/flash_test.go Updates assertions to match new flash markup/attributes and error rendering.
flash-messages/flash.tmpl Migrates flash demo markup to Pico and simplifies layout to tables/ins/del/small.
counter/counter.tmpl Adds Pico CSS and restructures counter layout using container/article/grid.
chat/chat_e2e_test.go Updates E2E selectors to match new chat header markup.
chat/chat.tmpl Removes .stats block and moves status line to <small>; keeps chat-specific CSS.
avatar-upload/avatar-upload.tmpl Removes extensive custom CSS and reworks UI using Pico primitives + <progress>.
CLAUDE.md Adds guidance for using Pico CSS consistently in future examples.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 123 to 135
@@ -131,7 +131,7 @@ func TestChatE2E(t *testing.T) {
waitFor(`document.querySelector('.messages') !== null`, 5*time.Second),

// Capture after-join state
chromedp.Text(".stats", &afterStatsText, chromedp.ByQuery),
chromedp.Text("small", &afterStatsText, chromedp.ByQuery),
chromedp.Evaluate(`document.querySelector('.messages') !== null`, &afterChatVisible),
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

chromedp.Text("small", ...) is too broad here because the page can contain multiple <small> elements (e.g., the empty-state message also uses <small>). This can make the test read the wrong text and become flaky. Please target the specific stats element (add a stable selector like an id/data-testid, or use a more specific query such as a direct child of the header/container).

Copilot uses AI. Check for mistakes.
chat/chat.tmpl Outdated
margin-bottom: 1rem;
opacity: 0.8;
}
/* Stats bar uses Pico's <small> but needs spacing */
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The inline CSS comment says the stats bar "needs spacing", but no rule is applied to the <small> element anymore (the old .stats rule was removed). Either add the minimal spacing rule for the new markup or remove/update the comment so it matches the actual styling.

Suggested change
/* Stats bar uses Pico's <small> but needs spacing */
/* Stats bar uses Pico's <small> but needs spacing */
main > small {
display: block;
margin-bottom: 0.75rem;
}

Copilot uses AI. Check for mistakes.
Comment on lines 44 to 47
<div>
<button name="clearItems" class="secondary">Clear All</button>
<button name="simulateError" class="contrast">Simulate Error</button>
</div>
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "Clear All" / "Simulate Error" controls are plain <button> elements not associated with any <form>. In standard HTML this won't submit anything, so ClearItems() / SimulateError() won't be triggered without custom JS. Wrap each button in a method="POST" form (or use the form attribute pointing at a form) so the actions work with native form submission like the other buttons on the page.

Suggested change
<div>
<button name="clearItems" class="secondary">Clear All</button>
<button name="simulateError" class="contrast">Simulate Error</button>
</div>
<form method="POST" name="clearItems">
<button type="submit" name="clearItems" class="secondary">Clear All</button>
</form>
<form method="POST" name="simulateError">
<button type="submit" name="simulateError" class="contrast">Simulate Error</button>
</form>

Copilot uses AI. Check for mistakes.
err := chromedp.Run(ctx,
chromedp.Evaluate(fmt.Sprintf(`window.liveTemplateClient.send({action: 'toggle', data: {id: '%s'}})`, itemID), nil),
e2etest.WaitFor(`!document.querySelector('ul li:nth-child(1) span').classList.contains('done')`, 5*time.Second),
e2etest.WaitFor(`!document.querySelector('tbody tr:nth-child(1) s') !== null`, 5*time.Second),
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The wait condition in this JS expression is incorrect: !document.querySelector(...) !== null will always evaluate to true (because !<object> is a boolean, and boolean !== null is always true). This can make the test pass without actually waiting for the todo to be un-done. Use an explicit null check instead (e.g., wait for the <s> element to be absent).

Suggested change
e2etest.WaitFor(`!document.querySelector('tbody tr:nth-child(1) s') !== null`, 5*time.Second),
e2etest.WaitFor(`document.querySelector('tbody tr:nth-child(1) s') === null`, 5*time.Second),

Copilot uses AI. Check for mistakes.
adnaan and others added 4 commits March 28, 2026 09:35
- Fix logic bug in todos-progressive ToggleUndo test (operator
  precedence: !querySelector !== null was always true)
- Remove dead CSS comments in chat template
- Remove dead <form> wrapping display-only text in todos-progressive
- Replace nested <article> with <hgroup> in production template
- Fix comment to explain WHY not WHAT in progressive-enhancement
- Normalize ins/del inline style format in ws-disabled

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Standalone <button name="action"> elements outside forms don't trigger
WebSocket actions — the client library only intercepts form submissions.
Wrap all standalone buttons in <form method="POST" name="action"> so
clicks are properly routed.

Affected: counter (3 variants), production/single-host, flash-messages,
todos-components, todos (pagination + clear completed).

Also add Button_Click_Increment E2E test to counter that verifies
actual button clicks update the UI (previous test only verified
WebSocket JSON messages, not real user interaction).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The client library v0.8.7 supports orphan button detection — standalone
<button name="action"> outside forms triggers the named action directly.
The previous commit unnecessarily wrapped buttons in forms.

Revert all form wrappers back to standalone buttons. Keep the new
Button_Click_Increment E2E test which verifies this works.

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

E2E tests that bypass the UI via liveTemplateClient.send() are false
positives — they pass even when buttons aren't wired up. Convert all
remaining raw WebSocket tests to real browser interactions:

- todos-progressive: all CRUD + filter tests now click actual buttons
- live-preview: add Live_Input_Updates_Preview test (types in input,
  verifies Change() method updates preview in real-time)
- profile-progressive: UpdateProfile now fills form and clicks submit

Also fix todos-progressive filter forms: use form name="filter" instead
of button name="filter" to avoid name collision with hidden input
(client strips form data fields matching the submitter button name).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@adnaan adnaan closed this in cb7bd2c Mar 30, 2026
adnaan added a commit that referenced this pull request Mar 30, 2026
The Pico CSS migration from PR #38 was never merged to main. Apply it
now to the 6 templates that were still using custom CSS:

- counter: bare HTML → Pico article, grid, secondary/contrast buttons
- profile-progressive: custom CSS → Pico aria-invalid, small, ins, article
- flash-messages: 144 lines custom CSS → Pico ins/del, blockquote, table
- login: 119 lines custom CSS → Pico article, label wrapping, contrast
  (preserved lvt-no-intercept on forms for cookie/redirect support)
- avatar-upload: 224 lines custom CSS → Pico progress, article, mark
- todos-progressive: 25 lines custom CSS → Pico table, fieldset, hgroup
  (preserved form name="filter" + active filter highlighting)

Also add CSS section to CLAUDE.md with Pico CSS guidelines.
Update test selectors to match new semantic HTML structure.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants