feat: Add design principles checklist to UI verification workflow#2349
feat: Add design principles checklist to UI verification workflow#2349marcusquinn merged 2 commits intomainfrom
Conversation
Add comprehensive design quality gates covering typography (max 740px paragraph width, min 16px text, 3 font family limit, line/letter spacing), layout (consistent padding/alignment/sizing, logo clear space, graceful responsive), interaction (44px touch targets, hover states, bold underlined links, meaningful alt/title, scroll wheel behaviour), colour (informational coding, dual-mode highlighting, logo home link), information architecture (visual hierarchy, naming conventions), and Mom Test usability evaluation.
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
WalkthroughAdded a Design Principles Checklist to the UI verification doc, duplicated the checklist placement, and integrated design-principles checks into workflow steps (notably steps 8 and 9). Expanded Quick Verification (Minimal) with paragraph width, text size, and touch-target spot-checks; minor wording updates throughout. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Thu Feb 26 02:27:33 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
🤖 Augment PR SummarySummary: Expands the UI verification workflow with an enforceable design-principles checklist to standardize visual/UX quality gates. Changes:
🤖 Was this summary useful? React with 👍 or 👎 |
| |-----------|------|---------------| | ||
| | **Maximum paragraph width** | No paragraph text wider than 740px (approximately 75 characters per line) | Inspect `max-width` on text containers; screenshot at desktop-lg to confirm text does not stretch edge-to-edge | | ||
| | **Minimum text size** | Body text minimum 16px (1rem). No text below 14px except legal footnotes or captions | Playwright `evaluate()` to check computed `font-size` on all text elements | | ||
| | **Font family limit** | Maximum 3 font families: (1) headings/titles, (2) body/buttons/forms/blockquotes, (3) code/monospace | Inspect computed `font-family` values across the page; flag any fourth family | |
There was a problem hiding this comment.
The Minimum text size rule says body text is minimum 16px, but then allows text down to 14px for footnotes/captions; that’s internally inconsistent and may be hard to enforce as a “quality gate” without clarifying the exception.
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| | Principle | Rule | How to verify | | ||
| |-----------|------|---------------| | ||
| | **Finger-friendly touch targets** | All interactive elements minimum 44x44px touch target (WCAG 2.5.8). Adequate spacing between adjacent targets to prevent mis-taps | Playwright `evaluate()` to measure element bounding boxes on mobile device emulation | | ||
| | **Hover/rollover state changes** | All clickable elements must have a visible hover state change (colour, underline, shadow, scale) to signal interactivity | Playwright `hover()` + screenshot comparison; verify visual change on buttons, links, cards | |
There was a problem hiding this comment.
The touch target reference looks mismatched: WCAG 2.2 SC 2.5.8 “Target Size (Minimum)” is 24×24 CSS px, while 44×44 is typically tied to WCAG 2.5.5 (AAA) / platform guidance; the citation here may mislead reviewers.
Severity: medium
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| | **Hover/rollover state changes** | All clickable elements must have a visible hover state change (colour, underline, shadow, scale) to signal interactivity | Playwright `hover()` + screenshot comparison; verify visual change on buttons, links, cards | | ||
| | **Link text styling in paragraphs** | Links within paragraph text must be bold, underlined, and use a distinct colour to be identifiable without relying on colour alone | Inspect computed styles on `<a>` elements within `<p>` tags; verify `font-weight`, `text-decoration`, and `color` differ from surrounding text | | ||
| | **Meaningful alt and title attributes** | All images must have descriptive `alt` text. Include relevant brand and subject keywords for SEO. Decorative images use `alt=""`. Interactive elements should have `title` attributes where helpful for screen readers | Playwright `evaluate()` to audit all `<img>` elements for `alt`; flag empty or generic values like "image" or "photo" | | ||
| | **Information-relevant icons** | Icons should reinforce meaning, not decorate. Every icon should be understandable without its label, or be paired with a text label | Visual check; verify icons match their associated action or content | |
There was a problem hiding this comment.
This guidance may be misleading for accessibility: title attributes aren’t reliably announced by screen readers, and recommending SEO keywords in alt text can encourage non-descriptive alt content (alt is primarily for users of assistive tech).
Severity: low
🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.
| ### Usability (Mom Test) | ||
|
|
||
| After all technical checks pass, evaluate the page against the Mom Test heuristic (see `seo/mom-test-ux.md`): | ||
|
|
There was a problem hiding this comment.
- Clarify 14px text exception is supplementary text only, not a contradiction - Fix touch target WCAG reference: 44px is Apple/Google HIG + WCAG 2.5.5 AAA, 24px is WCAG 2.5.8 AA minimum -- aim for 44, never below 24 - Prioritise accessibility over SEO in alt text guidance, recommend aria-label over title attribute (inconsistent screen reader support) - All three points raised by Augment code review
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Thu Feb 26 02:31:14 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.agents/workflows/ui-verification.md (1)
308-309: Define one severity rubric for all checklist violations.Line 329 requires severity reporting, but only the Mom Test section defines S1/S2 blocking criteria. Add a compact severity matrix for the full checklist so reviews stay consistent and automatable.
Also applies to: 329-329
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/workflows/ui-verification.md around lines 308 - 309, Add a compact, single severity rubric covering all checklist violations (not just the Mom Test) by defining S1/S2/S3 levels and examples for each checklist item type and placing the matrix near the existing "severity reporting" text; update the Mom Test section to reference this global "severity matrix" and ensure any checklist rules that previously implied S1/S2 link to the unified rubric so reviews are consistent and machine-readable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agents/workflows/ui-verification.md:
- Around line 278-279: Replace the current "title attribute" guidance under
"Meaningful alt and title attributes" with accessible-name guidance: remove
recommending use of title for screen readers and instead instruct authors to
ensure interactive elements have a programmatic accessible name via visible
text, aria-label, or aria-labelledby; update the Playwright audit step (the
existing evaluate() check) to verify the computed accessible name for
interactive elements and images (not just the presence of a title), and flag
empty/generic names like "image" or "photo" as well as missing
aria-label/aria-labelledby/visible text.
---
Nitpick comments:
In @.agents/workflows/ui-verification.md:
- Around line 308-309: Add a compact, single severity rubric covering all
checklist violations (not just the Mom Test) by defining S1/S2/S3 levels and
examples for each checklist item type and placing the matrix near the existing
"severity reporting" text; update the Mom Test section to reference this global
"severity matrix" and ensure any checklist rules that previously implied S1/S2
link to the unified rubric so reviews are consistent and machine-readable.



Summary
workflows/ui-verification.mdas enforceable quality gatesDesign Principles Added
Typography & Readability (7 rules)
Layout & Spacing (6 rules)
Interaction & Accessibility (6 rules)
Colour & Theming (3 rules)
Information Architecture (2 rules)
Usability — Mom Test (6 principles)
Testing
Summary by CodeRabbit