Skip to content

fix(a11y): use role group for ranking lists container - #3842

Merged
Chartman123 merged 1 commit into
nextcloud:mainfrom
yashgill-10:fix-3840-ranking-aria
Nov 8, 2026
Merged

Chartman123 merged 1 commit into
nextcloud:mainfrom
yashgill-10:fix-3840-ranking-aria

Conversation

@yashgill-10

Copy link
Copy Markdown
Contributor

Summary

Fixes #3840.

Problem: In submit (read-only) mode, the ranking question's outer div.question__content had role="list", but its direct children are the .ranking-unranked and .ranking-ranked sections — not list items. This violates aria-required-children (axe): a list must only contain listitem children.

Root cause: The explicit role="list" was placed on the wrapper instead of on the actual <ul> elements rendered by the TransitionGroups, and the ranked <li> elements redundantly carried role="listitem".

Solution:

  • Outer container now uses role="group" and keeps its existing aria-labelledby/aria-describedby/aria-errormessage/aria-invalid bindings.
  • The unranked <ul> gets aria-label="Unranked options".
  • The "Your ranking" label gets an id, and the ranked <ul> is named via aria-labelledby.
  • Removed redundant role="listitem" from ranked <li> elements (native <li> semantics already apply).

Tests: Added a Playwright case to playwright/e2e/a11y-question-inputs.spec.ts asserting the group contains two named lists.

Testing performed

  • prettier --check / eslint on both changed files: pass.
  • stylelint on QuestionRanking.vue: pass.
  • vue-tsc --noEmit: reports pre-existing errors across the codebase (e.g. QuestionShort.vue, Forms.vue, main.ts) unrelated to this template-only change; none reference the modified lines.
  • Playwright e2e requires a running Nextcloud server and could not be executed in this environment; the spec was added following existing conventions in the same file.

AI disclosure

This change was prepared with AI assistance (Devin) and reviewed by the contributor; the implementation and verification steps above were actually executed.

@Chartman123 Chartman123 added 3. to review Waiting for reviews enhancement New feature or request a11y related to accessibility labels Nov 7, 2026
@Chartman123

Copy link
Copy Markdown
Collaborator

/backport to stable5.4

@backportbot backportbot Bot added the backport-request Pending backport by the backport-bot label Nov 7, 2026
@Chartman123
Chartman123 requested a balanced review from Copilot November 7, 2026 18:52

Copilot AI 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.

🟡 Changes recommended

The accessibility test asserts visibility on an empty ranked list with a zero-height bounding box, causing it to fail; assert attachment instead or rank an option first.

1 open issue
What changed in this PR

This PR addresses #3840 by correcting the ranking question’s accessibility structure in submit mode, grouping the two option lists rather than treating their wrapper as a list.

Changes:

  • Changes the outer container to role="group" while preserving its ARIA bindings.
  • Names both inner lists and removes the redundant ranked-item role.
  • Adds a Playwright regression test for the group and named lists.
File Description
src/​components/​Questions/​QuestionRanking.vue Corrects container semantics and labels the ranking lists.
playwright/​e2e/​a11y-question-inputs.spec.ts Adds assertions for the ranking group and its named lists.

Review effort: Balanced

@Chartman123 Chartman123 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good now, thanks for the fix :)

Please squash the two commits into one, else the automatic backport might fail.

@AndyScherzinger AndyScherzinger left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM too

The submit-mode ranking question wrapped two inner lists in a container
with role="list" even though it is not a list itself. Change the outer
container to role="group", keep the named inner lists, and add an
explicit role="list" so WebKit exposes them as lists despite missing
list markers. The ranked list only gets the role while populated to
avoid an empty list role. Add a Playwright regression test asserting
the group, the list names and the explicit roles.

Signed-off-by: yashgill-10 <yashgill-10@users.noreply.github.com>
@yashgill-10
yashgill-10 force-pushed the fix-3840-ranking-aria branch from 745bffc to 42f21c2 Compare November 8, 2026 04:03
@Chartman123
Chartman123 merged commit f0571ce into nextcloud:main Nov 8, 2026
43 of 45 checks passed
@backportbot backportbot Bot removed the backport-request Pending backport by the backport-bot label Nov 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews a11y related to accessibility enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Ranking question: role="list" container without list items (aria-required-children)

4 participants