Skip to content

Post-merge-review: Fix template-require-mandatory-role-attributes: read StringLiteral .value not .original#2699

Merged
NullVoxPopuli merged 1 commit intoember-cli:masterfrom
johanrd:day_fix/template-require-mandatory-role-attributes
Apr 14, 2026
Merged

Post-merge-review: Fix template-require-mandatory-role-attributes: read StringLiteral .value not .original#2699
NullVoxPopuli merged 1 commit intoember-cli:masterfrom
johanrd:day_fix/template-require-mandatory-role-attributes

Conversation

@johanrd
Copy link
Copy Markdown
Contributor

@johanrd johanrd commented Apr 13, 2026

Summary

  • The mustache branch was dead code: GlimmerStringLiteral.original returns undefined; .value is the correct accessor
  • Required-ARIA-property errors on curly invocations like {{foo role="slider"}} were silently dropped; now reported correctly

Test plan

  • {{foo role="slider"}} → flagged (missing aria-valuenow)
  • {{foo role="checkbox"}} → flagged (missing aria-checked)

…alue not .original

The mustache branch was dead code due to an incorrect accessor on
GlimmerStringLiteral (.original returns undefined). Required-ARIA-property
errors on curly invocations like {{foo role="slider"}} were silently
dropped. Switch to .value to match the AST and add invalid-case tests.
@johanrd johanrd marked this pull request as ready for review April 13, 2026 17:20
@johanrd johanrd force-pushed the day_fix/template-require-mandatory-role-attributes branch from 8e8bb69 to 48d1c7b Compare April 13, 2026 17:39
@johanrd johanrd changed the title Fix template-require-mandatory-role-attributes: read StringLiteral .value not .original Post-merge-review: Fix template-require-mandatory-role-attributes: read StringLiteral .value not .original Apr 13, 2026
@NullVoxPopuli NullVoxPopuli merged commit 7f72f25 into ember-cli:master Apr 14, 2026
19 of 20 checks passed
@johanrd johanrd deleted the day_fix/template-require-mandatory-role-attributes branch April 14, 2026 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants