Skip to content

Keep form buttons' height and centre their spinner while they wait - #643

Merged
richshaw merged 3 commits into
mainfrom
claude/issue-636-async-button-centre
Oct 25, 2026
Merged

richshaw merged 3 commits into
mainfrom
claude/issue-636-async-button-centre

Conversation

@richshaw

Copy link
Copy Markdown
Owner

Fixes #636.

The problem

While a form button waits for its answer, it shows a spinner and a word like "Saving…". In AsyncSubmitButton they sat in an inline-flex span whose first item, the spinner, is an empty inline-block, so the span took its baseline from the spinner's bottom edge. The span then sat on the text's baseline with all 16px above it: the spinner rode high, and buttons that aren't flex boxes themselves grew taller while waiting. Save on a signup's Settings tab went from 36px to 39px, then snapped back.

The fix

  • src/components/ui/async-submit-button.tsx: the wrapper is now flex items-center justify-center gap-2 instead of inline-flex items-center gap-2, as the issue outlined. justify-center keeps the contents centred in buttons wider than their text, such as the consent screen's full-width ones.
  • Buttons that are flex boxes already (Publish and Close signup in the signup header, and in the phone's More actions sheet) are unchanged. There the wrapper is a flex item sized to its content, so justify-center does nothing, and the sheet's buttons stay left-aligned.
  • A CHANGELOG line under Unreleased › Fixed.

Tests

  • New tests/e2e/async-submit-button.spec.ts: at 1280px and 390px it holds the Settings tab's server actions open, then checks that Save ("Saving…") and "Yes, delete signup" ("Deleting…") keep their height and that the spinner's middle is within 0.5px of the button's.
  • Before the fix the spec failed at both widths (36px → 39px). With the fix it passes.
  • Ran and passed in this session: pnpm lint && pnpm typecheck && pnpm knip && pnpm test (1,389 tests), pnpm build && pnpm budgets, and the new spec plus responses, organizer and oauth-consent against the production build (22 passed).
  • affected-articles.mjs names three help pages that show these buttons. Their wording, steps and resting appearance are the same, so none needed editing.

Session: https://claude.ai/code/session_01Ac17ipj2bhcShNz3whFhDc


Generated by Claude Code

AsyncSubmitButton wrapped its spinner and word in an inline-flex span.
The spinner, an empty inline-block, gave that span its baseline, so the
span sat on the text's baseline with all of its height above it: the
spinner rode high and buttons that aren't flex boxes grew taller while
waiting (Save on Settings went from 36 to 39px). A block-level flex box
centres the contents and keeps the button's height. justify-center keeps
them centred in buttons wider than their text.

A new e2e spec holds the server action open and measures Save and
"Yes, delete signup" on the Settings tab at 1280 and 390px.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ac17ipj2bhcShNz3whFhDc
@richshaw richshaw added bug Something isn't working pipeline:ux Changes what a person using OpenSignup sees or does pipeline:working A session is working on it right now labels Oct 25, 2026 — with Claude
@richshaw
richshaw requested a balanced review from Copilot October 25, 2026 18:01

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

@richshaw richshaw added pipeline:tophat pipeline:working A session is working on it right now and removed pipeline:working A session is working on it right now labels Oct 25, 2026 — with Claude

Copy link
Copy Markdown
Owner Author

Tophat: passed

Ran 265de49 locally with pnpm dev, Postgres 16, Chromium at 1280×900 and 390×844 (phone with touch on). I used made-up signups ("Snack duty — Spring season …", one slot). To keep each button in its waiting state, I held its server-action request open. Then I measured the button's height at rest and while waiting, and how far the spinner's middle sat from the button's middle (negative means above). For the "before" rows, only async-submit-button.tsx was swapped back to the base (ee0f0d5), since it's the only runtime file the PR changes.

Check Result What I saw
Bug on base, Settings › Save (both widths) ✅ reproduced 36px → 39px while "Saving…", spinner 1.5px high
Bug on base, Yes, delete signup / Create signup ✅ reproduced 38 → 39px and 40 → 42px, spinner 1.5px / 1px high
Fix: Save, Yes, delete signup, Create signup, at 1280 and 390 ✅ Height unchanged (36 / 38 / 40px), spinner exactly centred (0px), contents still start at the same left padding
Neighbours: header Publish and Close signup (desktop) ✅ Same on base and PR: 28px, centred, same widths and positions
Neighbours: phone More actions sheet, Publish signup and Close signup ✅ Same on base and PR: 50.5px, contents stay left-aligned (not pulled to the centre)
Actions still complete when not held (both widths) ✅ Create signup → Build, Publish → status open, Save returns to "Save" at 36px, Delete → back to /app and the signup is a 404
Browser console and server log ✅ No console errors. The server's [Error: aborted] lines are the requests I held and then dropped on purpose

Not clicked: the OAuth consent screen's full-width buttons. The PR's run of oauth-consent.spec.ts covers that page, and justify-center is there to keep their contents centred.

For the owner (UX): while a form button waits (Save, Yes, delete signup, Create signup, Remove, Disconnect), it no longer grows 1–3px taller and then snaps back, and its spinner sits in the middle instead of slightly high. At rest, every button looks exactly as it did. Buttons in the signup header and the phone's More actions sheet don't change. Screenshots: 643-before-desktop-save / 643-after-desktop-save, 643-before-phone-delete / 643-after-phone-delete, 643-before-desktop-create / 643-after-desktop-create, plus 643-after-desktop-header-publish and 643-after-phone-sheet-close for the unchanged ones.

Screenshots are in https://claude.ai/code/session_01WTzR5GhEz2dQbgzgZTCJhi.


Generated by Claude Code

@richshaw richshaw added pipeline:review pipeline:fix pipeline:working A session is working on it right now and removed pipeline:tophat pipeline:working A session is working on it right now pipeline:review labels Oct 25, 2026 — with Claude
Keeps both Unreleased › Fixed CHANGELOG lines (#586 from main, #636 here).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R9pje6Yt5WfLsBarh5RpiK
@richshaw richshaw added pipeline:review pipeline:working A session is working on it right now and removed pipeline:fix pipeline:working A session is working on it right now labels Oct 25, 2026 — with Claude

@richshaw richshaw left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Review: passed (round 1)

This PR turns the wrapper inside AsyncSubmitButton from inline-flex items-center into flex items-center justify-center, as the issue outlined. The wrapper no longer gets its baseline from the empty spinner, so a waiting button keeps its height and its spinner sits in the middle. I checked every caller (Settings Save and "Yes, delete signup", "Yes, remove" on Responses, Create signup, Close signup in the ⋯ panel and the phone sheet, Disconnect, and Allow / Don't allow). None of their buttons is a flex box or uses text-left, so their contents were already centred by the button's default text-align: center. justify-center therefore keeps the resting look the same, including on the full-width consent and phone-sheet buttons. The new e2e spec measures height and spinner position at both widths, and the PR says it failed before the fix.

Blocking (0): none
Optional (1):

  • CHANGELOG.md:77: the entry lists "Publish" among the buttons this fixes, but PublishButton doesn't use AsyncSubmitButton. Its "Checking…" already sits in a block-level grid, as the issue says. Dropping "Publish and" would make the line accurate, but it isn't worth a round of its own.

Earlier threads: none (Copilot couldn't review: quota used up)
Checks: CI green on d9308bd (lint-type-test, e2e, CodeQL). The only commit since the passing tophat (265de49) is a merge from main; the shared runtime-change check prints 0. No local runs were needed: there were no findings to prove.

Why the owner: UX. Every waiting form button changes slightly: it no longer grows 1–3px or lifts its spinner. Look at the tophat's before/after screenshots (643-before/after-desktop-save, -phone-delete, -desktop-create) and the unchanged header Publish and phone-sheet Close.

https://claude.ai/code/session_01XikuxTBxb75uCEbycXLzmj


Generated by Claude Code

@richshaw richshaw added pipeline:owner Passed; waits for the owner (UX or high-risk) and removed pipeline:review pipeline:working A session is working on it right now labels Oct 25, 2026 — with Claude
@richshaw richshaw added pipeline:merge and removed pipeline:owner Passed; waits for the owner (UX or high-risk) labels Oct 25, 2026
@richshaw richshaw added the pipeline:working A session is working on it right now label Oct 25, 2026 — with Claude

Copy link
Copy Markdown
Owner Author

Approved by the owner at d9308bd.

Session: https://claude.ai/code/session_01MTLHRSYbwK1vKjngmLNWUb


Generated by Claude Code

@richshaw richshaw removed the pipeline:working A session is working on it right now label Oct 25, 2026 — with Claude

Copy link
Copy Markdown
Owner Author

Not merged: the branch is behind main and GitHub refuses to update it because of a merge conflict (#600 just landed, so CHANGELOG is the likely spot). Every other gate passed on d9308bd: CI, the review, the tophat, no open threads, and the owner's approval. Sent to pipeline:fix to merge main in. The owner's approval carries over if the fix adds nothing but that merge.

Session: https://claude.ai/code/session_016yEFqND3Wf8TSeER8F44Ny


Generated by Claude Code

@richshaw richshaw added the pipeline:working A session is working on it right now label Oct 25, 2026 — with Claude

Copy link
Copy Markdown
Owner Author

Merged main in at 8ff8d9c. The only conflict was CHANGELOG.md, where #600's Fixed entry landed beside this PR's, so I kept both. main's change to async-submit-button.tsx (a ref prop) merged cleanly next to this PR's one-line wrapper change. I resolved no runtime code by hand, and the PR has no commits of its own since d9308bd, so the review, the tophat and the owner's approval all still apply.

Ran on the merge and all passed: pnpm lint && pnpm typecheck && pnpm knip && pnpm test (1,764 tests). It goes back to pipeline:merge once CI is green.

Session: https://claude.ai/code/session_016yEFqND3Wf8TSeER8F44Ny


Generated by Claude Code

@richshaw richshaw added pipeline:merge pipeline:working A session is working on it right now and removed pipeline:fix pipeline:working A session is working on it right now labels Oct 25, 2026 — with Claude

Copy link
Copy Markdown
Owner Author

Approved by the owner at 8ff8d9c. Carried over from the approval at d9308bd: the PR has no commits of its own since then, only a merge from main.

Session: https://claude.ai/code/session_01DFKGuYh28iFmKJaws6mXzT


Generated by Claude Code

@richshaw
richshaw merged commit 043f972 into main Oct 25, 2026
5 checks passed
@richshaw
richshaw deleted the claude/issue-636-async-button-centre branch October 25, 2026 23:24
@richshaw richshaw removed pipeline:merge pipeline:ux Changes what a person using OpenSignup sees or does pipeline:working A session is working on it right now labels Oct 25, 2026 — with Claude
richshaw pushed a commit that referenced this pull request Oct 26, 2026
Through the server, telemetry and screen branches.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UW9ZpEu4MWkeqTv2Qe14ks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Form buttons grow and their spinner sits high while they wait

3 participants