Skip to content

Commit 9b33bad

Browse files
georgimkvgeorgimkv
authored
fix(ui5-card): refactor header to avoid nesting interactive elements (#5301)
The header now always has role="group". An inner element inside the header is now receiving the focus, while the focus outline is still drawn on the header element. This helps the case when the header is interactive, to avoid nesting a button element within a role="button" div. Adopted approach from UI5/openui5@67a5f79 Adjusted the status text style slightly to match the latest design spec.
1 parent 03269f4 commit 9b33bad

File tree

5 files changed

+91
-65
lines changed

5 files changed

+91
-65
lines changed

‎packages/main/src/CardHeader.hbs

Lines changed: 44 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,42 +1,57 @@
11
<div
2-
class="{{classes}}"
3-
@click="{{_headerClick}}"
4-
@keydown="{{_headerKeydown}}"
5-
@keyup="{{_headerKeyup}}"
6-
role="{{ariaHeaderRole}}"
7-
aria-labelledby="{{ariaLabelledByHeader}}"
8-
aria-level="{{_ariaLevel}}"
9-
aria-roledescription="{{ariaCardHeaderRoleDescription}}"
10-
tabindex="0"
112
id="{{_id}}--header"
3+
class="{{classes}}"
4+
role="group"
5+
aria-roledescription="{{ariaRoleDescription}}"
6+
@click="{{_click}}"
7+
@keydown="{{_keydown}}"
8+
@keyup="{{_keyup}}"
129
>
13-
{{#if hasAvatar}}
14-
<div id="{{_id}}-avatar" class="ui5-card-header-avatar" aria-label="{{ariaCardAvatarLabel}}">
15-
<slot name="avatar"></slot>
16-
</div>
17-
{{/if}}
10+
<div
11+
class="ui5-card-header-focusable-element"
12+
aria-labelledby="{{ariaLabelledBy}}"
13+
role="{{ariaRoleFocusableElement}}"
14+
data-sap-focus-ref
15+
tabindex="0"
16+
>
17+
{{#if hasAvatar}}
18+
<div id="{{_id}}-avatar" class="ui5-card-header-avatar" aria-label="{{ariaCardAvatarLabel}}">
19+
<slot name="avatar"></slot>
20+
</div>
21+
{{/if}}
1822

19-
<div class="ui5-card-header-text">
20-
<div class="ui5-card-header-first-line">
21-
{{#if titleText}}
22-
<div id="{{_id}}-title" class="ui5-card-header-title" part="title">{{titleText}}</div>
23-
{{/if}}
23+
<div class="ui5-card-header-text">
24+
<div class="ui5-card-header-first-line">
25+
{{#if titleText}}
26+
<div
27+
id="{{_id}}-title"
28+
class="ui5-card-header-title"
29+
part="title"
30+
aria-role="heading"
31+
aria-level="3"
32+
>{{titleText}}</div>
33+
{{/if}}
2434

25-
{{#if status}}
26-
<div class="ui5-card-header-status">
27-
<span id="{{_id}}-status" part="status" dir="auto">{{status}}</span>
28-
</div>
29-
{{/if}}
35+
{{#if status}}
36+
<div class="ui5-card-header-status">
37+
<span id="{{_id}}-status" part="status" dir="auto">{{status}}</span>
38+
</div>
39+
{{/if}}
3040

31-
</div>
41+
</div>
3242

33-
{{#if subtitleText}}
34-
<div id="{{_id}}-subtitle" class="ui5-card-header-subtitle" part="subtitle">{{subtitleText}}</div>
35-
{{/if}}
43+
{{#if subtitleText}}
44+
<div id="{{_id}}-subtitle" class="ui5-card-header-subtitle" part="subtitle">{{subtitleText}}</div>
45+
{{/if}}
46+
</div>
3647
</div>
3748

3849
{{#if hasAction}}
39-
<div class="ui5-card-header-action">
50+
<div
51+
class="ui5-card-header-action"
52+
@focusin="{{_actionsFocusin}}"
53+
@focusout="{{_actionsFocusout}}"
54+
>
4055
<slot name="action"></slot>
4156
</div>
4257
{{/if}}

‎packages/main/src/CardHeader.js

Lines changed: 21 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import { getI18nBundle } from "@ui5/webcomponents-base/dist/i18nBundle.js";
44
import { isSpace, isEnter } from "@ui5/webcomponents-base/dist/Keys.js";
55
import Integer from "@ui5/webcomponents-base/dist/types/Integer.js";
66
import CardHeaderTemplate from "./generated/templates/CardHeaderTemplate.lit.js";
7-
import Icon from "./Icon.js";
87

98
import {
109
AVATAR_TOOLTIP,
@@ -176,27 +175,23 @@ class CardHeader extends UI5Element {
176175
};
177176
}
178177

179-
get ariaHeaderRole() {
180-
return this.interactive ? "button" : "heading";
178+
get _root() {
179+
return this.shadowRoot.querySelector(".ui5-card-header");
181180
}
182181

183-
get _ariaLevel() {
184-
if (this.interactive) {
185-
return undefined;
186-
}
187-
188-
return this.ariaLevel;
182+
get ariaRoleDescription() {
183+
return this.interactive ? CardHeader.i18nBundle.getText(ARIA_ROLEDESCRIPTION_INTERACTIVE_CARD_HEADER) : CardHeader.i18nBundle.getText(ARIA_ROLEDESCRIPTION_CARD_HEADER);
189184
}
190185

191-
get ariaCardHeaderRoleDescription() {
192-
return this.interactive ? CardHeader.i18nBundle.getText(ARIA_ROLEDESCRIPTION_INTERACTIVE_CARD_HEADER) : CardHeader.i18nBundle.getText(ARIA_ROLEDESCRIPTION_CARD_HEADER);
186+
get ariaRoleFocusableElement() {
187+
return this.interactive ? "button" : null;
193188
}
194189

195190
get ariaCardAvatarLabel() {
196191
return CardHeader.i18nBundle.getText(AVATAR_TOOLTIP);
197192
}
198193

199-
get ariaLabelledByHeader() {
194+
get ariaLabelledBy() {
200195
const labels = [];
201196

202197
if (this.titleText) {
@@ -226,25 +221,29 @@ class CardHeader extends UI5Element {
226221
return !!this.action.length;
227222
}
228223

229-
static get dependencies() {
230-
return [Icon];
231-
}
232-
233224
static async onDefine() {
234225
CardHeader.i18nBundle = await getI18nBundle("@ui5/webcomponents");
235226
}
236227

237-
_headerClick(event) {
228+
_actionsFocusin() {
229+
this._root.classList.add("ui5-card-header-hide-focus");
230+
}
231+
232+
_actionsFocusout() {
233+
this._root.classList.remove("ui5-card-header-hide-focus");
234+
}
235+
236+
_click(event) {
238237
// prevents the native browser "click" event from firing
239238
event.stopImmediatePropagation();
240239

241-
if (this.interactive && event.target === event.currentTarget) {
240+
if (this.interactive && this._root.contains(event.target)) {
242241
this.fireEvent("click");
243242
}
244243
}
245244

246-
_headerKeydown(event) {
247-
if (!this.interactive || event.target !== event.currentTarget) {
245+
_keydown(event) {
246+
if (!this.interactive || !this._root.contains(event.target)) {
248247
return;
249248
}
250249

@@ -263,8 +262,8 @@ class CardHeader extends UI5Element {
263262
}
264263
}
265264

266-
_headerKeyup(event) {
267-
if (!this.interactive || event.target !== event.currentTarget) {
265+
_keyup(event) {
266+
if (!this.interactive || !this._root.contains(event.target)) {
268267
return;
269268
}
270269

‎packages/main/src/themes/CardHeader.css

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
align-items: flex-start;
1313
}
1414

15-
.ui5-card-header:focus:before {
15+
.ui5-card-header:not(.ui5-card-header-hide-focus):focus-within:before {
1616
outline: none;
1717
content: "";
1818
position: absolute;
@@ -28,6 +28,16 @@
2828
border-bottom-right-radius: var(--_ui5_card_header_focus_bottom_radius);
2929
}
3030

31+
.ui5-card-header-focusable-element:focus {
32+
outline: none;
33+
}
34+
35+
.ui5-card-header-focusable-element {
36+
display: inherit;
37+
align-items: inherit;
38+
flex: 1;
39+
}
40+
3141
.ui5-card-header.ui5-card-header--interactive:hover {
3242
cursor: pointer;
3343
background: var(--_ui5_card_header_hover_bg);
@@ -86,7 +96,8 @@
8696
overflow: hidden;
8797
white-space: nowrap;
8898
vertical-align: middle;
89-
margin-left: 1rem;
99+
margin-inline-start: 1rem;
100+
margin-block-start: 0.125rem;
90101
}
91102

92103
.ui5-card-header .ui5-card-header-text .ui5-card-header-title {

‎packages/main/test/pages/styles/Card.css

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,23 @@
11
.myCard {
2-
max-width: 300px;
2+
max-width: 300px;
33
}
44

55
.card1auto {
6-
background-color: var(--sapBackgroundColor);
6+
background-color: var(--sapBackgroundColor);
77
}
88

99
.myInput {
10-
margin: 0.5rem 1rem;
10+
margin: 0.5rem 1rem;
1111
}
1212

1313
.myContent {
14-
margin: 0.5rem 1rem;
14+
margin: 0.5rem 1rem;
1515
}
1616

1717
.myHeader {
18-
margin: 1rem;
18+
margin: 1rem;
1919
}
2020

2121
.myTextContent {
22-
padding: 0 1rem 1rem 1rem;
23-
}
22+
padding: 0 1rem 1rem 1rem;
23+
}

‎packages/main/test/specs/Card.spec.js

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -75,10 +75,11 @@ describe("Card general interaction", () => {
7575
const cardHeader = await $("#card2").$("ui5-card-header");
7676

7777
// Default value
78-
assert.strictEqual(await cardHeader.shadow$(".ui5-card-header").getAttribute("aria-level"), "3");
78+
assert.strictEqual(await cardHeader.shadow$(".ui5-card-header .ui5-card-header-title").getAttribute("aria-level"), "3");
7979

80-
await cardHeader.setAttribute("aria-level", 4);
81-
assert.strictEqual(await cardHeader.shadow$(".ui5-card-header").getAttribute("aria-level"), "4");
80+
const cardHeaderTitle = await cardHeader.shadow$(".ui5-card-header .ui5-card-header-title");
81+
await cardHeaderTitle.setAttribute("aria-level", 4);
82+
assert.strictEqual(await cardHeader.shadow$(".ui5-card-header .ui5-card-header-title").getAttribute("aria-level"), "4");
8283
});
8384
});
8485

@@ -88,8 +89,8 @@ describe("CardHeader", () => {
8889
});
8990

9091
it("tests header aria-labelledby", async () => {
91-
const header = await browser.$("#header").shadow$(".ui5-card-header");
92-
const header2 = await browser.$("#header2").shadow$(".ui5-card-header");
92+
const header = await browser.$("#header").shadow$(".ui5-card-header .ui5-card-header-focusable-element");
93+
const header2 = await browser.$("#header2").shadow$(".ui5-card-header .ui5-card-header-focusable-element");
9394
const headerId = await browser.$("#header").getProperty("_id");
9495
const headerId2 = await browser.$("#header2").getProperty("_id");
9596
const EXPECTED_ARIA_LABELLEDBY_HEADER = `${headerId}-title ${headerId}-subtitle ${headerId}-status`;

0 commit comments

Comments
 (0)