Skip to content

fix(email): a11y + mobile/UX correctness polish - #358

Open
DiogoDuart3 wants to merge 5 commits into
oblien:mainfrom
DiogoDuart3:pr3-a11y-correctness-polish
Open

fix(email): a11y + mobile/UX correctness polish#358
DiogoDuart3 wants to merge 5 commits into
oblien:mainfrom
DiogoDuart3:pr3-a11y-correctness-polish

Conversation

@DiogoDuart3

Copy link
Copy Markdown
Contributor

A batch of small, independent a11y and correctness fixes to the webmail client, verified live via a headless login + screenshot pass at 1280px and 390px before/after each change.

Mobile / correctness

  • Compose FAB: a floating action button on mobile mail-list views opens the same compose dialog. Previously only reachable through the sidebar, which on mobile is a Sheet that unmounts (not just hides) while closed - 2 taps to compose. Hidden on desktop (lg:hidden) to avoid overlapping the sidebar's own persistent "New email" trigger.
  • Thread toolbar: Reply-all drops its text label below md and the action row gets overflow-x-auto - the label was the one item wide enough to push the "..." menu off the right edge on a phone-width screen.
  • Fixed a bare 0 rendering next to the subject on a single-message thread: totalEmails && totalEmails > 1 && ... - 0 && x evaluates to 0, and unlike false/null/undefined, React renders a numeric 0 as text. Switched to a ternary.
  • Sidebar Folders/Labels section: hidden when there are no folders instead of showing a permanently empty section header.
  • Sidebar account block: was showing the account email twice (once as the name-line fallback, once as the email line) for a mailbox with no display name set.
  • Compose subject placeholder: "Re: Design review feedback" -> "Subject".
  • Hardened redirect URLs against being baked in at build time (getAppUrl() now reads window.location.origin at runtime consistently across all redirect call sites) - the client is an SPA served same-origin, so there's no reason a redirect should ever reference a build-time value.
  • Removed a stray literal + character sitting before a <button> tag in render-labels.tsx (pre-existing, not introduced by this PR) - valid JSX syntax so it doesn't break the build, but it gives a TooltipTrigger asChild two children instead of one, which Radix's Slot (React.Children.only) will throw on at runtime for any label with a visible tooltip.

Accessibility (Lighthouse-driven)

  • Removed a dead Radix Collapsible wrapper around sidebar nav items that never had a matching CollapsibleContent - left a dangling aria-controls reference (aria-valid-attr-value) and put a non-<li> wrapper directly inside <ul data-sidebar="menu"> (invalid list semantics, aria-required-children).
  • Added a <main> landmark around the mail app's primary content region.
  • aria-allowed-attr: two *Trigger asChild sites were injecting type="button"/aria-haspopup onto bare <div>s with no interactive role - invalid. Converted to real <button>s.
  • button-name: added aria-label to icon-only buttons across the mail list, thread view, sidebar, and composer (mostly reusing existing tooltip copy).
  • Contrast: unscoped text-[#8C8C8C] (fails ~3.36:1 on light backgrounds) now falls back through text-muted-foreground with dark:text-[#8C8C8C] for the dark-mode accent. Darkened the #006FFE brand blue to #0062D4 for the two buttons pairing it with white text (was ~4.46:1, just under the 4.5:1 minimum). Dropped an opacity-70 utility on list-row timestamps that diluted text-muted-foreground below 4.5:1 at rest in both themes (full opacity only kicked in on hover/select).
  • Fixed a real bug found while investigating a recurring Uncaught (in promise) console error (12-17x per session): the IndexedDB query-persister's shouldDehydrateQuery had dropped TanStack Query's default "only persist settled queries" check. In-flight queries still carry a live Promise field when dehydrated, which IndexedDB's structured-clone put() cannot serialize - throwing DataCloneError on nearly every persist right after login. Restored the missing status check.

Note on CI

Recent merged PRs against main (#322, #327) show Typecheck: SUCCESS / Test: FAILURE on their PR-context runs. Flagging in case it reproduces here too - doesn't look related to this change.

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.

1 participant