From 269ce1beac0fb164cada94342654e96ccfe0425f Mon Sep 17 00:00:00 2001 From: Mehdi Javan <32511762+mehdi-vaadin@users.noreply.github.com> Date: Wed, 19 Sep 2018 09:20:04 +0300 Subject: [PATCH] Fix/focus inside window chrome (#11142) Fixes #11087 --- .../com/vaadin/client/ui/ui/UIConnector.java | 85 +++++++++++-------- .../tests/components/window/InitialFocus.java | 74 ++++++++++++++++ .../components/window/InitialFocusTest.java | 54 ++++++++++++ 3 files changed, 176 insertions(+), 37 deletions(-) create mode 100644 uitest/src/main/java/com/vaadin/tests/components/window/InitialFocus.java create mode 100644 uitest/src/test/java/com/vaadin/tests/components/window/InitialFocusTest.java diff --git a/client/src/main/java/com/vaadin/client/ui/ui/UIConnector.java b/client/src/main/java/com/vaadin/client/ui/ui/UIConnector.java index adb06a450a..5630cfab3c 100644 --- a/client/src/main/java/com/vaadin/client/ui/ui/UIConnector.java +++ b/client/src/main/java/com/vaadin/client/ui/ui/UIConnector.java @@ -15,16 +15,6 @@ */ package com.vaadin.client.ui.ui; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; -import java.util.Comparator; -import java.util.HashMap; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.logging.Logger; - import com.google.gwt.core.client.Scheduler; import com.google.gwt.dom.client.Document; import com.google.gwt.dom.client.Element; @@ -101,9 +91,18 @@ import com.vaadin.shared.ui.ui.UIServerRpc; import com.vaadin.shared.ui.ui.UIState; import com.vaadin.shared.util.SharedUtil; import com.vaadin.ui.UI; - import elemental.client.Browser; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.Comparator; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.logging.Logger; + @Connect(value = UI.class, loadStyle = LoadStyle.EAGER) public class UIConnector extends AbstractSingleComponentContainerConnector implements Paintable, MayScrollChildren { @@ -375,34 +374,18 @@ public class UIConnector extends AbstractSingleComponentContainerConnector if (uidl.hasAttribute("focused")) { // set focused component when render phase is finished Scheduler.get().scheduleDeferred(() -> { - ComponentConnector connector = (ComponentConnector) uidl - .getPaintableAttribute("focused", getConnection()); + Timer timer = new Timer() { + @Override + public void run() { + ComponentConnector connector = (ComponentConnector) uidl + .getPaintableAttribute("focused", + getConnection()); - if (connector == null) { - // Do not try to focus invisible components which not - // present in UIDL - return; - } + focus(connector); + } + }; - final Widget toBeFocused = connector.getWidget(); - /* - * Two types of Widgets can be focused, either implementing GWT - * Focusable of a thinner Vaadin specific Focusable interface. - */ - if (toBeFocused instanceof com.google.gwt.user.client.ui.Focusable) { - final com.google.gwt.user.client.ui.Focusable toBeFocusedWidget = (com.google.gwt.user.client.ui.Focusable) toBeFocused; - toBeFocusedWidget.setFocus(true); - } else if (toBeFocused instanceof Focusable) { - ((Focusable) toBeFocused).focus(); - } else { - getLogger().severe( - "Server is trying to set focus to the widget of connector " - + Util.getConnectorString(connector) - + " but it is not focusable. The widget should implement either " - + com.google.gwt.user.client.ui.Focusable.class - .getName() - + " or " + Focusable.class.getName()); - } + timer.schedule(0); }); } @@ -437,6 +420,34 @@ public class UIConnector extends AbstractSingleComponentContainerConnector } } + private void focus(ComponentConnector connector) { + if (connector == null) { + // Do not try to focus invisible components which not + // present in UIDL + return; + } + + final Widget toBeFocused = connector.getWidget(); + /* + * Two types of Widgets can be focused, either implementing GWT + * Focusable of a thinner Vaadin specific Focusable interface. + */ + if (toBeFocused instanceof com.google.gwt.user.client.ui.Focusable) { + final com.google.gwt.user.client.ui.Focusable toBeFocusedWidget = (com.google.gwt.user.client.ui.Focusable) toBeFocused; + toBeFocusedWidget.setFocus(true); + } else if (toBeFocused instanceof Focusable) { + ((Focusable) toBeFocused).focus(); + } else { + getLogger().severe( + "Server is trying to set focus to the widget of connector " + + Util.getConnectorString(connector) + + " but it is not focusable. The widget should implement either " + + com.google.gwt.user.client.ui.Focusable.class + .getName() + + " or " + Focusable.class.getName()); + } + } + /** * Reads CSS strings and resources injected by {@link Styles#inject} from * the UIDL stream. diff --git a/uitest/src/main/java/com/vaadin/tests/components/window/InitialFocus.java b/uitest/src/main/java/com/vaadin/tests/components/window/InitialFocus.java new file mode 100644 index 0000000000..52dc916824 --- /dev/null +++ b/uitest/src/main/java/com/vaadin/tests/components/window/InitialFocus.java @@ -0,0 +1,74 @@ +package com.vaadin.tests.components.window; + +import com.vaadin.server.VaadinRequest; +import com.vaadin.tests.components.AbstractTestUI; +import com.vaadin.ui.Button; +import com.vaadin.ui.ComboBox; +import com.vaadin.ui.TextField; +import com.vaadin.ui.VerticalLayout; +import com.vaadin.ui.Window; + +public class InitialFocus extends AbstractTestUI { + + public static final String FOCUS_NAME_BUTTON_ID = "focusNameButton"; + public static final String FOCUS_GENDER_BUTTON_ID = "focusGenderButton"; + public static final String NAME_FIELD_ID = "nameField"; + public static final String GENDER_FIELD_ID = "genderField"; + + @Override + protected void setup(VaadinRequest request) { + Button focusNameButton = new Button("Open Window and focus Name"); + focusNameButton.setId(FOCUS_NAME_BUTTON_ID); + focusNameButton.addClickListener(event -> { + MyDialog myDialog = new MyDialog(); + myDialog.setClosable(true); + myDialog.center(); + getUI().addWindow(myDialog); + myDialog.bringToFront(); + myDialog.focusNameField(); + }); + addComponent(focusNameButton); + + Button focusGenderButton = new Button("Open Window and focus Gender"); + focusGenderButton.setId(FOCUS_GENDER_BUTTON_ID); + focusGenderButton.addClickListener(event -> { + MyDialog myDialog = new MyDialog(); + myDialog.setClosable(true); + myDialog.center(); + getUI().addWindow(myDialog); + myDialog.bringToFront(); + myDialog.focusGenderField(); + }); + addComponent(focusGenderButton); + } + + private static class MyDialog extends Window { + private TextField nameField; + private ComboBox genderField; + + private MyDialog() { + super("MyDialog"); + setWidth("400px"); + setHeight("300px"); + VerticalLayout hl = new VerticalLayout(); + hl.setSizeFull(); + nameField = new TextField("Name"); + nameField.setId(NAME_FIELD_ID); + hl.addComponent(this.nameField); + + genderField = new ComboBox("Gender"); + genderField.setId(GENDER_FIELD_ID); + hl.addComponentsAndExpand(genderField); + + this.setContent(hl); + } + + private void focusNameField() { + nameField.focus(); + } + + private void focusGenderField() { + genderField.focus(); + } + } +} diff --git a/uitest/src/test/java/com/vaadin/tests/components/window/InitialFocusTest.java b/uitest/src/test/java/com/vaadin/tests/components/window/InitialFocusTest.java new file mode 100644 index 0000000000..a6ab48803c --- /dev/null +++ b/uitest/src/test/java/com/vaadin/tests/components/window/InitialFocusTest.java @@ -0,0 +1,54 @@ +package com.vaadin.tests.components.window; + +import com.vaadin.testbench.elements.ComboBoxElement; +import com.vaadin.tests.tb3.MultiBrowserTest; +import org.junit.Assert; +import org.junit.Test; +import org.openqa.selenium.By; +import org.openqa.selenium.WebElement; + +public class InitialFocusTest extends MultiBrowserTest { + /** + * To test an implementation of + * {@code com.google.gwt.user.client.ui.Focusable} which is nameField of + * TextField type + */ + @Test + public void window_callingFocusNameField_nameFieldShouldGetFocused() { + openTestURL(); + WebElement openWindowButton = findElement( + By.id(InitialFocus.FOCUS_NAME_BUTTON_ID)); + openWindowButton.click(); + waitForElementPresent(By.id(InitialFocus.NAME_FIELD_ID)); + WebElement focusedElement = getFocusedElement(); + Assert.assertNotNull( + "Name TextField should have focus while focusedElement is null.", + focusedElement); + String focusedElementId = focusedElement.getAttribute("id"); + Assert.assertEquals( + "Name TextField should have focus while " + focusedElementId + + " has focus.", + InitialFocus.NAME_FIELD_ID, focusedElementId); + } + + /** + * To test an implementation of {@code com.vaadin.client.Focusable} which is + * genderField of ComboBox type + */ + @Test + public void window_callingFocusGenderField_genderFieldShouldGetFocused() { + openTestURL(); + WebElement openWindowButton = findElement( + By.id(InitialFocus.FOCUS_GENDER_BUTTON_ID)); + openWindowButton.click(); + waitForElementPresent(By.id(InitialFocus.GENDER_FIELD_ID)); + WebElement focusedElement = getFocusedElement(); + Assert.assertNotNull( + "Gender ComboBox should have focus while focusedElement is null.", + focusedElement); + ComboBoxElement genderField = $(ComboBoxElement.class).first(); + Assert.assertEquals( + "Gender ComboBox should have focus while another element has focus.", + genderField.getInputField(), focusedElement); + } +} -- 2.39.5