Skip to content
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.

Sign up for GitHub

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

fix(framework): correct use of arrow keys for ItemNavigation in RTL #5408

Merged
merged 3 commits into from
Jun 30, 2022
Merged

fix(framework): correct use of arrow keys for ItemNavigation in RTL #5408

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a7bac7f
fix(framework): correct use of arrow keys for ItemNavigation in RTL
georgimkv Jun 24, 2022
4eb9708
fix: change component under test
georgimkv Jun 24, 2022
31f1c09
fix: add line
georgimkv Jun 24, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions packages/base/src/delegate/ItemNavigation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,15 +133,20 @@ class ItemNavigation {

const horizontalNavigationOn = this._navigationMode === NavigationMode.Horizontal || this._navigationMode === NavigationMode.Auto;
const verticalNavigationOn = this._navigationMode === NavigationMode.Vertical || this._navigationMode === NavigationMode.Auto;
const isRTL = this.rootWebComponent.effectiveDir === "rtl";

if (isUp(event) && verticalNavigationOn) {
this._handleUp();
} else if (isDown(event) && verticalNavigationOn) {
this._handleDown();
if (isRTL && isLeft(event) && horizontalNavigationOn) {
this._handleRight();
} else if (isRTL && isRight(event) && horizontalNavigationOn) {
this._handleLeft();
} else if (isLeft(event) && horizontalNavigationOn) {
this._handleLeft();
} else if (isRight(event) && horizontalNavigationOn) {
this._handleRight();
} else if (isUp(event) && verticalNavigationOn) {
this._handleUp();
} else if (isDown(event) && verticalNavigationOn) {
this._handleDown();
} else if (isHome(event)) {
this._handleHome();
} else if (isEnd(event)) {
Expand Down
35 changes: 30 additions & 5 deletions packages/main/test/pages/ItemNavigation.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,20 @@
<html>
<head>
<meta http-equiv="X-UA-Compatible" content="IE=edge">
<meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0, user-scalable=no">
<meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0, user-scalable=no">
<meta charset="utf-8">
<title>Item Navigation</title>


<script src="../../bundle.esm.js" type="module"></script>


<link rel="stylesheet" type="text/css" href="./styles/ItemNavigation.css">
</head>

<body class="itemnavigation1auto">
<div>
<ui5-label show-colon>Manually switch RTL</ui5-label>
<ui5-switch id="sw"></ui5-switch>
</div>

<h2>Focus does not cycle</h2>
<ui5-list>
<ui5-li id="item1">Option 1</ui5-li>
Expand All @@ -27,6 +29,13 @@ <h2>Vertical navigation only</h2>
<ui5-li id="item5">Option 2.3</ui5-li>
</ui5-list>

<h2>Horizontal navigation only</h2>
<ui5-avatar-group type="Individual" id="horizontalNavigation">
<ui5-avatar size="S" initials="A"></ui5-avatar>
<ui5-avatar size="S" initials="B"></ui5-avatar>
<ui5-avatar size="S" initials="C"></ui5-avatar>
</ui5-avatar-group>

<h1>Test PAGE UP/DOWN</h1>
<br><br>
<ui5-list>
Expand Down Expand Up @@ -64,5 +73,21 @@ <h1>Test PAGE UP/DOWN</h1>
<ui5-li>29</ui5-li>
<ui5-li>30</ui5-li>
</ui5-list>

<script>
// Utility function to change RTL and apply the changes
function setDir(dir) {
document.body.dir = dir;
window['sap-ui-webcomponents-bundle'].applyDirection();
}

document.getElementById("sw").addEventListener("ui5-change", (e) => {
if (e.target.checked) {
setDir("rtl");
} else {
setDir("ltr");
}
});
</script>
</body>
</html>
</html>
48 changes: 47 additions & 1 deletion packages/main/test/specs/ItemNavigation.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,50 @@ describe("Item Navigation Tests", () => {
await itemOnFocus2.keys("PageUp");
assert.ok(await nextFocusedItem2.isFocused(), "The 6th is focused.");
});
});

it("tests left and right arrow keys in LTR", async () => {
// Setup
const items = await browser.$$("#horizontalNavigation ui5-avatar"),
first = items[0],
third = items[2];

await first.click();

// Assert
assert.strictEqual(await first.isFocused(), true, "first item is focused");

// Act
await browser.keys("ArrowRight");
await browser.keys("ArrowRight");

// Assert
assert.strictEqual(await third.isFocused(), true, "third item is focused");

});


it("tests left and right arrow keys in RTL", async () => {
// Setup
const switchEl = await browser.$("#sw");
await switchEl.click();

const items = await browser.$$("#horizontalNavigation ui5-avatar"),
first = items[0],
third = items[2];

await first.click();

// Assert
assert.strictEqual(await first.isFocused(), true, "first item is focused");

// Act
await browser.keys("ArrowLeft");
await browser.keys("ArrowLeft");

// Assert
assert.strictEqual(await third.isFocused(), true, "third item is focused");

// Clean-up
await switchEl.click();
});
});