Skip to content

Commit e18447e

Browse files
GerganaKremenskaGerganaKremenska
committed
docs(ui5-a11y): Address review feedback on accessibility skill
- SKILL.md: drop non-standard `allowed-tools` line; rewrite topic-file paths to plain `references/X.md` to match sibling skills; expand description with WCAG / screen-reader / focus-handling keywords; rename topic 4 to "Focus & keyboard" so it matches keyboard.md. - tests/fixtures: add gap-keyboard.view.xml covering all three topic-4 violations (missing initialFocus, missing sap-ui-fastnavgroup, tabindex > 0) — this was the one topic without a gap fixture. - tests/README.md: register the new fixture and clarify that fixtures are run manually (no auto harness).
1 parent ad435fa commit e18447e

3 files changed

Lines changed: 75 additions & 23 deletions

File tree

‎plugins/ui5/skills/ui5-best-practices-accessibility/SKILL.md‎

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,20 @@
11
---
22
name: ui5-best-practices-accessibility
3-
description: >
3+
description: |
44
This skill should be used when the user asks to audit, fix, check, review, or
5-
improve accessibility, a11y, ARIA, landmarks, labeling, heading levels,
6-
keyboard shortcuts, screen reader support, invisible messaging, reading order,
7-
or touch target size in UI5 application files (views, fragments, controllers).
8-
Also use when the user creates a new UI5 view, fragment, or controller and
9-
wants it to be accessible, or asks whether a specific control (Dialog, Table,
10-
Panel, etc.) meets accessibility requirements.
11-
allowed-tools: Read Bash AskUserQuestion
5+
improve accessibility, a11y, ARIA, WCAG compliance, landmarks, labeling, heading
6+
levels, focus handling, keyboard navigation, keyboard shortcuts, screen reader
7+
support, invisible messaging, reading order, or touch / target size in UI5
8+
application files (views, fragments, controllers). Also use when the user
9+
creates a new UI5 view, fragment, or controller and wants it to be accessible,
10+
or asks whether a specific control (Dialog, Table, Panel, etc.) meets
11+
accessibility requirements.
12+
13+
Keywords: accessibility, a11y, ARIA, WCAG, screen reader, NVDA, JAWS, VoiceOver,
14+
landmark, landmarkInfo, accessibleRole, ariaLabelledBy, ariaDescribedBy,
15+
labelFor, tooltip, alt text, decorative, heading level, initialFocus, F6,
16+
fast nav, fastnavgroup, tabindex, CommandExecution, keyboard shortcut,
17+
InvisibleMessage, announce, reading order, reactiveAreaMode, target size.
1218
---
1319

1420
# Accessibility Review
@@ -46,14 +52,14 @@ API pattern and wrong/correct examples.
4652

4753
| # | Topic | What to detect | Topic file |
4854
|---|-------|---------------|------------|
49-
| 1 | Landmarks | `DynamicPage`, `Page`, `Panel`, `ObjectPage`, `FlexibleColumnLayout` missing `landmarkInfo` or `accessibleRole`; landmark role set without its corresponding label (e.g. `rootRole` without `rootLabel`) | `${CLAUDE_SKILL_DIR}/references/landmark.md` |
50-
| 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` | `${CLAUDE_SKILL_DIR}/references/labeling.md` |
51-
| 3 | Heading levels | `<Title>` without explicit `level`; heading level jumps (e.g. H1 → H3) within a view | `${CLAUDE_SKILL_DIR}/references/heading.md` |
52-
| 4 | Focus handling | `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 | `${CLAUDE_SKILL_DIR}/references/keyboard.md` |
53-
| 5 | Keyboard shortcuts | Action buttons (save, delete, etc.) using plain `press=".onX"` that should support keyboard shortcuts but have no `CommandExecution` | `${CLAUDE_SKILL_DIR}/references/shortcut.md` |
54-
| 6 | Invisible messaging | Dynamic state changes (save confirmations, errors, filter results) that are visible-only with no `InvisibleMessage.announce()` call in the handler | `${CLAUDE_SKILL_DIR}/references/invisible-message.md` |
55-
| 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 | `${CLAUDE_SKILL_DIR}/references/reading-order.md` |
56-
| 8 | Target size | `Link`, `ObjectIdentifier`, `ObjectStatus`, `ObjectNumber`, `ObjectMarker`, `ObjectAttribute` inside an interactive container without `reactiveAreaMode`; or in other dense layout without spacing | `${CLAUDE_SKILL_DIR}/references/target-size.md` |
55+
| 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` |
56+
| 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` |
57+
| 3 | Heading levels | `<Title>` without explicit `level`; heading level jumps (e.g. H1 → H3) within a view | `references/heading.md` |
58+
| 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` |
59+
| 5 | Keyboard shortcuts | Action buttons (save, delete, etc.) using plain `press=".onX"` that should support keyboard shortcuts but have no `CommandExecution` | `references/shortcut.md` |
60+
| 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` |
61+
| 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` |
62+
| 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` |
5763

5864
## Step 3 — Report
5965

‎plugins/ui5/skills/ui5-best-practices-accessibility/tests/README.md‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
# Skill Test Fixtures
22

3-
Run `/ui5-best-practices-accessibility` against each fixture and verify the output matches expectations below.
3+
These fixtures are **not** auto-run — there is no test harness, CI job, or assertion
4+
framework. They are reference cases for manually validating the skill's behaviour:
5+
invoke `/ui5-best-practices-accessibility` against each file and verify the output
6+
matches the expectations below.
47

58
## Positive fixtures — skill MUST report these gaps
69

@@ -23,6 +26,13 @@ Run `/ui5-best-practices-accessibility` against each fixture and verify the outp
2326
| `Title "Order Details"` missing `level` property | Heading levels |
2427
| Heading jump: H1 → H3 with no H2 | Heading levels |
2528

29+
### `gap-keyboard.view.xml`
30+
| Expected finding | Topic |
31+
|---|---|
32+
| `VBox#filterRegion` — distinct logical region without `sap-ui-fastnavgroup` CustomData | Focus & keyboard |
33+
| `Input` with `tabindex="3"` CustomData — value greater than 0 overrides natural reading order | Focus & keyboard |
34+
| `Dialog#confirmDialog` — multiple focusable elements and no `initialFocus` | Focus & keyboard |
35+
2636
### `gap-dialog.controller.js`
2737
| Expected finding | Topic |
2838
|---|---|
@@ -70,9 +80,3 @@ Run `/ui5-best-practices-accessibility` against each fixture and verify the outp
7080

7181
### `ok-target-size.view.xml`
7282
- `ObjectIdentifier` (`titleActive`) and `ObjectStatus` (`active`) both have `reactiveAreaMode="Overlay"` → **correct, do not flag**
73-
74-
---
75-
76-
## How to run
77-
78-
Run the `ui5-best-practices-accessibility` skill against each fixture file and verify the output matches the expected findings above.

‎plugins/ui5/skills/ui5-best-practices-accessibility/tests/fixtures/gap-keyboard.view.xml‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
<mvc:View
2+
controllerName="my.app.controller.Main"
3+
xmlns:mvc="sap.ui.core.mvc"
4+
xmlns:core="sap.ui.core"
5+
xmlns="sap.m">
6+
<App>
7+
<Page title="Order Details" id="mainPage">
8+
<content>
9+
<!-- GAP: distinct logical region (toolbar + filter strip) without sap-ui-fastnavgroup CustomData, so F6 users cannot jump to it as its own group -->
10+
<VBox id="filterRegion">
11+
<Toolbar>
12+
<Title text="Filters" level="H2"/>
13+
<ToolbarSpacer/>
14+
<Button icon="sap-icon://filter" tooltip="Apply filter"/>
15+
</Toolbar>
16+
<HBox>
17+
<Input placeholder="Customer"/>
18+
<!-- GAP: tabindex greater than 0 overrides the natural reading order -->
19+
<Input placeholder="Order number">
20+
<customData>
21+
<core:CustomData key="tabindex" value="3" writeToDom="true"/>
22+
</customData>
23+
</Input>
24+
</HBox>
25+
</VBox>
26+
27+
<!-- GAP: Dialog has multiple focusable elements but no initialFocus, so focus lands on the first focusable element (the close button) instead of the primary action -->
28+
<Dialog id="confirmDialog" title="Confirm deletion">
29+
<content>
30+
<Text text="Delete the selected order? This cannot be undone."/>
31+
</content>
32+
<beginButton>
33+
<Button id="confirmDeleteBtn" text="Delete" type="Reject" press=".onConfirmDelete"/>
34+
</beginButton>
35+
<endButton>
36+
<Button id="cancelBtn" text="Cancel" press=".onCancel"/>
37+
</endButton>
38+
</Dialog>
39+
</content>
40+
</Page>
41+
</App>
42+
</mvc:View>

0 commit comments

Comments
 (0)