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.

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

feat(ui5-wizard): set initial focus when a step is changed #3310

Merged
merged 8 commits into from
May 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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/fiori/src/Wizard.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import clamp from "@ui5/webcomponents-base/dist/util/clamp.js";
import ResizeHandler from "@ui5/webcomponents-base/dist/delegate/ResizeHandler.js";
import { isPhone } from "@ui5/webcomponents-base/dist/Device.js";
import debounce from "@ui5/webcomponents-base/dist/util/debounce.js";
import { getFirstFocusableElement } from "@ui5/webcomponents-base/dist/util/FocusableElements.js";
import Button from "@ui5/webcomponents/dist/Button.js";
import ResponsivePopover from "@ui5/webcomponents/dist/ResponsivePopover.js";

Expand Down Expand Up @@ -133,7 +134,7 @@ const metadata = {
* @event sap.ui.webcomponents.fiori.Wizard#step-change
* @param {HTMLElement} step the new step
* @param {HTMLElement} previousStep the previous step
* @param {Boolean} changeWithClick the step change occurs due to user's click on step within the navigation
* @param {Boolean} changeWithClick the step change occurs due to user's click or 'Enter'/'Space' key press on step within the navigation
* @public
*/
"step-change": {
Expand Down Expand Up @@ -427,7 +428,7 @@ class Wizard extends UI5Element {
*/
onSelectionChangeRequested(event) {
this.selectionRequestedByClick = true;
this.changeSelectionByStepClick(event.target);
this.changeSelectionByStepAction(event.target);
}

/**
Expand Down Expand Up @@ -649,16 +650,20 @@ class Wizard extends UI5Element {
/**
* Called upon <code>onSelectionChangeRequested</code>.
* Selects the external step (ui5-wizard-step),
* based on the clicked step in the header (ui5-wizard-tab).
* based on the clicked or activated via keyboard step in the header (ui5-wizard-tab).
* @param {HTMLElement} stepInHeader the step equivalent in the header
* @private
*/
changeSelectionByStepClick(stepInHeader) {
async changeSelectionByStepAction(stepInHeader) {
const stepRefId = stepInHeader.getAttribute("data-ui5-content-ref-id");
const selectedStep = this.selectedStep;
const stepToSelect = this.getStepByRefId(stepRefId);
const bExpanded = stepInHeader.getAttribute(EXPANDED_STEP) === "true";
const newlySelectedIndex = this.slottedSteps.indexOf(stepToSelect);
const firstFocusableElement = await getFirstFocusableElement(stepToSelect.firstElementChild);

// Focus the first focusable element within the step content corresponding to the currently focused tab
firstFocusableElement.focus();

// If the currently selected (active) step is clicked,
// just scroll to its starting point and stop.
Expand Down
7 changes: 2 additions & 5 deletions packages/fiori/src/WizardTab.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import UI5Element from "@ui5/webcomponents-base/dist/UI5Element.js";
import litRender from "@ui5/webcomponents-base/dist/renderer/LitRenderer.js";
import { isSpace, isEnter } from "@ui5/webcomponents-base/dist/Keys.js";
import Icon from "@ui5/webcomponents/dist/Icon.js";

import Icon from "@ui5/webcomponents/dist/Icon.js";
import WizardTabTemplate from "./generated/templates/WizardTabTemplate.lit.js";
import WizardTabCss from "./generated/themes/WizardTab.css.js";

Expand Down Expand Up @@ -181,11 +181,8 @@ class WizardTab extends UI5Element {
return;
}

if (isSpace(event)) {
if (isSpace(event) || isEnter(event)) {
event.preventDefault();
}

if (isEnter(event)) {
this.fireEvent("selection-change-requested");
}
}
Expand Down
9 changes: 0 additions & 9 deletions packages/fiori/test/pages/Wizard.html
Original file line number Diff line number Diff line change
Expand Up @@ -372,9 +372,6 @@ <h2>Wizard non-standard 2</h2>
<ui5-label wrap>
Integer pellentesque leo sit amet dui vehicula, quis ullamcorper est pulvinar. Nam in libero sem. Suspendisse arcu metus, molestie a turpis a, molestie aliquet dui. Donec ppellentesque leo sit amet dui vehicula, quis ullamcorper est pulvinar. Nam in libero sem. Suspendisse arcu metus, molestie a turpis a, molestie aliquet dui. Donec pulvinar, sapien corper eu, posuere malesuada nisl. Integer pellentesque leo sit amet dui vehicula, quis ullamcorper est pulvinar. Nam in libero sem. Suspendisse arcu metus, molestie a turpis a, molestie aliquet dui. Donec pulvinar, sapien
</ui5-label>
<ui5-messagestrip>
The Wizard control is supposed to break down large tasks, into smaller steps, easier for the user to work with.
</ui5-messagestrip><br>

<div style="display: flex; flex-direction: column;">
<div style="display: flex; flex-direction: row; justify-content: flex-end; align-items: center; margin-top: 1rem">
Expand Down Expand Up @@ -496,9 +493,6 @@ <h2>Wizard non-standard 3</h2>
<ui5-label wrap>
Integer pellentesque leo sit amet dui vehicula, quis ullamcorper est pulvinar. Nam in libero sem. Suspendisse arcu metus, molestie a turpis a, molestie aliquet dui. Donec ppellentesque leo sit amet dui vehicula, quis ullamcorper est pulvinar. Nam in libero sem. Suspendisse arcu metus, molestie a turpis a, molestie aliquet dui. Donec pulvinar, sapien corper eu, posuere malesuada nisl. Integer pellentesque leo sit amet dui vehicula, quis ullamcorper est pulvinar. Nam in libero sem. Suspendisse arcu metus, molestie a turpis a, molestie aliquet dui. Donec pulvinar, sapien
</ui5-label>
<ui5-messagestrip>
The Wizard control is supposed to break down large tasks, into smaller steps, easier for the user to work with.
</ui5-messagestrip><br>

<div style="display: flex; flex-direction: column;">
<div style="display: flex; flex-direction: row; justify-content: flex-end; align-items: center; margin-top: 1rem">
Expand Down Expand Up @@ -536,9 +530,6 @@ <h2>Wizard non-standard 3</h2>
<ui5-label wrap>
Integer pellentesque leo sit amet dui vehicula, quis ullamcorper est pulvinar. Nam in libero sem. Suspendisse arcu metus, molestie a turpis a, molestie aliquet dui. Donec ppellentesque leo sit amet dui vehicula, quis ullamcorper est pulvinar. Nam in libero sem. Suspendisse arcu metus, molestie a turpis a, molestie aliquet dui. Donec pulvinar, sapien corper eu, posuere malesuada nisl. Integer pellentesque leo sit amet dui vehicula, quis ullamcorper est pulvinar. Nam in libero sem. Suspendisse arcu metus, molestie a turpis a, molestie aliquet dui. Donec pulvinar, sapien
</ui5-label>
<ui5-messagestrip>
The Wizard control is supposed to break down large tasks, into smaller steps, easier for the user to work with.
</ui5-messagestrip><br>

<div style="display: flex; flex-direction: column;">
<div style="display: flex; flex-direction: row; justify-content: flex-end; align-items: center; margin-top: 1rem">
Expand Down
51 changes: 36 additions & 15 deletions packages/fiori/test/specs/Wizard.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,16 @@ describe("Wizard general interaction", () => {
const wiz = browser.$("#wizTest");
const step1 = browser.$("#st1");
const step2 = browser.$("#st2");

const step1InHeader = wiz.shadow$(`[data-ui5-index="1"]`);
const step2InHeader = wiz.shadow$(`[data-ui5-index="2"]`);
const inpStepChangeCounter = browser.$("#inpStepChangeCounter");
const inpStepChangeCause = browser.$("#inpStepChangeCause");

const wizardStep = browser.$("ui5-wizard-step");
const messageStrip = wizardStep.shadow$("ui5-messagestrip")
const firstFocusableElement = messageStrip.shadow$(`ui5-button`);

// act - click on the first step in the header
step1InHeader.click();

Expand All @@ -101,6 +106,16 @@ describe("Wizard general interaction", () => {
assert.strictEqual(step1InHeader.getAttribute("disabled"), null,
"First step in header is enabled.");

assert.strictEqual(firstFocusableElement.getAttribute("focused"), "true", "The First focusable element in the step content is focused.");

step1InHeader.keys(["Shift", "Tab"]);
step2InHeader.keys("Space");
assert.strictEqual(firstFocusableElement.getAttribute("focused"), "true", "The First focusable element in the step content is focused.");

step1InHeader.keys(["Shift", "Tab"]);
step2InHeader.keys("Enter");
assert.strictEqual(firstFocusableElement.getAttribute("focused"), "true", "The First focusable element in the step content is focused.");

// assert - that second step in the content and in the header are not selected
assert.strictEqual(step2.getAttribute("selected"), null,
"Second step in the content is not selected.");
Expand All @@ -126,6 +141,7 @@ describe("Wizard general interaction", () => {
// act - bring the focus to the first step in the header
// act - use keyboard to move to step2
step1InHeader.click();
step1InHeader.keys(["Shift", "Tab"]);
step1InHeader.keys("ArrowRight");
step2InHeader.keys("Space");

Expand All @@ -146,29 +162,34 @@ describe("Wizard general interaction", () => {
// assert - step-change second time
assert.strictEqual(inpStepChangeCounter.getProperty("value"), "2", "Event step-change fired 2nd time.");

// act - use keyboard to move back to step1
step2InHeader.keys("ArrowLeft");
// act - move back to step1 then move the focus to the step 2 and press enter
step1InHeader.click();
step1InHeader.keys(["Shift", "Tab"]);
step1InHeader.keys("ArrowRight");
step1InHeader.keys("Enter");

// assert - that first step in the content and in the header are selected
assert.strictEqual(step1.getAttribute("selected"), "true",
assert.strictEqual(step2.getAttribute("selected"), "true",
"First step in the content is not selected.");
assert.strictEqual(step1InHeader.getAttribute("selected"), "true",
assert.strictEqual(step2InHeader.getAttribute("selected"), "true",
"First step in the header not is selected.");
assert.strictEqual(step1.getAttribute("disabled"), null,
assert.strictEqual(step2.getAttribute("disabled"), null,
"First step is enabled.");
assert.strictEqual(step1InHeader.getAttribute("disabled"), null,
assert.strictEqual(step2InHeader.getAttribute("disabled"), null,
"First step in header is enabled.");

// assert - that second step in the content and in the header are not selected
assert.strictEqual(step2.getAttribute("selected"), null,
// assert - that first step in the content and in the header are not selected
assert.strictEqual(step1.getAttribute("selected"), null,
"Second step in the content is not selected.");
assert.strictEqual(step2InHeader.getAttribute("selected"), null,
assert.strictEqual(step1InHeader.getAttribute("selected"), null,
"Second step in the header is not selected.");

// assert - step-change second time
assert.strictEqual(inpStepChangeCounter.getProperty("value"), "3",
"Event step-change fired 3rd time.");
assert.strictEqual(inpStepChangeCounter.getProperty("value"), "4",
"Event step-change fired 4th time.");

// Activate the first step for the next tests
step1InHeader.click();
});

it("move to next step by scroll", () => {
Expand All @@ -191,8 +212,8 @@ describe("Wizard general interaction", () => {
assert.strictEqual(step2.getAttribute("disabled"), null, "Second step is enabled.");
assert.strictEqual(step2InHeader.getAttribute("disabled"), null, "Second step in header is enabled.");

assert.strictEqual(inpStepChangeCounter.getProperty("value"), "4",
"Event step-change fired 4th time due to scrolling.");
assert.strictEqual(inpStepChangeCounter.getProperty("value"), "6",
"Event step-change fired 6th time due to scrolling.");

// assert - step-change fired not because of user click
assert.strictEqual(inpStepChangeCause.getProperty("value"), "false",
Expand All @@ -219,8 +240,8 @@ describe("Wizard general interaction", () => {
"Third step in the content is selected.");
assert.strictEqual(step3InHeader.getAttribute("selected"), "true",
"Third step in the header is selected.");
assert.strictEqual(inpStepChangeCounter.getProperty("value"), "5",
"Event step-change fired once for 5th time due to scrolling.");
assert.strictEqual(inpStepChangeCounter.getProperty("value"), "7",
"Event step-change fired once for 7th time due to scrolling.");
});

it("tests no scrolling to step, if the step was not changed", ()=>{
Expand Down