Skip to content

fix: SITES-24510 The ARIA design pattern for grid has been partially implemented - #412

Merged
StoianLucian merged 4 commits into
adobe:masterfrom
mariang-ens:issue/SITES-24510
Jun 9, 2026
Merged

StoianLucian merged 4 commits into
adobe:masterfrom
mariang-ens:issue/SITES-24510

Conversation

@mariang-ens

Copy link
Copy Markdown
Contributor

JIRA Ticket: https://jira.corp.adobe.com/browse/SITES-24510

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.

@surajmall

Copy link
Copy Markdown

build (12.x) failed on the latest commit. Fix the build (and confirm whether failures are from these changes or the environment) before merge.

expect(el.parentElement.getAttribute('aria-label')).to.equal('Masonry Label', 'Masonry parent element should receive same aria-label as Masonry');
expect(el.parentElement.getAttribute('aria-labelledby')).to.equal('Masonry Labelledby', 'Masonry parent element should receive same aria-labelledby as Masonry');

expect(el.getAttribute('role')).to.equal('row', '<coral-masonry> should have role="row"');

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

always gets role="row" when aria grid is on, even when aria-rowcount > 1. All gridcell children can have different aria-rowindex values (2, 3, …) while sitting under one row in the accessibility tree.

That conflicts with the usual grid → row → gridcell model and is why the parallel fix in coralui-component-masonry PR #71 switches the masonry wrapper to role="presentation" when there is more than one visual band.

Recommendation: When spatial.rowcount > 1, use role="presentation" on masonry (and drop aria-rowindex on the wrapper), or introduce real row containers. Keep role="row" only for a single visual row.

this.parentElement.setAttribute('aria-colcount', String(spatial.colcount));
this.parentElement.setAttribute('aria-rowcount', String(spatial.rowcount));
} else if (this.items.length > 0) {
this.parentElement.setAttribute('aria-colcount', String(this.items.length));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Tests only cover one visual row (three columns). Add a case similar to the corp masonry PR: narrow width, multiple tiers, expect aria-rowcount >= 2 and correct per-item indices (and presentation/row behavior if you adopt #2).


@private
*/
_getSpatialAriaGridMeta() {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If any placed item lacks columnIndex / itemIndex, the whole meta call returns null and parent falls back to aria-colcount = items.length, aria-rowcount = 1, while individual items may still get mixed spatial vs linear indices. Consider partial application or documenting that all items must have layout data after _doLayout.


if (activateAriaGrid === ariaGrid.ON) {
this.parentElement.setAttribute('aria-colcount', this.items.length);
const spatial = this._getSpatialAriaGridMeta();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

_getSpatialAriaGridMeta skips ignored and placeholder items, but _updateAriaRoleForItems walks all items. Ignored items without valid ld get the linear fallback path and can receive duplicate or misleading aria-colindex values.

Recommendation: Skip placeholders (_placeholder) and ignored items for gridcell roles, or clear their grid attributes explicitly.

Marian Giurge and others added 2 commits May 28, 2026 17:32
Use presentation role when aria-rowcount > 1, partial spatial meta for
parent counts, skip ignored/placeholder items for gridcell ARIA, and add
narrow multi-row tests with per-item index assertions.

Co-authored-by: Cursor <cursoragent@cursor.com>
Presentation on the wrapper breaks grid → row → gridcell in axe when the
example uses multiple visual rows. Spatial aria-rowindex/colindex and
parent aria-rowcount still convey wrapped layout.

Co-authored-by: Cursor <cursoragent@cursor.com>
@mariang-ens
mariang-ens requested a review from surajmall May 29, 2026 06:40

@surajmall surajmall 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.

LGTM

@StoianLucian
StoianLucian merged commit 5ff9397 into adobe:master Jun 9, 2026
3 checks passed
@mariang-ens mariang-ens changed the title SITES-24510 The ARIA design pattern for grid has been partially implemented fix: SITES-24510 The ARIA design pattern for grid has been partially implemented Jun 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants