-
Notifications
You must be signed in to change notification settings - Fork 49
Website - CodeBlock
a11y improvements docs
#2896
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
CodeBlock
a11y improvements docsCodeBlock
a11y improvements docs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
7a3f514
to
0201cbd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a couple of questions
@@ -11,6 +11,15 @@ This component uses [prism.js](https://prismjs.com/) under the hood. | |||
<C.Property @name="<[CB].Description>" @type="yielded component"> | |||
`ContentBlock::Description` yielded as contextual component (see below). | |||
</C.Property> | |||
<C.Property @name="ariaLabel" @type="string"> | |||
Accepts a localized string. The `ariaLabel` value is applied to the code block pre element. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[question] why are we saying "localized" here? we don't mention localization in any other parts o the APIs (plus it works even if it's not localized)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was matching the language used for the aria attributes in the component api of the Code Editor docs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I'll ask the team what they think of this.
@@ -11,6 +11,15 @@ This component uses [prism.js](https://prismjs.com/) under the hood. | |||
<C.Property @name="<[CB].Description>" @type="yielded component"> | |||
`ContentBlock::Description` yielded as contextual component (see below). | |||
</C.Property> | |||
<C.Property @name="ariaLabel" @type="string"> | |||
Accepts a localized string. The `ariaLabel` value is applied to the code block pre element. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I'll ask the team what they think of this.
📌 Summary
If merged, this PR would update the
CodeBlock
docs to include new aria arguments from #2879.The
ariaDescribedBy
argument was also added to theCodeEditor
docs, which was already present in the component through its extension of thehds-code-editor
modifier.🔗 External links
Jira ticket: HDS-3349
👀 Component checklist
💬 Please consider using conventional comments when reviewing this PR.