From 881d80fd8b658491e26125222020401d0e5a5d08 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Olli=20Tiet=C3=A4v=C3=A4inen?= Date: Wed, 24 Jan 2018 11:29:10 +0200 Subject: [PATCH] Make modal window focus circulate correctly (#10497) --- .../java/com/vaadin/client/ui/FocusUtil.java | 13 ++++- .../components/window/ModalWindowFocus.java | 22 +++++++++ .../window/ModalWindowFocusTest.java | 49 +++++++++++++------ 3 files changed, 67 insertions(+), 17 deletions(-) diff --git a/client/src/main/java/com/vaadin/client/ui/FocusUtil.java b/client/src/main/java/com/vaadin/client/ui/FocusUtil.java index 0f517d89ed..fe0f765f47 100644 --- a/client/src/main/java/com/vaadin/client/ui/FocusUtil.java +++ b/client/src/main/java/com/vaadin/client/ui/FocusUtil.java @@ -119,8 +119,17 @@ public class FocusUtil { */ public static void focusOnFirstFocusableElement(Element parent) { Element[] focusableChildren = getFocusableChildren(parent); - if (focusableChildren.length > 0) { - focusableChildren[0].focus(); + if (focusableChildren.length == 0) { + return; + } + // find the first element that doesn't have "disabled" in the class name + for (int i = 0; i < focusableChildren.length; i++) { + Element element = focusableChildren[i]; + String classes = element.getAttribute("class"); + if (classes == null || !classes.toLowerCase().contains("disabled")) { + element.focus(); + return; + } } } diff --git a/uitest/src/main/java/com/vaadin/tests/components/window/ModalWindowFocus.java b/uitest/src/main/java/com/vaadin/tests/components/window/ModalWindowFocus.java index 9e77600213..c736aa2151 100644 --- a/uitest/src/main/java/com/vaadin/tests/components/window/ModalWindowFocus.java +++ b/uitest/src/main/java/com/vaadin/tests/components/window/ModalWindowFocus.java @@ -20,6 +20,8 @@ import com.vaadin.server.VaadinRequest; import com.vaadin.tests.components.AbstractReindeerTestUI; import com.vaadin.ui.Button; import com.vaadin.ui.HorizontalLayout; +import com.vaadin.ui.TextField; +import com.vaadin.ui.VerticalLayout; import com.vaadin.ui.Window; @Widgetset("com.vaadin.DefaultWidgetSet") @@ -53,6 +55,26 @@ public class ModalWindowFocus extends AbstractReindeerTestUI { addWindow(w3); }); }); + Button button2 = new Button( + "Open unclosable and unresizable modal window"); + addComponent(button2); + button2.setId("modalWindowButton"); + button2.addClickListener(event -> { + Window modalWindow = new Window("Modal window"); + modalWindow.setModal(true); + modalWindow.setClosable(false); + modalWindow.setResizable(false); + VerticalLayout vl = new VerticalLayout(); + TextField tf = new TextField("Textfield"); + tf.setId("focusfield"); + tf.addFocusListener(e -> tf.setValue("this has been focused")); + TextField tf2 = new TextField("Another Textfield"); + tf2.focus(); + vl.addComponents(tf, tf2); + modalWindow.setContent(vl); + addWindow(modalWindow); + }); + } @Override diff --git a/uitest/src/test/java/com/vaadin/tests/components/window/ModalWindowFocusTest.java b/uitest/src/test/java/com/vaadin/tests/components/window/ModalWindowFocusTest.java index 8cbb9d6ae9..b65ff037a6 100644 --- a/uitest/src/test/java/com/vaadin/tests/components/window/ModalWindowFocusTest.java +++ b/uitest/src/test/java/com/vaadin/tests/components/window/ModalWindowFocusTest.java @@ -26,6 +26,7 @@ import org.openqa.selenium.WebElement; import org.openqa.selenium.interactions.Actions; import com.vaadin.testbench.By; +import com.vaadin.testbench.elements.TextFieldElement; import com.vaadin.tests.tb3.MultiBrowserTest; /** @@ -43,8 +44,8 @@ public class ModalWindowFocusTest extends MultiBrowserTest { } /** - * First scenario: press button -> two windows appear, press Esc two times - * -> all windows should be closed + * First scenario: press first button -> two windows appear, press Esc two + * times -> all windows should be closed */ @Test public void testModalWindowFocusTwoWindows() throws IOException { @@ -57,17 +58,17 @@ public class ModalWindowFocusTest extends MultiBrowserTest { assertTrue("Second window should be opened", findElements(By.id("windowButton")).size() == 1); - pressEscAndWait(); - pressEscAndWait(); + pressKeyAndWait(Keys.ESCAPE); + pressKeyAndWait(Keys.ESCAPE); assertTrue("All windows should be closed", findElements(By.className("v-window")).size() == 0); } /** - * Second scenario: press button -> two windows appear, press button in the - * 2nd window -> 3rd window appears on top, press Esc three times -> all - * windows should be closed + * Second scenario: press first button -> two windows appear, press button + * in the 2nd window -> 3rd window appears on top, press Esc three times -> + * all windows should be closed */ @Test public void testModalWindowFocusPressButtonInWindow() throws IOException { @@ -84,14 +85,37 @@ public class ModalWindowFocusTest extends MultiBrowserTest { assertTrue("Third window should be opened", findElements(By.id("window3")).size() == 1); - pressEscAndWait(); - pressEscAndWait(); - pressEscAndWait(); + pressKeyAndWait(Keys.ESCAPE); + pressKeyAndWait(Keys.ESCAPE); + pressKeyAndWait(Keys.ESCAPE); assertTrue("All windows should be closed", findElements(By.className("v-window")).size() == 0); } + /** + * Third scenario: press second button -> a modal unclosable and + * unresizeable window with two text fields opens -> second text field is + * automatically focused -> press tab -> the focus rolls around to the top + * of the modal window -> the first text field is focused and shows a text + */ + @Test + public void testModalWindowWithoutButtonsFocusHandling() { + waitForElementPresent(By.id("modalWindowButton")); + WebElement button = findElement(By.id("modalWindowButton")); + button.click(); + waitForElementPresent(By.id("focusfield")); + pressKeyAndWait(Keys.TAB); + TextFieldElement tfe = $(TextFieldElement.class).id("focusfield"); + assertTrue("First TextField should have received focus", + "this has been focused".equals(tfe.getValue())); + } + + private void pressKeyAndWait(Keys key) { + new Actions(driver).sendKeys(key).build().perform(); + sleep(100); + } + @Test public void verifyAriaModalAndRoleAttributes() { waitForElementPresent(By.id("firstButton")); @@ -107,9 +131,4 @@ public class ModalWindowFocusTest extends MultiBrowserTest { } - private void pressEscAndWait() { - new Actions(driver).sendKeys(Keys.ESCAPE).build().perform(); - sleep(100); - } - } -- 2.39.5