Skip to content

fix: role for aside element when nested within sectioning content#30

Merged
drwpow merged 2 commits intodrwpow:mainfrom
jlp-craigmorten:fix-aside-inside-landmark-role
Jan 25, 2025
Merged

fix: role for aside element when nested within sectioning content#30
drwpow merged 2 commits intodrwpow:mainfrom
jlp-craigmorten:fix-aside-inside-landmark-role

Conversation

@jlp-craigmorten
Copy link
Contributor

@jlp-craigmorten jlp-craigmorten commented Jan 25, 2025

Changes

Relates to #25

Updates the role returned for a aside element to be either:

  1. complementary when the element is named;
  2. complementary when the element is not nested within sectioning content;
  3. generic otherwise.

This detail is currently missing from the HTML-ARIA spec, but is describe in the sibling HTML-AAM spec:

How to Review

Mirrors the existing functionality and tests for combination of the img element which has similar named behaviours, and the footer which has similar scoping/nesting behaviours.

Undecided whether would be cleaner to scope the calculateAccessibleName() check within the new getAsideRole() - perhaps feels a bit awkward to have a getAsideRole() that holds partial logic for getting the role, the rest living elsewhere 🤔 wdyt?

Starting to see patterns arising, e.g. attributes['aria-label'] || attributes['aria-labelledby'] repeated, but perhaps still borderline rule or 3 to warrant refactoring at this stage?

Checklist

  • Unit tests updated

@changeset-bot
Copy link

changeset-bot bot commented Jan 25, 2025

🦋 Changeset detected

Latest commit: 9d45089

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
html-aria Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@jlp-craigmorten jlp-craigmorten marked this pull request as ready for review January 25, 2025 12:30
@jlp-craigmorten jlp-craigmorten changed the title fix: role for aside element when nested within landmark fix: role for aside element when nested within sectioning content Jan 25, 2025
@drwpow
Copy link
Owner

drwpow commented Jan 25, 2025

Undecided whether would be cleaner to scope the calculateAccessibleName() check within the new getAsideRole() - perhaps feels a bit awkward to have a getAsideRole() that holds partial logic for getting the role, the rest living elsewhere 🤔 wdyt?

I don’t mind this. I see this as more of a “private” API that can remain undocumented. I think just to be safe I’d err on the side of not exporting it from the entry so we could always refactor later if needed.

Starting to see patterns arising, e.g. attributes['aria-label'] || attributes['aria-labelledby'] repeated, but perhaps still borderline rule or 3 to warrant refactoring at this stage?

Ha I usually wait till way past 3 to refactor, until it’s fairly obvious.

@drwpow drwpow merged commit b6219e8 into drwpow:main Jan 25, 2025
6 checks passed
@github-actions github-actions bot mentioned this pull request Jan 25, 2025
@jlp-craigmorten jlp-craigmorten deleted the fix-aside-inside-landmark-role branch January 25, 2025 15:28
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.

2 participants