crueber/walhub
Watch
1
0
Fork
You've already forked walhub
 0
Code Issues 2 Pull requests Projects Releases Packages Wiki Activity

Styled Upload profile image button (fixes #619) #620

Merged
crueber merged 2 commits from fix/issue-619 into main 2026-09-16 11:44:42 +00:00
Conversation 1 Commits 2 Files changed 4 +145 -4
crueber commented 2026-09-16 11:40:26 +00:00
Owner
Copy link

Replaces the raw Choose File input in the isSelf-gated profile sidebar stack (web/src/pages/Repos.jsx) with the hidden-input pattern: full-width label, hidden input (same accept + onChange upload+reset), visible span.btn w-full justify-center reading Upload profile image, muted centered helper below. Placement above Regenerate unchanged; Org/Release/import inputs out of scope.

Tests: new web/test/unit/avatar-upload-button-619.test.js + #619-scoped update to user-avatar-upload-601.test.js; full-minus-smoke node --test green (1597 pass); vite build + esbuild SDK green; go vet clean. Docs: FIXED amendment in docs/go/12_web_ui.md.

Replaces the raw Choose File input in the isSelf-gated profile sidebar stack (web/src/pages/Repos.jsx) with the hidden-input pattern: full-width label, hidden input (same accept + onChange upload+reset), visible span.btn w-full justify-center reading Upload profile image, muted centered helper below. Placement above Regenerate unchanged; Org/Release/import inputs out of scope. Tests: new web/test/unit/avatar-upload-button-619.test.js + #619-scoped update to user-avatar-upload-601.test.js; full-minus-smoke node --test green (1597 pass); vite build + esbuild SDK green; go vet clean. Docs: FIXED amendment in docs/go/12_web_ui.md.
Replace the raw file input in the isSelf-gated profile action stack
with the hidden-input pattern: label wrapper, hidden input (same
accept + onChange upload+reset), visible span.btn w-full
justify-center, muted centered helper below. Placement above
Regenerate unchanged; Org/Release/import inputs out of scope.

Tests: new web/test/unit/avatar-upload-button-619.test.js; #619-scoped
update to user-avatar-upload-601.test.js. Docs: FIXED amendment in
docs/go/12_web_ui.md.
Review of PR #620 found one substantive defect: class="hidden"
(display:none) drops the file input from the Tab order, regressing
keyboard access vs the visible pre-fix input, with no focus passthrough
on the label/span. The input is now peer sr-only (visually hidden but
Tab-reachable) and the button carries the #533 ToggleSwitch
peer-focus-visible emerald ring (both themes), so keyboard focus lands
visibly on the button. Headless pins updated (no display:none, sr-only +
ring pins); docs amendment (law 12) updated in the same change. No new
deps (law 1); Tailwind-only; accept/onChange/gate/placement untouched.
crueber commented 2026-09-16 11:44:16 +00:00
Author
Owner
Copy link

Independent review of PR #620 (fix/issue-619) vs Forgejo #619 — verdict: APPROVE, with one substantive fix landed on the branch (a85e006, pushed).

Acceptance (all verified against origin/main):

  • Styled full-width .btn matching siblings, no visible Choose File: PASS. Span carries btn w-full justify-center px-3 py-1 + cursor-pointer (correct on a span, which has no pointer cursor by default); raw w-full text-xs treatment gone.
  • Picker functional + value reset: PASS. Label wraps input+span (no for/id to mismatch, no pointer-events:none anywhere on the path), so span clicks activate the input natively; onChange (uploadAvatar + e.currentTarget.value = "") and accept="image/png,image/jpeg,image/gif" verified byte-identical vs origin/main.
  • isSelf gate + placement above Regenerate unchanged: PASS.
  • Helper copy centered muted: PASS — muted mt-1 block text-center text-xs below the button. Copy cross-checked vs internal/identity/avatar.go: PNG/JPEG/GIF allowlist (sniffUserAvatarUpload), 2 MiB cap (maxUserAvatarUploadBytes = 2<<20), server center-crop square. Accurate.
  • Tailwind-only, no new deps: PASS (pinned by test over package.json + ui.css).
  • role="button" on the span: redundant inside a label (activation is native; the span is not itself focusable) but harmless and verbatim from the issue's proposed shape — kept, non-blocking.
  • Sibling raw inputs untouched: PASS. Org.jsx (whose accept deliberately includes webp — the org twin), Release.jsx, and Repos import pinned/verified; Repos.jsx contains no other file input.
  • 601-pin re-anchor justified: PASS — the old anchor text no longer exists; same assertions, new anchor.
  • Law 12 amendment present: PASS (and extended for the fix below).
  • Tests fail pre-fix: PASS — new suite run against origin/main fails 6/7 (only the out-of-scope pin passes).

Substantive defect found and fixed (do not waive): cb10a2f used class="hidden" (= display:none), which drops the input from the Tab order with no focus-within/peer passthrough — a keyboard-access regression vs the previously visible, Tab-reachable input. Fixed in a85e006: input is now "peer sr-only" (visually hidden but Tab-reachable, Enter opens the picker) and the button carries the #533 ToggleSwitch peer-focus-visible emerald ring (both themes incl. dark ring-offset), so keyboard focus lands visibly on the button. Test pins updated (asserts never display:none + ring pins); docs amendment updated in the same change. Compiled-CSS check confirms the ring/sr-only utilities are emitted; vite build + esbuild green; unit suite 15/15 on the touched files, full run 1599/1600 with the single failure being smoke.test.js hitting the stale Sep-15 walhub serve process on :8080 (403 /setup) — environmental, excluded per the minus-smoke convention.

One follow-up candidate, deliberately out of scope here: Release.jsx's asset upload uses the same class="hidden"-in-label pattern with no focus passthrough. Suggest a separate issue if a row-wide keyboard standard is wanted.

Independent review of PR #620 (fix/issue-619) vs Forgejo #619 — verdict: APPROVE, with one substantive fix landed on the branch (a85e006, pushed). Acceptance (all verified against origin/main): - Styled full-width .btn matching siblings, no visible Choose File: PASS. Span carries btn w-full justify-center px-3 py-1 + cursor-pointer (correct on a span, which has no pointer cursor by default); raw w-full text-xs treatment gone. - Picker functional + value reset: PASS. Label wraps input+span (no for/id to mismatch, no pointer-events:none anywhere on the path), so span clicks activate the input natively; onChange (uploadAvatar + e.currentTarget.value = "") and accept="image/png,image/jpeg,image/gif" verified byte-identical vs origin/main. - isSelf gate + placement above Regenerate unchanged: PASS. - Helper copy centered muted: PASS — muted mt-1 block text-center text-xs below the button. Copy cross-checked vs internal/identity/avatar.go: PNG/JPEG/GIF allowlist (sniffUserAvatarUpload), 2 MiB cap (maxUserAvatarUploadBytes = 2<<20), server center-crop square. Accurate. - Tailwind-only, no new deps: PASS (pinned by test over package.json + ui.css). - role="button" on the span: redundant inside a label (activation is native; the span is not itself focusable) but harmless and verbatim from the issue's proposed shape — kept, non-blocking. - Sibling raw inputs untouched: PASS. Org.jsx (whose accept deliberately includes webp — the org twin), Release.jsx, and Repos import pinned/verified; Repos.jsx contains no other file input. - 601-pin re-anchor justified: PASS — the old anchor text no longer exists; same assertions, new anchor. - Law 12 amendment present: PASS (and extended for the fix below). - Tests fail pre-fix: PASS — new suite run against origin/main fails 6/7 (only the out-of-scope pin passes). Substantive defect found and fixed (do not waive): cb10a2f used class="hidden" (= display:none), which drops the input from the Tab order with no focus-within/peer passthrough — a keyboard-access regression vs the previously visible, Tab-reachable input. Fixed in a85e006: input is now "peer sr-only" (visually hidden but Tab-reachable, Enter opens the picker) and the button carries the #533 ToggleSwitch peer-focus-visible emerald ring (both themes incl. dark ring-offset), so keyboard focus lands visibly on the button. Test pins updated (asserts never display:none + ring pins); docs amendment updated in the same change. Compiled-CSS check confirms the ring/sr-only utilities are emitted; vite build + esbuild green; unit suite 15/15 on the touched files, full run 1599/1600 with the single failure being smoke.test.js hitting the stale Sep-15 walhub serve process on :8080 (403 /setup) — environmental, excluded per the minus-smoke convention. One follow-up candidate, deliberately out of scope here: Release.jsx's asset upload uses the same class="hidden"-in-label pattern with no focus passthrough. Suggest a separate issue if a row-wide keyboard standard is wanted.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
actions

   
bug
Something is not working

  
cli

   
duplicate
This issue or pull request already exists

  
enhancement
New feature

  
fork

   
forum

   
git storage

   
help wanted
Need some help

  
insights

   
invalid
Something is wrong

  
issues

   
moderation

   
oidc

   
ownership transfer

   
packages

   
pr/merge protection rules

   
projects

   
pull requests

   
question
More information is needed

  
releases

   
sponsorships

   
tags

   
webhooks

   
wiki

   
wontfix
This won't be fixed

No labels
actions
bug
cli
duplicate
enhancement
fork
forum
git storage
help wanted
insights
invalid
issues
moderation
oidc
ownership transfer
packages
pr/merge protection rules
projects
pull requests
question
releases
sponsorships
tags
webhooks
wiki
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
crueber/walhub!620
No description provided.