New documentation website#3217
Conversation
| </dd> | ||
| </dl> | ||
|
|
||
| <p class="summary">{{ component.summary }}</p> |
There was a problem hiding this comment.
component.summary is rendered without | safe, but the manifest processing converts backticks to <code> tags (and summaries may already contain HTML). This will likely be escaped by Nunjucks and render the tags as text. Consider rendering it as {{ component.summary | safe }} (and optionally only output the <p> when a summary exists).
| const { OpenIssueCount: OpenIssueCountClass } = await import( | ||
| '@sl-design-system/doc-components/open-issue-count/open-issue-count' | ||
| ); |
There was a problem hiding this comment.
The dynamic import path does not match the package exports map in docs/components/package.json, which only exports subpaths ending in .js (e.g. ./open-issue-count/open-issue-count.js). Importing @sl-design-system/doc-components/open-issue-count/open-issue-count will fail under Node/package-exports resolution; update the import to include the .js suffix.
| "repository": { | ||
| "type": "git", | ||
| "url": "https://github.com/sl-design-system/components.git", | ||
| "url": "https://github.com/sl-design-system/components", | ||
| "directory": "packages/components/tooltip" | ||
| }, |
There was a problem hiding this comment.
The repository.directory points to packages/components/tooltip, but this package is time-field. This metadata ends up incorrect in the generated docs/links. Update it to packages/components/time-field.
🕸 Preview deploys |
a11ymiko
left a comment
a11ymiko
left a comment
There was a problem hiding this comment.
Link with GitHub logo that takes user to our components repo does not have accessible name:
<a href="https://github.com/sl-design-system/components" target="_blank" rel="noopener noreferrer">
<sl-icon name="fab-github" aria-hidden="true"></sl-icon>
</a>
Fix recommendation:
aria-label='GitHub repository' or aria-label='SL Design system on GitHub' should be added to <a> so it would be announced with proper a11y name by screen reader
a11ymiko
left a comment
a11ymiko
left a comment
There was a problem hiding this comment.
Switch used to change mode between light and dark currently have name that may be misunderstood by users. Screen reader announce this component as "Switch between light and dark mode, ON/OFF, switch". For some users information "ON/OFF" may be understood as 'switch is active or switch is dimmed' more than 'light mode/dark mode is selected'.
Fix recommendation:
Changing a11y name of that switch to 'Light mode' would result in this component being announced as 'Light mode, ON/OFF, switch' that is more clear about what mode is currently selected and as shorter name of component it will also take less time for user to get to know this component.
|
|
||
| override render(): TemplateResult { | ||
| return html` | ||
| <nav aria-label="Site navigation" role="tree"> |
There was a problem hiding this comment.
It's better idea to remove role='tree' from <nav> element as:
- it's more usable for assistive technologies users when it's role is 'navigation' than 'tree'. there are landmark shortcuts for navigation.
- role 'navigation' better explains the purpose of that element than role
tree - role
treehas additional requirements when it comes to it's children that are currently not met but are not required for rolenavigation.
No description provided.