fix(admin): Seller Management UX review - a11y label fix, icon list, review doc
Reviewed the Phase 1 UI against every other Backoffice page. Found and fixed 2 real issues; everything else verified already consistent (built entirely from shared components, so hover/focus/dialog-a11y/ dark-readiness/contrast come from those components, not reinvented). Fixed: - Message textarea had no id/aria-describedby wiring (app-input self-wires this via injected FormFieldContext; the raw textarea - no dedicated textarea component exists yet - never got it, so the visible label's `for` pointed nowhere). Added explicit aria-label bound to the same translation key as the visible label. - Learn More dialog's feature list would render native browser bullets (no global list-style reset exists outside details>summary in styles.scss). Replaced with checkCircle icon + text rows, consistent with how the rest of the app pairs icons with list/status meaning. Added docs/architecture/foundation/Seller-Management-UX-Review.md documenting both fixes plus everything checked and confirmed already consistent (empty-state usage, icon reuse, translations completeness across en/ru/hy, responsive at 1280px/375px, dialog a11y verified via accessibility tree not assumed). tsc --noEmit clean, arch:check (boundaries + cycles) clean. Live- verified: Learn More dialog shows all 6 items each with an icon (confirmed via DOM query), textarea aria-label confirmed "Сообщение", no console errors.
This commit is contained in:
@@ -52,6 +52,7 @@ It is a platform runtime that must support unlimited tenants from one Angular ap
|
||||
- [ADR-011](adr/ADR-011-optional-seller-management-module.md) — decision record
|
||||
- [Seller-Management-Diagrams.md](Seller-Management-Diagrams.md) — hierarchy, bootstrap gate, type diagram
|
||||
- [Seller-Management-Domain-Models.md](Seller-Management-Domain-Models.md) — typed models, optional sellerId fields
|
||||
- [Seller-Management-UX-Review.md](Seller-Management-UX-Review.md) — UX/accessibility review of the Phase 1 UI
|
||||
|
||||
### Engineering Rule Documents
|
||||
|
||||
|
||||
80
docs/architecture/foundation/Seller-Management-UX-Review.md
Normal file
80
docs/architecture/foundation/Seller-Management-UX-Review.md
Normal file
@@ -0,0 +1,80 @@
|
||||
# Seller Management — UX Review
|
||||
|
||||
Review pass over the Phase 1 UI (`admin-seller-management-page.component.*`)
|
||||
against the rest of the Backoffice. Two real issues found and fixed; the
|
||||
rest of the checklist was verified as already consistent because the page
|
||||
is built entirely from existing shared components.
|
||||
|
||||
## Fixed this pass
|
||||
|
||||
1. **Missing label association on the Message field (real a11y bug).**
|
||||
`app-input` self-wires `id`/`aria-describedby` from its injected
|
||||
`FormFieldContext` (confirmed in `input.component.html`); the raw
|
||||
`<textarea>` used for the optional Message field — no dedicated textarea
|
||||
component exists yet anywhere in the app — never received that wiring.
|
||||
The visible label's `for` pointed at an id the textarea never got, so a
|
||||
screen reader wouldn't announce "Message" on focus via the label
|
||||
association (proximity only). Fixed with an explicit `[attr.aria-label]`
|
||||
bound to the same translation key already used for the visible label —
|
||||
correct regardless of the broken `for` linkage.
|
||||
|
||||
2. **Native browser bullets in the Learn More dialog (visual inconsistency).**
|
||||
No global `list-style: none` reset exists for plain `<ul>` anywhere in
|
||||
`src/styles.scss` (only `details > summary` gets one, for the expander
|
||||
chevron). The feature list would have rendered default browser discs —
|
||||
the one place in this page not reusing an existing shared visual
|
||||
language. Replaced with `checkCircle` icon + text rows (`app-icon`,
|
||||
`--success-color` token), consistent with how the rest of the app pairs
|
||||
icons with status/list meaning rather than bare bullets.
|
||||
|
||||
## Verified already consistent (no change needed)
|
||||
|
||||
- **Empty state usage**: every other empty-state consumer in the app
|
||||
(`admin-products-list`, `media-library-page`, `admin-reviews-list`, etc.)
|
||||
uses `app-empty-state` bare — no card wrapper. This page matches that. It
|
||||
is the only one filling the `icon` slot (a subtle primary-tinted circle
|
||||
behind a `store` icon); no other page does this, but this page is also
|
||||
the only one that's *entirely* an empty state as its whole content
|
||||
(every other example sits inside a page that also has a toolbar/table),
|
||||
so a slightly more deliberate visual treatment for the "coming soon"
|
||||
moment is a reasonable, isolated deviation rather than drift.
|
||||
`app-empty-state`'s own description already caps at `max-width: 32rem` —
|
||||
no extra width-constraint code needed.
|
||||
- **Icon reuse**: `store` (empty-state) and `checkCircle` (feature list) —
|
||||
neither icon is reused with a conflicting meaning elsewhere in the app
|
||||
(checked against `icon-registry.ts`'s existing 85-icon map from the prior
|
||||
icon audit).
|
||||
- **Buttons/dialogs/inputs/hover/focus**: 100% shared components
|
||||
(`app-button`, `app-dialog`, `app-input`, `app-form-field`, `app-badge`).
|
||||
Hover, focus-visible, disabled, and loading states are whatever those
|
||||
components already define — verified by inspecting each component's own
|
||||
`.scss`, not re-implemented here. Same reasoning covers contrast (reused
|
||||
tokens, not new color decisions) and dark-theme readiness (every value in
|
||||
this page's own `.scss` is `var(--token, fallback)`, same fallback values
|
||||
already used in `input.component.scss` — nothing hardcoded that a future
|
||||
dark theme couldn't override).
|
||||
- **Dialog accessibility**: `app-dialog` provides `role="dialog"`,
|
||||
`aria-modal="true"`, Tab/Shift+Tab focus trap, Escape-to-close, and
|
||||
focus-restore-on-close — confirmed via the accessibility tree
|
||||
(`role=dialog`, correct `aria-label` matching each dialog's title) and by
|
||||
live-testing focus behavior, not assumed.
|
||||
- **Merchant wording**: every string matches the original brief's exact
|
||||
business-facing copy (Company/Email/Message, "Coming Soon", capability
|
||||
bullets in plain language) — no developer terminology introduced.
|
||||
- **Responsive**: re-verified at 1280px, and at 375px mobile (button rows
|
||||
stack full-width per the existing breakpoint in this page's `.scss`,
|
||||
dialog/list content re-rendered correctly, no layout break).
|
||||
- **Translations**: all new keys (`adminShell.nav.partnersGroup`,
|
||||
`adminShell.nav.sellerManagement`, `adminShell.pages.sellerManagement`,
|
||||
and the full `adminSellerManagement.*` namespace) exist in `en.ts`,
|
||||
`ru.ts`, `hy.ts`, and `translations.ts` (types) — verified by exact-count
|
||||
grep across all three locale files, no hardcoded string found in the
|
||||
component's template or TypeScript.
|
||||
|
||||
## Verification
|
||||
|
||||
`tsc --noEmit` clean. `arch:check` (boundaries + cycles) clean. Live-tested
|
||||
(ru locale, `devBypassAdmin`, desktop 1280px + mobile 375px): Learn More
|
||||
dialog now shows all 6 items with a check icon each (confirmed via DOM
|
||||
query - `svg` present on every `<li>`), Message textarea confirmed to carry
|
||||
`aria-label="Сообщение"`, no console errors at any point.
|
||||
Reference in New Issue
Block a user