Skip to content

Commit ed26b90

Browse files
TeodorTaushanovTeodorTaushanov
authored
fix(ui5-popover, ui5-dialog): fix finding keyboard focusable scroll containers (#10891)
* chore: add test * fix(ui5-popover, ui5-dialog): fix focusing keyboard focusable scrollers * chore: fix content outline styles * chore: add tests
1 parent 3c3d641 commit ed26b90

File tree

4 files changed

+149
-0
lines changed

4 files changed

+149
-0
lines changed

‎packages/base/src/util/FocusableElements.ts

+14
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,21 @@
11
import isElementHidden from "./isElementHidden.js";
22
import isElementClickable from "./isElementClickable.js";
33
import { instanceOfUI5Element } from "../UI5Element.js";
4+
import { isSafari } from "../Device.js";
45

56
type FocusableElementPromise = Promise<HTMLElement | null>;
67

78
const isFocusTrap = (el: HTMLElement) => {
89
return el.hasAttribute("data-ui5-focus-trap");
910
};
1011

12+
const isScrollable = (el: HTMLElement) => {
13+
const computedStyle = getComputedStyle(el);
14+
15+
return (el.scrollHeight > el.clientHeight && ["scroll", "auto"].indexOf(computedStyle.overflowY) >= 0)
16+
|| (el.scrollWidth > el.clientWidth && ["scroll", "auto"].indexOf(computedStyle.overflowX) >= 0);
17+
};
18+
1119
const getFirstFocusableElement = async (container: HTMLElement, startFromContainer?: boolean): FocusableElementPromise => {
1220
if (!container || isElementHidden(container)) {
1321
return null;
@@ -67,6 +75,12 @@ const findFocusableElement = async (container: HTMLElement, forward: boolean, st
6775
}
6876

6977
focusableDescendant = await findFocusableElement(child, forward);
78+
79+
// check if it is a keyboard focusable scroll container
80+
if (!isSafari() && !focusableDescendant && isScrollable(child)) {
81+
return (child && typeof child.focus === "function") ? child : null;
82+
}
83+
7084
if (focusableDescendant) {
7185
return (focusableDescendant && typeof focusableDescendant.focus === "function") ? focusableDescendant : null;
7286
}

‎packages/main/cypress/specs/Popover.cy.tsx

+60
Original file line numberDiff line numberDiff line change
@@ -154,3 +154,63 @@ describe("Popover interaction", () => {
154154
});
155155
});
156156
});
157+
158+
describe("Focusing", () => {
159+
it("tests no focusable elements, but content is scrolling", () => {
160+
cy.mount(
161+
<>
162+
<Button id="btnOpen">Open</Button>
163+
<Popover id="popoverId"
164+
style="width: 10rem; height: 10rem;"
165+
opener="btnOpen">
166+
<div>
167+
Note: The content of the prop will be rendered into a by assigning the
168+
respective slot attribute (slot="footer"). Since you can't change the
169+
DOM order of slots when declaring them within a prop, it might prove
170+
beneficial to manually mount them as part of the component's children,
171+
especially when facing problems with the reading order of screen
172+
readers. Note: When passing a custom React component to this prop, you
173+
have to make sure your component reads the slot prop and appends it to
174+
the most outer element of your component. Learn more about it here.
175+
</div>
176+
</Popover>
177+
</>
178+
);
179+
180+
// act
181+
cy.get("#popoverId").invoke("prop", "open", "true");
182+
183+
cy.get("#popoverId")
184+
.shadow()
185+
.find(".ui5-popup-content")
186+
.should("be.focused");
187+
});
188+
189+
it("tests first element is keyboard focusable scroll container", () => {
190+
cy.mount(
191+
<>
192+
<Button id="btnOpen">Open</Button>
193+
<Popover id="popoverId"
194+
opener="btnOpen">
195+
<div id="innerContent" style="width: 10rem; height: 10rem; overflow-y: auto;">
196+
Note: The content of the prop will be rendered into a by assigning the
197+
respective slot attribute (slot="footer"). Since you can't change the
198+
DOM order of slots when declaring them within a prop, it might prove
199+
beneficial to manually mount them as part of the component's children,
200+
especially when facing problems with the reading order of screen
201+
readers. Note: When passing a custom React component to this prop, you
202+
have to make sure your component reads the slot prop and appends it to
203+
the most outer element of your component. Learn more about it here.
204+
</div>
205+
<Button>Button</Button>
206+
</Popover>
207+
</>
208+
);
209+
210+
// act
211+
cy.get("#popoverId").invoke("prop", "open", "true");
212+
213+
cy.get("#innerContent")
214+
.should("be.focused");
215+
});
216+
});

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

+7
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,13 @@
3333
flex: auto;
3434
}
3535

36+
/* when the content is keyboard focusable scroll container */
37+
.ui5-popup-content:focus {
38+
outline: var(--sapContent_FocusWidth) var(--sapContent_FocusStyle) var(--sapContent_FocusColor);
39+
outline-offset: calc(-1 * var(--sapContent_FocusWidth));
40+
border-radius: var(--_ui5_popup_border_radius);
41+
}
42+
3643
.ui5-popup-footer-root {
3744
background: var(--sapPageFooter_Background);
3845
border-top: 1px solid var(--sapPageFooter_BorderColor);

‎packages/main/test/pages/PopoverKeyboardFocusableScrollers.html

+68
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
<!DOCTYPE html>
2+
<html class="popover1auto">
3+
4+
<head>
5+
<meta charset="UTF-8">
6+
<meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0, user-scalable=no">
7+
<title>Popover</title>
8+
9+
<script data-ui5-config type="application/json">
10+
{
11+
"language": "EN",
12+
"libs": "sap.ui.webc.main"
13+
}
14+
</script>
15+
16+
17+
<script src="%VITE_BUNDLE_PATH%" type="module"></script>
18+
19+
20+
<link rel="stylesheet" type="text/css" href="./styles/Popover.css">
21+
22+
<script>
23+
// delete Document.prototype.adoptedStyleSheets
24+
</script>
25+
</head>
26+
27+
<body class="popover2auto">
28+
<ui5-button onclick="popoverAttr.open = true;" id="btnOpenWithAttr">Open with Attribute</ui5-button>
29+
<ui5-popover id="popoverAttr"
30+
opener="btnOpenWithAttr">
31+
<div style="width: 10rem; height: 10rem; overflow: auto">
32+
<Text>
33+
Note: The content of the prop will be rendered into a by assigning the
34+
respective slot attribute (slot="footer"). Since you can't change the
35+
DOM order of slots when declaring them within a prop, it might prove
36+
beneficial to manually mount them as part of the component's children,
37+
especially when facing problems with the reading order of screen
38+
readers. Note: When passing a custom React component to this prop, you
39+
have to make sure your component reads the slot prop and appends it to
40+
the most outer element of your component. Learn more about it here.
41+
</Text>
42+
</div>
43+
<ui5-button>Button</ui5-button>
44+
</ui5-popover>
45+
46+
<ui5-button onclick="popoverAttr1.open = true;" id="btnOpenWithAttr1">Open with Attribute 2</ui5-button>
47+
<ui5-popover id="popoverAttr1"
48+
style="width: 10rem; height: 10rem;"
49+
opener="btnOpenWithAttr1">
50+
<div>
51+
<Text>
52+
Note: The content of the prop will be rendered into a by assigning the
53+
respective slot attribute (slot="footer"). Since you can't change the
54+
DOM order of slots when declaring them within a prop, it might prove
55+
beneficial to manually mount them as part of the component's children,
56+
especially when facing problems with the reading order of screen
57+
readers. Note: When passing a custom React component to this prop, you
58+
have to make sure your component reads the slot prop and appends it to
59+
the most outer element of your component. Learn more about it here.
60+
</Text>
61+
</div>
62+
</ui5-popover>
63+
<script>
64+
65+
</script>
66+
</body>
67+
68+
</html>

0 commit comments

Comments
 (0)