-
Notifications
You must be signed in to change notification settings - Fork 9
feat(ui5): Add accessibility best practices skill #85
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
Merged
Merged
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
ad435fa
feat(ui5): Add accessibility best practices skill
nikolay-kolarov e18447e
docs(ui5-a11y): Address review feedback on accessibility skill
GerganaKremenska f8e484e
test(ui5-a11y): Add eval.json with binary assertions for fixtures
GerganaKremenska 92be973
docs(ui5-a11y): Address LilyanaOviPe review feedback
GerganaKremenska 6f27c79
docs(ui5-a11y): Address flovogt review feedback
GerganaKremenska File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
74 changes: 74 additions & 0 deletions
74
plugins/ui5/skills/ui5-best-practices-accessibility/SKILL.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| --- | ||
| name: ui5-best-practices-accessibility | ||
| description: | | ||
| This skill should be used when the user asks to audit, fix, check, review, or | ||
| improve accessibility, a11y, ARIA, WCAG compliance, landmarks, labeling, heading | ||
| levels, focus handling, keyboard navigation, keyboard shortcuts, screen reader | ||
| support, invisible messaging, reading order, or touch / target size in UI5 | ||
| application files (views, fragments, controllers). Also use when the user | ||
| creates a new UI5 view, fragment, or controller and wants it to be accessible, | ||
| or asks whether a specific control (Dialog, Table, Panel, etc.) meets | ||
| accessibility requirements. | ||
|
|
||
| Keywords: accessibility, a11y, ARIA, WCAG, screen reader, NVDA, JAWS, VoiceOver, | ||
| landmark, landmarkInfo, accessibleRole, ariaLabelledBy, ariaDescribedBy, | ||
| labelFor, tooltip, alt text, decorative, heading level, initialFocus, F6, | ||
| fast nav, fastnavgroup, tabindex, CommandExecution, keyboard shortcut, | ||
| InvisibleMessage, announce, reading order, reactiveAreaMode, target size. | ||
| --- | ||
|
|
||
| # Accessibility Review | ||
|
|
||
| Accessibility in UI5 is incorporated in two levels: framework and application. | ||
| This review supports what application developers must still provide explicitly | ||
| to improve the accessibility of their application. | ||
|
|
||
| ## Step 1 — Find the files | ||
|
|
||
| If `$ARGUMENTS` lists specific files, review only those. | ||
|
|
||
| Otherwise, discover all app source files automatically: | ||
|
|
||
| ```! | ||
| find . \( -name "*.view.xml" -o -name "*.fragment.xml" -o -name "*.controller.js" \) \ | ||
| -not -path "*/node_modules/*" \ | ||
| -not -path "*/dist/*" \ | ||
| -not -path "*/test/*" \ | ||
| -not -path "*/resources/*" \ | ||
| | sort | ||
| ``` | ||
|
|
||
| If more than 15 files are found, use `AskUserQuestion` to let the user choose: | ||
| - Review everything (may take a moment) | ||
| - Focus on a specific folder or area | ||
|
|
||
| Read each file in scope. | ||
|
|
||
| ## Step 2 — Review | ||
|
|
||
| Check the code against the eight topics below. For each topic where you find a gap, | ||
| **read the corresponding topic file before writing the fix** — it contains the correct | ||
| API pattern and wrong/correct examples. | ||
|
|
||
| | # | Topic | What to detect | Topic file | | ||
| |---|-------|---------------|------------| | ||
| | 1 | Landmarks | `DynamicPage`, `Page`, `Panel`, `ObjectPage`, `FlexibleColumnLayout` missing `landmarkInfo` or `accessibleRole`; landmark role set without its corresponding label (e.g. `rootRole` without `rootLabel`) | `references/landmark.md` | | ||
| | 2 | Labeling | Inputs without `<Label labelFor>` (except inside `SimpleForm`); Tables without `ariaLabelledBy`; icon-only `Button` without `tooltip`; standalone `Icon` without `alt` and not marked `decorative`; `Image` with `decorative=false` and no `alt`; `Dialog` with `showHeader:false` and no `ariaLabelledBy` | `references/labeling.md` | | ||
| | 3 | Heading levels | `<Title>` without explicit `level`; heading level jumps (e.g. H1 → H3) within a view | `references/heading.md` | | ||
| | 4 | Focus & keyboard | `Dialog` or `Popover` without `initialFocus` when a specific starting element is required; larger composite areas that act as distinct logical regions and need a `sap-ui-fastnavgroup` `CustomData` entry; `tabindex` values greater than 0 in rendered HTML | `references/keyboard.md` | | ||
| | 5 | Keyboard shortcuts | Action buttons (save, delete, etc.) using plain `press=".onX"` that should support keyboard shortcuts but have no `CommandExecution` | `references/shortcut.md` | | ||
| | 6 | Invisible messaging | Dynamic state changes (save confirmations, errors, filter results) that are visible-only with no `InvisibleMessage.announce()` call in the handler | `references/invisible-message.md` | | ||
| | 7 | Reading order | Controls visually reordered via CSS/layout but out of sequence in XML; `ariaDescribedBy` pointing to IDs that appear later in the DOM | `references/reading-order.md` | | ||
| | 8 | Target size | `Link`, `ObjectIdentifier`, `ObjectStatus`, `ObjectNumber`, `ObjectMarker`, `ObjectAttribute` inside an interactive container without `reactiveAreaMode`; or in other dense layout without spacing | `references/target-size.md` | | ||
|
|
||
| ## Step 3 — Report | ||
|
|
||
| For each gap found: | ||
|
|
||
| **Issue**: one line — names the control and the missing property/association | ||
| **Impact**: `critical` (blocks AT users entirely) | `serious` (significant barrier) | `moderate` (partial barrier) | `minor` (best practice, low direct impact) | ||
| **Why**: one sentence on user impact | ||
| **Fix**: minimal corrected XML or JS snippet (only the changed part) | ||
|
|
||
| Group findings by topic, critical/serious first within each group. End with a summary count by impact level. | ||
| If no gaps are found, say so. |
72 changes: 72 additions & 0 deletions
72
plugins/ui5/skills/ui5-best-practices-accessibility/references/heading.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| # Heading Levels | ||
|
|
||
| Heading hierarchy lets screen reader users scan the page structure and jump between | ||
| sections. Missing or skipped heading levels break this navigation. | ||
|
|
||
| **Rule: every `<Title>` used as a heading must have an explicit `level` property set.** | ||
|
|
||
| **Wrong:** | ||
| ```xml | ||
| <Title text="Order Details"/> | ||
|
flovogt marked this conversation as resolved.
Outdated
|
||
| ``` | ||
|
|
||
| **Correct:** | ||
| ```xml | ||
| <Title level="H1" text="Order Details"/> <!-- page title --> | ||
| <Title level="H2" text="Line Items"/> <!-- section header --> | ||
| <Title level="H3" text="Item Notes"/> <!-- subsection --> | ||
| ``` | ||
|
|
||
| ## Heading hierarchy rules | ||
|
|
||
| - Each page should have exactly one `H1` — the page title. | ||
| - Section headers = `H2`, subsection headers = `H3`, and so on. | ||
| - **Never skip levels** — H1 → H3 with no H2 breaks the document outline and | ||
| confuses AT users navigating by heading. | ||
|
|
||
| ## `sap.m.Dialog` heading level | ||
|
|
||
| When using the `title` property, the framework renders it as `<h1>` automatically — no extra configuration needed. When using `customHeader` or `showHeader: false`, set `level: TitleLevel.H1` explicitly on the `Title` control. | ||
|
|
||
| **Wrong:** | ||
| ```xml | ||
| <Dialog> | ||
| <customHeader> | ||
| <Toolbar> | ||
| <Title id="dlgTitle" text="Confirm Deletion"/> | ||
| </Toolbar> | ||
| </customHeader> | ||
| </Dialog> | ||
| ``` | ||
|
|
||
| **Correct:** | ||
| ```xml | ||
| <Dialog ariaLabelledBy="dlgTitle"> | ||
| <customHeader> | ||
| <Toolbar> | ||
| <Title id="dlgTitle" text="Confirm Deletion" level="H1"/> | ||
| </Toolbar> | ||
| </customHeader> | ||
| </Dialog> | ||
| ``` | ||
|
|
||
| ## `sap.m.Panel` heading level | ||
|
|
||
| A `Panel` with `headerText` renders that text as a heading. If the heading level needs | ||
| to be explicitly controlled, replace `headerText` with a `headerToolbar` aggregation | ||
| containing a `Title` with an explicit `level`: | ||
|
|
||
| ```xml | ||
| <Panel accessibleRole="Region"> | ||
| <headerToolbar> | ||
| <Toolbar> | ||
| <Title level="H3" text="Shipping Details"/> | ||
| </Toolbar> | ||
| </headerToolbar> | ||
| ... | ||
| </Panel> | ||
| ``` | ||
|
|
||
| ## Available values | ||
|
|
||
| `H1` · `H2` · `H3` · `H4` · `H5` · `H6` · `Auto` (avoid — use explicit levels) | ||
62 changes: 62 additions & 0 deletions
62
...ins/ui5/skills/ui5-best-practices-accessibility/references/invisible-message.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| # Invisible Messaging | ||
|
|
||
| Use `sap.ui.core.InvisibleMessage` to announce dynamic state changes to screen reader | ||
| users who would otherwise miss purely visual updates (badge counts, success banners, | ||
| loading indicators, filter results). | ||
|
|
||
| ## When not to use | ||
|
|
||
| - Do not provide information exclusively for AT users — screen reader users should not receive content that sighted users cannot access | ||
| - Do not use it to hide long texts — if the information matters, show it visibly | ||
|
flovogt marked this conversation as resolved.
Outdated
|
||
|
|
||
| ## Dynamic announcements — `InvisibleMessage` | ||
|
|
||
| ```js | ||
| sap.ui.define([ | ||
| "sap/ui/core/mvc/Controller", | ||
| "sap/ui/core/InvisibleMessage", | ||
| "sap/ui/core/library" | ||
| ], function(Controller, InvisibleMessage, library) { | ||
| "use strict"; | ||
|
|
||
| var InvisibleMessageMode = library.InvisibleMessageMode; | ||
|
|
||
| return Controller.extend("my.app.Controller", { | ||
| onInit: function () { | ||
| this.oIM = InvisibleMessage.getInstance(); | ||
| }, | ||
|
|
||
| onDeleteItems: function () { | ||
| // ... perform deletion ... | ||
| this.oIM.announce("3 items deleted", InvisibleMessageMode.Polite); | ||
| }, | ||
|
|
||
| onSubmitError: function () { | ||
| // ... handle error ... | ||
| this.oIM.announce( | ||
| "Submission failed. Please check required fields.", | ||
| InvisibleMessageMode.Assertive | ||
| ); | ||
| } | ||
| }); | ||
| }); | ||
| ``` | ||
|
|
||
| | Mode | Behavior | When to use | | ||
| |---|---|---| | ||
| | `Polite` | Waits for a pause in current speech | Status updates, counts, non-urgent feedback | | ||
| | `Assertive` | Interrupts current speech immediately | Errors, warnings, critical state changes | | ||
|
|
||
| ## Static ARIA references — `core:InvisibleText` | ||
|
|
||
| Use when you need a hidden text node that other controls reference via `ariaLabelledBy` | ||
| or `ariaDescribedBy`, and a visible `<Label>` is not suitable: | ||
|
|
||
| ```xml | ||
| <core:InvisibleText id="postalLabel" text="Postal code"/> | ||
| <core:InvisibleText id="cityLabel" text="City"/> | ||
| <Input ariaLabelledBy="postalLabel" value="12345" fieldWidth="35%"/> | ||
| <Input ariaLabelledBy="cityLabel" value="Sofia" fieldWidth="35%"/> | ||
| ``` | ||
|
|
||
| Place `InvisibleText` nodes **before** the controls that reference them. | ||
68 changes: 68 additions & 0 deletions
68
plugins/ui5/skills/ui5-best-practices-accessibility/references/keyboard.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| # Focus Handling and Keyboard Navigation | ||
|
|
||
| ## Initial focus — `initialFocus` | ||
|
|
||
| When a Dialog or Popover opens, set which element receives focus. Without this, focus lands | ||
| on the first focusable element, which may not be the right starting point for the task. | ||
|
|
||
| ```xml | ||
| <Popover title="Product Details" initialFocus="firstActionBtn"> | ||
| <content> | ||
| <VBox> | ||
| <Text text="Notebook Basic 15"/> | ||
| <Button id="firstActionBtn" text="Add to Cart"/> | ||
| </VBox> | ||
| </content> | ||
| </Popover> | ||
| ``` | ||
|
|
||
| Same attribute on `<Dialog initialFocus="elementId">`. | ||
|
|
||
| ## F6 fast navigation | ||
|
|
||
| F6 / Shift+F6 lets users jump between logical groups. Some standard containers create | ||
| F6 groups automatically — for example, `sap.m.Panel` (the whole panel is one group) | ||
| and `sap.uxap.ObjectPageSection` (each section is a separate group). | ||
|
|
||
| **Adding or removing a custom area from the F6 chain** | ||
|
|
||
| Standard containers create their own F6 groups automatically. Groups can also be nested — | ||
| pressing F6 inside a nested group moves focus to the next group at that level; if none | ||
| exists, focus moves up to the parent group. | ||
|
|
||
| To add a custom area as an F6 group, use the `sap-ui-fastnavgroup` key via `CustomData`: | ||
|
|
||
| ```xml | ||
| <!-- XML view --> | ||
| <VBox> | ||
| <customData> | ||
| <core:CustomData key="sap-ui-fastnavgroup" value="true" writeToDom="true"/> | ||
| </customData> | ||
| </VBox> | ||
| ``` | ||
|
|
||
| ```js | ||
| // Controller / JS | ||
| oControl.data("sap-ui-fastnavgroup", "true", true /* writeToDom */); | ||
| ``` | ||
|
|
||
| To remove a group that a control defines by default, set the value to `"false"`: | ||
|
|
||
| ```js | ||
| oControl.data("sap-ui-fastnavgroup", "false", true /* writeToDom */); | ||
| ``` | ||
|
|
||
| ## Custom interactive elements — `tabindex` | ||
|
|
||
| Native HTML elements (button, input, a) are focusable by default. Custom elements | ||
| rendered with a non-interactive tag need explicit `tabindex`: | ||
|
|
||
| ```html | ||
| <!-- Focusable custom widget --> | ||
| <div role="combobox" tabindex="0" ...> ... </div> | ||
|
|
||
| <!-- Remove from tab order but keep programmatically focusable --> | ||
| <div tabindex="-1" ...> ... </div> | ||
| ``` | ||
|
|
||
| Avoid `tabindex` values greater than 0 — they override the natural reading order. |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.