Skip to content

fix: add aria-live polite region for form submission announcements (#85) - #155

Merged
komalharshita merged 4 commits into
komalharshita:mainfrom
vedika76:fix/aria-live-form-status
Jun 1, 2026
Merged

fix: add aria-live polite region for form submission announcements (#85)#155
komalharshita merged 4 commits into
komalharshita:mainfrom
vedika76:fix/aria-live-form-status

Conversation

@vedika76

Copy link
Copy Markdown
Contributor

Summary [required]

Added an ARIA live region to the recommendation form to ensure screen readers announce form submission status (loading and error messages), improving accessibility for visually impaired users.

Related Issue [required]

Closes #85

Type of Change [required]

  • Bug fix — resolves a broken behaviour
  • Feature — adds new functionality
  • Data — adds new projects to data/projects.json
  • Documentation — updates docs, README, or code comments only
  • Style — CSS or visual changes only, no logic change
  • Refactor — restructures code without changing behaviour
  • Test — adds or updates tests

What Was Changed [required]

File Change made
templates/index.html Wrapped form-error-general and results-loading elements in a <div> with aria-live="polite" and aria-atomic="true" attributes.

How to Test This PR [required]

  1. Clone this branch: git checkout fix/aria-live-form-status
  2. Install dependencies: pip install -r requirements.txt
  3. Run the app: python app.py
  4. Open http://127.0.0.1:5000
  5. Submit the form with empty fields to see error message
  6. Submit the form with valid data to see loading state
  7. Run the tests: python tests/test_basic.py

Expected test output:

27 passed, 0 failed out of 27 tests

Test Results [required]

(Screenshot shows here)
passed_test

27 passed, 0 failed out of 27 tests
All tests passed successfully.

Screenshots (if UI change)

N/A - No visual UI changes, only ARIA attributes added

Self-Review Checklist [required]

  • I have read CONTRIBUTING.md and followed all guidelines
  • My branch name follows the convention: fix/aria-live-form-status
  • I have run python tests/test_basic.py and all 27 tests pass
  • I have run flake8 . locally and there are no errors (N/A- no python files changed)
  • I have not introduced any print() or console.log() debug statements
  • Every new function I wrote has a docstring (N/A- no new functions added)
  • I have not modified files outside the scope of the linked issue
  • If I changed the UI, I tested it at 375px (mobile) and 1280px (desktop) (N/A- no visual changes)
  • If I added a project to the dataset, it has all required JSON fields (N/A- no project added)

Notes for Reviewer

The issue example suggested one wrapper, but due to the actual file structure (elements in different sections), I added two live regions: one in the form section and one in the results section. Both provide full screen reader coverage.

@vercel

vercel Bot commented May 16, 2026

Copy link
Copy Markdown

@vedika76 is attempting to deploy a commit to the komalsony234-1530's projects Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for submitting your first pull request to DevPath.

Before review:

  • Complete the PR template fully
  • Ensure all tests pass
  • Link your PR to an issue
  • Keep changes scoped to the issue

A maintainer will review your contribution soon.

@vedika76

Copy link
Copy Markdown
Contributor Author

Hello @komalharshita ,
I've completed the changes for this PR. Could you please review it when you get a chance? Happy to make any adjustments if needed.

@komalharshita

Copy link
Copy Markdown
Owner

@vedika76 kindly resolve the merge conflicts before merging

@komalharshita komalharshita added the need review Further information is requested label May 17, 2026

@komalharshita komalharshita left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution. Improving loading-state announcements for screen-reader users is a valuable accessibility goal, and adding aria-live support is the right direction conceptually.

However, the current implementation should not be merged yet because it introduces structural duplication and invalid DOM behavior.

Main issues:

  • The PR now creates duplicate id="results-loading" elements in multiple places.
  • The loading-state markup was duplicated instead of enhancing the existing structure.
  • Duplicate IDs can break JavaScript selectors, loading-state synchronization, and assistive-technology behavior.
  • Multiple live regions announcing similar content may also create repetitive or confusing screen-reader output.

A cleaner and safer approach would be:

  • keep a single unique loading container
  • add aria-live="polite" to the existing status region only
  • avoid duplicating loading markup
  • ensure announcements remain concise and non-repetitive

Please refactor the implementation to enhance the existing loading/error structure instead of duplicating it. Once the DOM structure is corrected and duplicate IDs are removed, the PR can be re-reviewed.

@komalharshita komalharshita left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for addressing the previous review comments.

I re-checked the updated implementation and the earlier concerns have been resolved:

• The duplicate loading container has been removed.
• The existing #results-loading element is now enhanced with aria-live="polite" and aria-atomic="true" instead of duplicating markup.
• No duplicate IDs are introduced.
• Existing JavaScript selectors and loading-state behavior remain intact.
• The accessibility improvement is implemented in a clean and minimal way.

This is now a focused accessibility enhancement that improves screen-reader announcements without altering existing functionality.

Approved. Nice work refining the implementation based on feedback.

@komalharshita
komalharshita merged commit fc21592 into komalharshita:main Jun 1, 2026
7 of 8 checks passed
@komalharshita komalharshita added level:beginner quality:clean gssoc:approved and removed need review Further information is requested labels Jun 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add aria-live="polite" region for form submission status announcements

2 participants