feat(ui): P0 a11y essentials on index.html + personalize.html#24
Conversation
Closes the P0 batch from the UX audit. Visual identity preserved; this is
strictly assistive-tech, keyboard, and discoverability work.
index.html
- `<input type=url>` now has a real `<label>` (visually hidden via .sr-only)
instead of relying on placeholder text alone — screen readers were
announcing "edit" with no context.
- Wrapped fields in `<form id=downloadForm novalidate>` with `type=submit`;
removed the inline `onclick="startDownload()"` and the manual keypress
handler. Enter key + assistive-tech "submit" both work natively now and
the legacy CSP-hostile inline handler is gone.
- Replaced `alert('Por favor, insira uma URL válida')` with an inline
`#errorMessage` region (`role=alert`, `aria-live=assertive`). Error
surfaces both for client-side validation AND backend 400/connection
failures (previously logs hid the failure after 5s).
- Log container: `role=log` + `aria-live=polite` + `aria-label`. SSE
progress updates are now announced to screen readers as they arrive.
- Success banner: `role=status` + `aria-live=polite`; focus migrates to
the manual download link on completion (keyboard users land on the
next actionable element instead of nowhere).
- `<main>` element with `aria-busy` toggled true/false around the
in-flight worker so AT can announce the long task state change.
- Removed the auto-clear of the URL input after success — re-running for
a variant is a common pattern. Logs no longer auto-hide when the
session ended in error (you can still read the failure trace).
- New `<a href="/personalize">` discovery link at the bottom of the card
(it was previously reachable only via URL bar / docs).
- `button:focus-visible / input:focus-visible / a:focus-visible` gets a
3px outline + 2px offset so keyboard users see where focus lands.
- Subtitle color darkened from `#666` to `#4a4a4a` to clear WCAG AA
contrast on the white-glass card.
personalize.html
- Both `.status` divs now have `role=status` + `aria-live=polite`.
- `<main aria-busy>` toggles during the two fetch calls.
- Sections labelled via `aria-labelledby` referencing their `<h2 id>` —
landmark navigation now identifies each step.
- Focus moves to `#company` after `/api/personalize/structure` succeeds
(keyboard users follow the flow); focus to `#step-out` after run.
- "← Voltar" link to `/` at the bottom (was stranded before).
- Result rendering switched from `.innerHTML` interpolation to
`createElement(code) + textContent` — same visual output, no string
interpolation of server payload into the DOM (defensive even though
the API field is operator-controlled).
- Status colors hardened: `.error` red darkened `#c1272d` → `#991b1b` and
bolded; `.success` `#2a7f3a` → `#166534` and bolded.
- Matching `:focus-visible` outline rule on all interactive elements.
tests/test_template_a11y.py (new, +14 cases)
Regression net for the a11y contract. Asserts presence of:
- `<label for=urlInput>` + the label text
- `id=downloadForm` + `type=submit` + ABSENCE of `onclick=`
- `id=errorMessage` + `role=alert` + `aria-live=assertive`
- `id=logContainer` + `role=log` + `aria-live=polite`
- `id=successMessage` + `role=status`
- `id=mainCard` + `aria-busy=false` (initial)
- `href="/personalize"` discovery link
- The specific `alert('Por favor` legacy call is gone
- Both personalize.html status regions are polite live
- All 3 personalize sections have `aria-labelledby`
- `href="/"` back-link on /personalize
- `:focus-visible` style present on both templates
Verifications:
- pytest -q → 233 passed, 2 skipped (was 219 + 2; +14 a11y cases)
- pytest tests/test_template_a11y.py -v → 14/14 in 0.31s
- ruff check kratos_clone/ scripts/ tests/test_template_a11y.py → clean
- ruff format --check → 12 files already formatted
- mypy --config-file pyproject.toml → Success, 21 source files
- bandit --severity-level medium → 0 medium/high (10 low unchanged)
Not in this PR (P1/P2 from the audit, deferred):
- Elapsed-time indicator during processing
- Reconnect/timeout logic on SSE drop
- Captures dropdown (replace free-text `html_dir`)
- Step indicator (1/3, 2/3, 3/3) in personalize
- localStorage persistence of last URL
📝 WalkthroughWalkthroughThis PR improves accessibility across the Website Downloader and Personalize templates by adding ARIA landmarks, semantic error/status regions, keyboard navigation, and focus management. Index errors migrate from ChangesAccessibility Improvements for Download and Personalize Templates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0433164ee0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| el.classList.add('active'); | ||
| $('urlInput').setAttribute('aria-invalid', 'true'); | ||
| // Move focus to input so screen readers + keyboard users find the error | ||
| $('urlInput').focus(); |
There was a problem hiding this comment.
When showError() is called from /start-download failures or the SSE error branch, urlInput is still disabled because setLoading(false)/resetUI() has not run yet. Calling focus() on a disabled input is ignored, so keyboard and screen-reader users are not actually moved to the error even though this change is meant to add focus management. Re-enable before calling showError() or focus the alert region instead.
Useful? React with 👍 / 👎.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request focuses on enhancing the accessibility and user experience of the website downloader and personalization templates by implementing ARIA roles, live regions, focus management, and native form submission. It also introduces a comprehensive suite of accessibility regression tests and improves security by replacing innerHTML with textContent in several locations. The review feedback identifies remaining WCAG AA contrast violations in specific text elements and suggests refining the auto-hide logic for success messages to prevent focus loss for keyboard and screen reader users.
| if (!isError) { | ||
| setTimeout(() => { | ||
| $('logContainer').classList.remove('active'); | ||
| hideSuccess(); | ||
| clearLogs(); | ||
| }, 5000); | ||
| } |
There was a problem hiding this comment.
Auto-hiding success messages that contain interactive elements (like the manual download link) can be disorienting for screen reader and keyboard users. If the message disappears while the user is navigating towards it or reading it, focus is lost and the viewport may jump.
Consider checking if the user is currently interacting with the message before hiding it, or removing the auto-hide for success messages entirely as you did for error logs.
| if (!isError) { | |
| setTimeout(() => { | |
| $('logContainer').classList.remove('active'); | |
| hideSuccess(); | |
| clearLogs(); | |
| }, 5000); | |
| } | |
| if (!isError) { | |
| setTimeout(() => { | |
| if ($('successMessage').contains(document.activeElement)) return; | |
| $('logContainer').classList.remove('active'); | |
| hideSuccess(); | |
| clearLogs(); | |
| }, 5000); | |
| } |
| </div> | ||
| </div> | ||
|
|
||
| <p style="margin-top:1.5rem;font-size:0.85rem;color:#666;text-align:center"> |
There was a problem hiding this comment.
The color #666 on a white background has a contrast ratio of 3.94:1, which fails the WCAG AA requirement of 4.5:1 for normal text. Since you've already updated the .subtitle class to #4a4a4a (9.8:1) for better contrast, you should apply the same color here for consistency and accessibility compliance.
| <p style="margin-top:1.5rem;font-size:0.85rem;color:#666;text-align:center"> | |
| <p style="margin-top:1.5rem;font-size:0.85rem;color:#4a4a4a;text-align:center"> |
| button:focus-visible, input:focus-visible, textarea:focus-visible, a:focus-visible { | ||
| outline: 3px solid #4a6cf7; | ||
| outline-offset: 2px; | ||
| } |
There was a problem hiding this comment.
The .subtitle class in this file (defined at line 22, outside this diff) still uses #666, which has insufficient contrast (3.94:1). To match the accessibility improvements made in index.html and meet WCAG AA standards, you should override it here with #4a4a4a.
button:focus-visible, input:focus-visible, textarea:focus-visible, a:focus-visible {
outline: 3px solid #4a6cf7;
outline-offset: 2px;
}
.subtitle { color: #4a4a4a; }| </section> | ||
| </div> | ||
|
|
||
| <p style="margin-top:1.5rem;font-size:0.85rem;color:#666;text-align:center"> |
There was a problem hiding this comment.
Similar to the footer in index.html, the color #666 here fails WCAG AA contrast requirements. Use #4a4a4a instead to ensure the link text is readable against the white background.
| <p style="margin-top:1.5rem;font-size:0.85rem;color:#666;text-align:center"> | |
| <p style="margin-top:1.5rem;font-size:0.85rem;color:#4a4a4a;text-align:center"> |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
templates/index.html (1)
231-386: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winConsolidate into a single script block.
Two separate script blocks violate the coding guideline: "In frontend (
templates/index.html), use a single inline<script>block at end of body." Merge the browser logger (393-554) into the main script block (231-386) to comply.As per coding guidelines: "In frontend (
templates/index.html), use a single inline<script>block at end of body — no separate JS files"Also applies to: 393-554
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@templates/index.html` around lines 231 - 386, The page currently has two separate inline <script> blocks; merge the second "browser logger" script into the existing single script block so all front-end JS lives at the end of body. Copy the functions and variables from the other block (e.g., addLog, clearLogs, setLoading, resetUI, startDownload, connectSSE, triggerDownload, showSuccess, hideSuccess, currentSessionId, eventSource and the $ helper) into the main script, remove duplicates (do not re-declare $ or duplicate function names), keep the form submit handler ($('downloadForm').addEventListener(...)) and SSE setup (connectSSE/eventSource) intact, then delete the extra <script> tag so only one inline script remains. Ensure references to elements like 'downloadBtn', 'urlInput', 'logContainer', 'successMessage' and 'downloadLink' still resolve after the merge.
🧹 Nitpick comments (2)
templates/index.html (1)
377-379: 💤 Low valueConsider
textContent = ''for consistency.While
innerHTML = ''works for clearing the log container, usingtextContent = ''would be more consistent with the XSS-prevention approach used elsewhere (e.g., personalize.html line 210).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@templates/index.html` around lines 377 - 379, Update the clearLogs function to assign an empty string to the element's textContent instead of innerHTML: locate the clearLogs function and the $('logContainer') reference and replace the innerHTML = '' assignment with textContent = '' to match the XSS-safe approach used elsewhere (e.g., personalize.html usage pattern).templates/personalize.html (1)
110-224: 💤 Low valueConsider consolidating script blocks for consistency.
While the coding guideline only mandates a single script block for
index.html, consolidating both blocks (main app + logger) into one would improve consistency across templates and reduce the chance of script ordering issues.Also applies to: 230-326
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@templates/personalize.html` around lines 110 - 224, Consolidate the two separate script blocks into a single IIFE so the DOM helpers and UI logic (the var $ helper, setStatus, setBusy, card, extractBtn/runBtn listeners, and FormData flow in the anonymous function) and the logger initialization are defined in one place; merge the logger setup into this IIFE before any calls that may use it, avoid duplicate globals (remove the second var $ or rename if needed), and ensure all event listeners (extractBtn.addEventListener, runBtn.addEventListener) and status/update calls keep their original behavior and ordering so script execution and accessibility focus handling are unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@templates/index.html`:
- Around line 231-386: The page currently has two separate inline <script>
blocks; merge the second "browser logger" script into the existing single script
block so all front-end JS lives at the end of body. Copy the functions and
variables from the other block (e.g., addLog, clearLogs, setLoading, resetUI,
startDownload, connectSSE, triggerDownload, showSuccess, hideSuccess,
currentSessionId, eventSource and the $ helper) into the main script, remove
duplicates (do not re-declare $ or duplicate function names), keep the form
submit handler ($('downloadForm').addEventListener(...)) and SSE setup
(connectSSE/eventSource) intact, then delete the extra <script> tag so only one
inline script remains. Ensure references to elements like 'downloadBtn',
'urlInput', 'logContainer', 'successMessage' and 'downloadLink' still resolve
after the merge.
---
Nitpick comments:
In `@templates/index.html`:
- Around line 377-379: Update the clearLogs function to assign an empty string
to the element's textContent instead of innerHTML: locate the clearLogs function
and the $('logContainer') reference and replace the innerHTML = '' assignment
with textContent = '' to match the XSS-safe approach used elsewhere (e.g.,
personalize.html usage pattern).
In `@templates/personalize.html`:
- Around line 110-224: Consolidate the two separate script blocks into a single
IIFE so the DOM helpers and UI logic (the var $ helper, setStatus, setBusy,
card, extractBtn/runBtn listeners, and FormData flow in the anonymous function)
and the logger initialization are defined in one place; merge the logger setup
into this IIFE before any calls that may use it, avoid duplicate globals (remove
the second var $ or rename if needed), and ensure all event listeners
(extractBtn.addEventListener, runBtn.addEventListener) and status/update calls
keep their original behavior and ordering so script execution and accessibility
focus handling are unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 30160f60-8baa-4bf4-834f-2a09299a9a3e
📒 Files selected for processing (3)
templates/index.htmltemplates/personalize.htmltests/test_template_a11y.py
1 participant
Summary
Closes the P0 batch from the UX audit. Visual identity preserved; this is strictly assistive-tech, keyboard-navigation, and discoverability work. 7 a11y categories addressed across both Flask templates; 14 new regression tests lock down the contract.
What's in (7 categories)
<input>only had placeholder, no<label>— AT announced "edit"<label class=sr-only for=urlInput>+ sr-only utilityalert()for validation errors — blocked AT, ugly mobile, unstyled#errorMessagewithrole=alert,aria-live=assertiverole=log+aria-live=polite+aria-label<form>wrapping → Enter via manual keypress handler<form id=downloadForm>+type=submitaria-busyduring long worker — AT didn't know task was in flight<main aria-busy>toggled around fetchesonclick=inline mixed withaddEventListener— CSP-hostile, inconsistentDiscoverability bonuses
<a href=\"/personalize\">link at the bottom of/. Previously the personalize feature was only reachable via URL bar or docs./personalizeback to/.Visual polish (collateral)
#666→#4a4a4ato clear WCAG AA on the white-glass card..errorred darkened (#c1272d→#991b1b) + bolded for visibility.:focus-visibleoutline (3px solid + 2px offset) on every interactive element — keyboard users see where focus lands.Behavior changes
.innerHTML(server-payload interpolation) tocreateElement(code) + textContent— same visual output, defensive even though the field is operator-controlled.Test plan
pytest tests/test_template_a11y.py -v→ 14/14 in 0.31spytest -q→ 233 passed, 2 skipped (was 219 + 2; +14 a11y cases)ruff check tests/test_template_a11y.py kratos_clone/ scripts/→ cleanruff format --check→ 12 files already formattedmypy --config-file pyproject.toml→ Success, 21 source filesbandit --severity-level medium→ 0 medium/high (10 low unchanged from baseline)Deferred (P1/P2 from audit, separate PRs)
GET /api/capturesto replace free-texthtml_dirfield on /personalizeSummary by CodeRabbit
Release Notes
New Features
Bug Fixes
Tests