Fix theme-toggle icon flash, add avatar-stack/empty-state a11y labels, add Button loading prop - #322
Merged
chonilius merged 4 commits intoAug 25, 2026
Conversation
ThemeProvider initialized theme state unconditionally to "light", then corrected it a render later inside a useEffect by reading back the class themeInitScript had already applied to <html> before hydration. Between first paint and that effect running, theme was "light" regardless of the visitor's actual preference — ThemeToggle renders its icon directly from this state, so a visitor whose page was actually dark saw a Moon icon (meaning "click to go dark") for one render even though the page was already dark: the icon momentarily backwards relative to the real state, a visible flash on every page load in dark mode. Switched to a lazy useState initializer that reads document.documentElement.classList synchronously at mount/hydration time instead of hardcoding "light" and fixing it up afterward — safe here since ThemeProvider is a Client Component executing after the inline theme-init script has already run. Guarded for SSR, where this component still executes once with no `document` available; the client's own hydration render (which is what actually paints) always has it by then (closes MergeFi#208).
AvatarStack's numeric overflow indicator had no aria-label, title, or
any text alternative beyond the bare "+{rest}" string. A screen reader
encountering it read only "+3" with no indication it represents
additional, hidden contributors beyond the ones already announced —
sighted users infer this from the stacked-circle visual context, which
isn't conveyed to assistive technology. This badge sits right next to
the homepage's "Joined by N contributors already earning" copy, so it's
one of the first interactive-looking elements a screen reader user
encounters on the page.
Added aria-label={`+${rest} more contributors`} plus a title listing
the hidden seeds, for sighted mouse users hovering it (closes MergeFi#209).
EmptyState's icon had no aria-hidden and no accessible-name suppression, despite every meaningful message it conveys already being present as text immediately below it — the icon is purely decorative, but some screen readers may still attempt to announce it (lucide icons render as inline SVG, and an unlabeled SVG's a11y treatment varies by browser/AT). EmptyState is used across every dashboard and list view in the app, so this same gap repeated everywhere it's rendered. Added aria-hidden="true", the same way StatCard.tsx already does for its own decorative icons (closes MergeFi#210).
…the pending pattern
Button exposed no loading/pending prop — every consumer performing an
async action reimplemented the same manual pattern independently
(disabled={pending || connecting} + a manually swapped text label),
across five separate call sites in IssueActions, MilestoneActions
(x2), and ConnectPanel. None set aria-busy while pending, and none
rendered a visual spinner — only the text label changed, which isn't
reliably announced to a screen reader without an aria-live region, and
a low-vision user relying on zoom/high contrast may not notice a
text-only change either. Every one of these is a real, often
money-moving action (fund, claim, refund, deposit, wallet connect).
Added a `loading` prop to Button: sets aria-busy="true", forces
disabled (independent of an explicitly-passed disabled), and renders a
small inline spinner. Migrated all five existing call sites from
disabled={pending || connecting} to loading={pending || connecting},
so the standardized behavior actually replaces the five independent
reimplementations rather than existing unused alongside them. Added
Button.test.tsx (no prior test file existed) covering the default,
loading, and explicitly-disabled states (closes MergeFi#211).
|
@richardtoms100 is attempting to deploy a commit to the chonilius' projects Team on Vercel. A member of the Team first needs to authorize it. |
|
@richardtoms100 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Four fixes spanning a visual flash bug and three accessibility gaps:
Theme icon flash on load (#208) —
ThemeProviderinitializedthemestate unconditionally to"light", correcting it a render later inside auseEffectby reading back the class the inlinethemeInitScripthad already applied to<html>before hydration. Between first paint and that effect running,ThemeTogglerendered its icon from the wrong state — a visitor whose page was actually dark saw aMoonicon (meaning "click to go dark") for one render, a visible flash on every page load in dark mode. Switched to a lazyuseStateinitializer that reads the DOM class synchronously at mount/hydration time instead.Unlabeled avatar overflow badge (#209) —
AvatarStack's"+N"overflow indicator had noaria-label/title/text alternative — a screen reader read only"+3"with no indication it represents hidden contributors. Addedaria-label={+${rest} more contributors}plus atitlelisting the hidden seeds.Non-decorative-marked EmptyState icon (#210) — the icon had no
aria-hidden, despite every meaningful message already being present as text below it.EmptyStateis used across every dashboard/list view, so the gap repeated everywhere. Addedaria-hidden="true", matchingStatCard.tsx's existing pattern.No built-in loading/aria-busy support on Button (#211) — every async-action call site (fund, claim, refund, deposit, wallet connect — 5 places total) reimplemented
disabled={pending || connecting}+ a manually swapped text label independently, with noaria-busyanywhere. Added aloadingprop toButtonthat setsaria-busy="true", forcesdisabled, and renders a small spinner — then migrated all five existing call sites to use it instead of leaving the new prop unused alongside the old pattern.Changes
src/context/ThemeContext.tsx— lazy initial-state read.src/components/ui/Avatar.tsx—aria-label/titleon the overflow badge.src/components/ui/EmptyState.tsx—aria-hiddenon the icon.src/components/ui/Button.tsx/ newButton.test.tsx—loadingprop + tests.src/app/issues/[id]/IssueActions.tsx,src/app/milestones/MilestoneActions.tsx,src/app/connect/ConnectPanel.tsx— migrated toloading.Test plan
npx tsc --noEmit— cleannpx eslinton all changed files — cleannpx jest(full suite) — 207/207 passing, no regressionsCloses #208
Closes #209
Closes #210
Closes #211