feat(documentation): add accessibility reference relationship stories and demo components #6123
Conversation
|
myrta2302
changed the title
…ttps://github.com/swisspost/design-system into add-for-label-stories-in-the-accessibility-section
...ture-and-semantics/reference-crossing-the-shadowdom/aria-labelledby/aria-labelledby.docs.mdx
Outdated
Show resolved
Hide resolved
...ture-and-semantics/reference-crossing-the-shadowdom/aria-labelledby/aria-labelledby.docs.mdx
Outdated
Show resolved
Hide resolved
...antics/reference-crossing-the-shadowdom/aria-labelledby/examples/defaultExample2.sample.html
Show resolved
Hide resolved
|
oliverschuerch
left a comment
oliverschuerch
left a comment
There was a problem hiding this comment.
The preview URL is not working.
|
I merged the conflicts to main and it is working now. |
oliverschuerch
left a comment
oliverschuerch
left a comment
There was a problem hiding this comment.
Currently, the guidelines are always visible, no matther which mode the docs are rendered in (development | production).
Shouldn't we hide the guidelines in production mode, or is the content also relevant for our users?
The docs pages aria-describedby and aria-labelledby are almost the same (compared by page structur and content).
Please update the CRs requested for one also on the other.
During the review, I asked my self if the examples are just to specific? At least it's a lot of repeating information on all of the example pages.
I think it could be beneficial to add a page with general information about, how different HTML attributes or JS properties can cross the DOM borders or not. And then additionally only document exceptions.
e.g.
- light -> light ✔️
- light -> shadow ❌
- shadow -> light ❌
- shadow -> slotted ❌
- shadow A -> shadow A ✔️
- shadow B -> shadow B ❌
- ...
if we'd do that and add examples, most of the content of the other pages could become obsolete.
Since you've already put a lot of effort into it, I'll leave it up to you if you want to do that, ask the team about it or completely ignore this idea.
| } | ||
| } | ||
|
|
||
| const ariaPropertyName = isLabelled ? 'ariaLabelledByElements' : 'ariaDescribedByElements'; |
There was a problem hiding this comment.
It's a so called "good practice" to define variables at the beginning of a function. Please move this to line 32.
| attributeChangedCallback(name: string, _oldValue: string, newValue: string) { | ||
| if (name === 'workaround') this.workaround = newValue; | ||
| if (name === 'arialabelledby-id') this.arialabelledbyId = newValue; | ||
| if (name === 'ariadescribedby-id') this.ariadescribedbyId = newValue; | ||
| if (name === 'button-version') this.buttonVersion = newValue as '1' | '2' | '3'; | ||
| this.render(); | ||
| } |
There was a problem hiding this comment.
Since only the observed attributes will execute this callback, you could easily make it generic.
attributeChangedCallback(name: string, _oldValue: string, newValue: string) {
const member = name.replace(/-./g, m => m[1].toUpperCase());
this[member] = newValue;
this.render();
}There was a problem hiding this comment.
To be inline with the given file naming, I suggest to rename all the sample files:
- Write names in kebab-case
- Add context for the html samples and only use numbers/letters if there is no better word to describe the context
(e.g.default-example-lightdom.sample.htmlanddefault-example-shadowdom.sample.htmlornot-working-example-1.sample.htmlandnot-working-example-2.sample.html).
| <p> | ||
| Below is a working example of an `aria-describedby`/`id` relationship within the shadowDom of a | ||
| custom button component. You can <b>test it with NVDA</b> to confirm the correct description | ||
| announcement. | ||
| </p> | ||
| <Canvas of={CCR.ExampleHTML} sourceState="hide" /> |
There was a problem hiding this comment.
The text says it's a working shadowdom example, but the story contains a lightdom example.
| import WorkingExampleIV from './examples/workingExample-IV.sample.html?raw'; | ||
|
|
||
| <Meta of={CCR} /> | ||
|
|
There was a problem hiding this comment.
I would add something like the following as a general statement:
Note: The aria-describedby attribute is not designed to reference descriptions from external resources. As its value is one or more ids (space-separated if multiple), it must reference elements in the same DOM document.
If you find a better description about this fact, that be even better.
| <!--The aria-labelledby attribute is set automatically by the browser --> | ||
| <div ref="targetRef" role="button" tabindex="0" aria-labelledby> | ||
|
|
||
| <!-- slotted label element --> |
There was a problem hiding this comment.
| <!-- slotted label element --> |
There was a problem hiding this comment.
Keep file naming consistant.
|
|
||
| <custom-button> | ||
| ⏷ #shadow-root (open) | ||
| <input ref="targetRef" id="inputId" aria-labelledby /> |
There was a problem hiding this comment.
| <input ref="targetRef" id="inputId" aria-labelledby /> | |
| <input ref="targetRef" id="inputId" aria-labelledby /> |
| </b> | ||
| </p> | ||
|
|
||
| #### Workaround: `Element:ariaLabelledByElements` |
There was a problem hiding this comment.
This complete section reminds me almost 100% of the aria-labelledby page.
Do we need to keep both or would it be enough to say here, that you can use the ariaLabelledByElements workaround on the other page?
There was a problem hiding this comment.
Check the links. some of them did not work.
📄 Description
This PR adds documentation for the following cross-component referencing scenarios under the (private) Accessibility section:
To support these examples, a new demo-components folder has been added under
documentation/src, containing the following native web components:📝 Checklist