From acf28902774c68919726051984e847bcedaf7a3f Mon Sep 17 00:00:00 2001 From: Sauli Tähkäpää Date: Wed, 12 Nov 2014 10:06:22 +0200 Subject: Revert "Table column width can be changed from defined to expandratio (#15101)" This reverts commit 7237b88645a27b157bc85d62292dc93faddd19f9. Change-Id: Ifbe26c90a91a21a1ac416bda8e59acbbd80cf5f0 --- client/src/com/vaadin/client/ui/VScrollTable.java | 1 - 1 file changed, 1 deletion(-) (limited to 'client') diff --git a/client/src/com/vaadin/client/ui/VScrollTable.java b/client/src/com/vaadin/client/ui/VScrollTable.java index b07d2dc9c0..6c241f1033 100644 --- a/client/src/com/vaadin/client/ui/VScrollTable.java +++ b/client/src/com/vaadin/client/ui/VScrollTable.java @@ -3636,7 +3636,6 @@ public class VScrollTable extends FlowPanel implements HasWidgets, c.setWidth(widthWithoutAddedIndent, true); } } else if (col.hasAttribute("er")) { - c.setUndefinedWidth(); c.setExpandRatio(col.getFloatAttribute("er")); } else if (recalcWidths) { -- cgit v1.2.3 From c4075e1f2fe0b35a7f90564c4315f03403826caa Mon Sep 17 00:00:00 2001 From: Sauli Tähkäpää Date: Fri, 14 Nov 2014 14:57:15 +0200 Subject: Revert "DateField popup doesn't close when click on popup button (#14857)" This reverts commit 457e802e2fe59ec35089a55acdc7b0321a2d4a5a. Patch does not fix the issue with "slow" clicks when closing the calendar. Change-Id: I48384e081cf66dd4fc6cded8ecbd94cef5db57bb --- .../src/com/vaadin/client/ui/VPopupCalendar.java | 79 +++------------------- .../datefield/DateFieldPopupClosing.java | 43 ------------ .../datefield/DateFieldPopupClosingTest.java | 42 ------------ 3 files changed, 11 insertions(+), 153 deletions(-) delete mode 100644 uitest/src/com/vaadin/tests/components/datefield/DateFieldPopupClosing.java delete mode 100644 uitest/src/com/vaadin/tests/components/datefield/DateFieldPopupClosingTest.java (limited to 'client') diff --git a/client/src/com/vaadin/client/ui/VPopupCalendar.java b/client/src/com/vaadin/client/ui/VPopupCalendar.java index fbd66cff09..51b2ee22ec 100644 --- a/client/src/com/vaadin/client/ui/VPopupCalendar.java +++ b/client/src/com/vaadin/client/ui/VPopupCalendar.java @@ -27,10 +27,6 @@ import com.google.gwt.event.dom.client.ClickEvent; import com.google.gwt.event.dom.client.ClickHandler; import com.google.gwt.event.dom.client.DomEvent; import com.google.gwt.event.dom.client.KeyCodes; -import com.google.gwt.event.dom.client.MouseOutEvent; -import com.google.gwt.event.dom.client.MouseOutHandler; -import com.google.gwt.event.dom.client.MouseOverEvent; -import com.google.gwt.event.dom.client.MouseOverHandler; import com.google.gwt.event.logical.shared.CloseEvent; import com.google.gwt.event.logical.shared.CloseHandler; import com.google.gwt.i18n.client.DateTimeFormat; @@ -64,8 +60,7 @@ import com.vaadin.shared.ui.datefield.Resolution; * */ public class VPopupCalendar extends VTextualDate implements Field, - ClickHandler, MouseOverHandler, MouseOutHandler, - CloseHandler, SubPartAware { + ClickHandler, CloseHandler, SubPartAware { /** For internal use only. May be removed or replaced in the future. */ public final Button calendarToggle = new Button(); @@ -80,20 +75,6 @@ public class VPopupCalendar extends VTextualDate implements Field, public boolean parsable = true; private boolean open = false; - /* - * To resolve #14857. If to click on calendarToggle button when calendar - * popup is opened (*1) then we have the following chain of calls: - * - * 1) onClose() 2) onClick() - * - * In this case we should prevent calling openCalendarPanel() in onClick. - */ - private boolean preventOpenPopupCalendar = false; - /* - * To resolve #14857. To determine this situation (*1) we use onMouseOver - * and OnMouseOut (for calendarToggle button). - */ - private boolean cursorOverCalendarToggleButton = false; private boolean textFieldEnabled = true; @@ -108,10 +89,6 @@ public class VPopupCalendar extends VTextualDate implements Field, calendarToggle.setText(""); calendarToggle.addClickHandler(this); - - calendarToggle.addMouseOverHandler(this); - calendarToggle.addMouseOutHandler(this); - // -2 instead of -1 to avoid FocusWidget.onAttach to reset it calendarToggle.getElement().setTabIndex(-2); @@ -464,10 +441,7 @@ public class VPopupCalendar extends VTextualDate implements Field, @Override public void onClick(ClickEvent event) { if (event.getSource() == calendarToggle && isEnabled()) { - if (!preventOpenPopupCalendar) { - openCalendarPanel(); - } - preventOpenPopupCalendar = false; + openCalendarPanel(); } } @@ -490,22 +464,15 @@ public class VPopupCalendar extends VTextualDate implements Field, focus(); } - open = false; - - if (cursorOverCalendarToggleButton) { - preventOpenPopupCalendar = true; - - // To resolve the problem: onMouseOut event not triggered when - // moving mouse fast (GWT - all browsers) - Timer unPreventClickTimer = new Timer() { - @Override - public void run() { - preventOpenPopupCalendar = false; - } - }; - - unPreventClickTimer.schedule(300); - } + // TODO resolve what the "Sigh." is all about and document it here + // Sigh. + Timer t = new Timer() { + @Override + public void run() { + open = false; + } + }; + t.schedule(100); } } @@ -675,28 +642,4 @@ public class VPopupCalendar extends VTextualDate implements Field, calendar.setRangeEnd(rangeEnd); } - /* - * (non-Javadoc) - * - * @see - * com.google.gwt.event.dom.client.MouseOverHandler#onMouseOver(com.google - * .gwt.event.dom.client.MouseOverEvent) - */ - @Override - public void onMouseOver(MouseOverEvent event) { - cursorOverCalendarToggleButton = true; - } - - /* - * (non-Javadoc) - * - * @see - * com.google.gwt.event.dom.client.MouseOutHandler#onMouseOut(com.google - * .gwt.event.dom.client.MouseOutEvent) - */ - @Override - public void onMouseOut(MouseOutEvent event) { - cursorOverCalendarToggleButton = false; - } - } diff --git a/uitest/src/com/vaadin/tests/components/datefield/DateFieldPopupClosing.java b/uitest/src/com/vaadin/tests/components/datefield/DateFieldPopupClosing.java deleted file mode 100644 index 60508a30d4..0000000000 --- a/uitest/src/com/vaadin/tests/components/datefield/DateFieldPopupClosing.java +++ /dev/null @@ -1,43 +0,0 @@ -/* - * Copyright 2000-2014 Vaadin Ltd. - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may not - * use this file except in compliance with the License. You may obtain a copy of - * the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT - * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the - * License for the specific language governing permissions and limitations under - * the License. - */ -package com.vaadin.tests.components.datefield; - -import com.vaadin.server.VaadinRequest; -import com.vaadin.tests.components.AbstractTestUI; -import com.vaadin.ui.DateField; - -public class DateFieldPopupClosing extends AbstractTestUI { - - static final String DATEFIELD_ID = "datefield"; - - @Override - protected void setup(VaadinRequest request) { - final DateField df = new DateField(); - df.setId(DATEFIELD_ID); - addComponent(df); - } - - @Override - protected String getTestDescription() { - return "DateField popup should be closed when click on popup button"; - } - - @Override - protected Integer getTicketNumber() { - return 14857; - } - -} diff --git a/uitest/src/com/vaadin/tests/components/datefield/DateFieldPopupClosingTest.java b/uitest/src/com/vaadin/tests/components/datefield/DateFieldPopupClosingTest.java deleted file mode 100644 index ecbd6dd667..0000000000 --- a/uitest/src/com/vaadin/tests/components/datefield/DateFieldPopupClosingTest.java +++ /dev/null @@ -1,42 +0,0 @@ -package com.vaadin.tests.components.datefield; - -import java.io.IOException; - -import org.junit.Test; -import org.openqa.selenium.By; -import org.openqa.selenium.support.ui.ExpectedConditions; - -import com.vaadin.testbench.elements.DateFieldElement; -import com.vaadin.tests.tb3.MultiBrowserTest; - -public class DateFieldPopupClosingTest extends MultiBrowserTest { - /* - * try to open/close many times (not one time) because this defect is - * reproduced randomly (depends on timer) - */ - private static final int N = 100; - - @Test - public void testDateFieldPopupClosing() throws InterruptedException, - IOException { - openTestURL(); - - for (int i = 0; i < N; i++) { - clickDateDatePickerButton(); - - waitUntil(ExpectedConditions.visibilityOfElementLocated(By - .className("v-datefield-popup"))); - - clickDateDatePickerButton(); - - waitUntil(ExpectedConditions.invisibilityOfElementLocated(By - .className("v-datefield-popup"))); - } - } - - private void clickDateDatePickerButton() { - DateFieldElement dateField = $(DateFieldElement.class).first(); - dateField.findElement(By.tagName("button")).click(); - } - -} \ No newline at end of file -- cgit v1.2.3 From 828f1f0f20a296d97c693dd2acd004d4c90ad5cd Mon Sep 17 00:00:00 2001 From: Leif Åstrand Date: Sat, 20 Sep 2014 11:52:55 +0300 Subject: Don't iterate all connectors for unregistering (#14714) This reduces the time spent in unregisterRemovedConnectors when updating the text of one Label with about 380 connectors registered from 20 to 2 ms as reported by the Vaadin profiler. Performance when removing lots of connectors remains at about 10 ms for removing 360 connectors. Profiled in Firefox 32 on OS X. Change-Id: I203fd8790f8ccc7c098ee91c44831a5ac6c4a82b --- .../com/vaadin/client/ApplicationConnection.java | 80 ++++++++++++++-------- .../tests/performance/BasicPerformanceTest.java | 25 ++++++- 2 files changed, 76 insertions(+), 29 deletions(-) (limited to 'client') diff --git a/client/src/com/vaadin/client/ApplicationConnection.java b/client/src/com/vaadin/client/ApplicationConnection.java index cba9639067..d7c1c54a2d 100644 --- a/client/src/com/vaadin/client/ApplicationConnection.java +++ b/client/src/com/vaadin/client/ApplicationConnection.java @@ -135,6 +135,11 @@ public class ApplicationConnection implements HasHandlers { * Needed to know where captions might need to get updated */ private FastStringSet parentChangedIds = FastStringSet.create(); + + /** + * Connectors for which the parent has been set to null + */ + private FastStringSet detachedConnectorIds = FastStringSet.create(); } public static final String MODIFIED_CLASSNAME = "v-modified"; @@ -1624,7 +1629,7 @@ public class ApplicationConnection implements HasHandlers { json.getValueMap("dd")); } - unregisterRemovedConnectors(); + unregisterRemovedConnectors(connectorHierarchyUpdateResult.detachedConnectorIds); VConsole.log("handleUIDLMessage: " + (Duration.currentTimeMillis() - processUidlStart) @@ -1722,7 +1727,7 @@ public class ApplicationConnection implements HasHandlers { sendHierarchyChangeEvents(connectorHierarchyUpdateResult.events); // Unregister all the old connectors that have now been removed - unregisterRemovedConnectors(); + unregisterRemovedConnectors(connectorHierarchyUpdateResult.detachedConnectorIds); getLayoutManager().cleanMeasuredSizes(); } @@ -1877,47 +1882,63 @@ public class ApplicationConnection implements HasHandlers { Profiler.leave("sendStateChangeEvents"); } - private void unregisterRemovedConnectors() { - Profiler.enter("unregisterRemovedConnectors"); + private void verifyConnectorHierarchy() { + Profiler.enter("verifyConnectorHierarchy - this is only performed in debug mode"); - int unregistered = 0; JsArrayObject currentConnectors = connectorMap .getConnectorsAsJsArray(); int size = currentConnectors.size(); for (int i = 0; i < size; i++) { ServerConnector c = currentConnectors.get(i); if (c.getParent() != null) { - // only do this check if debug mode is active - if (ApplicationConfiguration.isDebugMode()) { - Profiler.enter("unregisterRemovedConnectors check parent - this is only performed in debug mode"); - // this is slow for large layouts, 25-30% of total - // time for some operations even on modern browsers - if (!c.getParent().getChildren().contains(c)) { - VConsole.error("ERROR: Connector is connected to a parent but the parent does not contain the connector"); - } - Profiler.leave("unregisterRemovedConnectors check parent - this is only performed in debug mode"); + if (!c.getParent().getChildren().contains(c)) { + VConsole.error("ERROR: Connector " + + c.getConnectorId() + + " is connected to a parent but the parent (" + + c.getParent().getConnectorId() + + ") does not contain the connector"); } } else if (c == getUIConnector()) { - // UIConnector for this connection, leave as-is + // UIConnector for this connection, ignore } else if (c instanceof WindowConnector && getUIConnector().hasSubWindow( (WindowConnector) c)) { - // Sub window attached to this UIConnector, leave - // as-is + // Sub window attached to this UIConnector, ignore } else { // The connector has been detached from the - // hierarchy, unregister it and any possible - // children. The UIConnector should never be - // unregistered even though it has no parent. - Profiler.enter("unregisterRemovedConnectors unregisterConnector"); - connectorMap.unregisterConnector(c); - Profiler.leave("unregisterRemovedConnectors unregisterConnector"); - unregistered++; + // hierarchy but was not unregistered. + VConsole.error("ERROR: Connector " + + c.getConnectorId() + + " is not attached to a parent but has not been unregistered"); } } - VConsole.log("* Unregistered " + unregistered + " connectors"); + Profiler.leave("verifyConnectorHierarchy - this is only performed in debug mode"); + } + + private void unregisterRemovedConnectors( + FastStringSet detachedConnectors) { + Profiler.enter("unregisterRemovedConnectors"); + + JsArrayString detachedArray = detachedConnectors.dump(); + for (int i = 0; i < detachedArray.length(); i++) { + ServerConnector connector = connectorMap + .getConnector(detachedArray.get(i)); + + Profiler.enter("unregisterRemovedConnectors unregisterConnector"); + connectorMap.unregisterConnector(connector); + Profiler.leave("unregisterRemovedConnectors unregisterConnector"); + } + + if (ApplicationConfiguration.isDebugMode()) { + // Do some extra checking if we're in debug mode (i.e. debug + // window is open) + verifyConnectorHierarchy(); + } + + VConsole.log("* Unregistered " + detachedArray.length() + + " connectors"); Profiler.leave("unregisterRemovedConnectors"); } @@ -2327,7 +2348,8 @@ public class ApplicationConnection implements HasHandlers { for (int i = 0; i < maybeDetachedArray.length(); i++) { ServerConnector removed = connectorMap .getConnector(maybeDetachedArray.get(i)); - recursivelyDetach(removed, result.events); + recursivelyDetach(removed, result.events, + result.detachedConnectorIds); } Profiler.leave("updateConnectorHierarchy detach removed connectors"); @@ -2339,7 +2361,9 @@ public class ApplicationConnection implements HasHandlers { } private void recursivelyDetach(ServerConnector connector, - JsArrayObject events) { + JsArrayObject events, + FastStringSet detachedConnectors) { + detachedConnectors.add(connector.getConnectorId()); /* * Reset state in an attempt to keep it consistent with the @@ -2400,7 +2424,7 @@ public class ApplicationConnection implements HasHandlers { if (child.getParent() != connector) { continue; } - recursivelyDetach(child, events); + recursivelyDetach(child, events, detachedConnectors); } Profiler.leave("ApplicationConnection recursivelyDetach perform detach"); diff --git a/uitest/src/com/vaadin/tests/performance/BasicPerformanceTest.java b/uitest/src/com/vaadin/tests/performance/BasicPerformanceTest.java index 04fe18fc08..a97f2611d1 100644 --- a/uitest/src/com/vaadin/tests/performance/BasicPerformanceTest.java +++ b/uitest/src/com/vaadin/tests/performance/BasicPerformanceTest.java @@ -4,6 +4,7 @@ import java.util.Iterator; import com.vaadin.server.VaadinRequest; import com.vaadin.tests.util.TestUtils; +import com.vaadin.ui.AbstractOrderedLayout; import com.vaadin.ui.Button; import com.vaadin.ui.Button.ClickEvent; import com.vaadin.ui.Component; @@ -18,6 +19,8 @@ import com.vaadin.ui.themes.Reindeer; public class BasicPerformanceTest extends UI { + private int updateOneCount = 0; + private final VerticalLayout contentLayout = new VerticalLayout(); private int clientLimit; @@ -64,7 +67,7 @@ public class BasicPerformanceTest extends UI { TestUtils.installPerformanceReporting(performanceReportArea); VerticalLayout leftBar = new VerticalLayout(); - leftBar.setSizeUndefined(); + leftBar.setWidth("250px"); leftBar.addComponent(new Label("This is the left bar")); leftBar.addComponent(performanceReportArea); leftBar.addComponent(reportPerformanceButton); @@ -126,6 +129,26 @@ public class BasicPerformanceTest extends UI { } })); + leftBar.addComponent(new Button("Update one label", + new Button.ClickListener() { + @Override + public void buttonClick(ClickEvent event) { + Component child = contentLayout.getComponent(0); + if (child instanceof Panel) { + Panel panel = (Panel) child; + child = panel.getContent(); + } + + AbstractOrderedLayout layout = (AbstractOrderedLayout) ((AbstractOrderedLayout) child) + .getComponent(0); + Label label = (Label) layout.getComponent(0); + + label.setValue("New value " + updateOneCount++); + + updatePerformanceReporting("Update one", 10, 10); + } + })); + leftBar.addComponent(new Button("Clear content", new Button.ClickListener() { @Override -- cgit v1.2.3 From 65a4fd14bff1043ab62c045d7d176d2097242fb2 Mon Sep 17 00:00:00 2001 From: Anna Miroshnik Date: Tue, 25 Nov 2014 12:19:05 +0300 Subject: Position tooltips in the visible area (#15129). Based on Mika's reverted patch, with additional fix and test for regression "an empty tooltip appears while the application is initializing". Change-Id: I8237fc9340265708a05a7576a5d9e8e374dc1fea --- client/src/com/vaadin/client/VTooltip.java | 23 ++- .../vaadin/tests/components/TooltipPosition.java | 70 +++++++++ .../tests/components/TooltipPositionTest.java | 173 +++++++++++++++++++++ .../tests/components/menubar/MenuTooltipTest.java | 1 + 4 files changed, 265 insertions(+), 2 deletions(-) create mode 100644 uitest/src/com/vaadin/tests/components/TooltipPosition.java create mode 100644 uitest/src/com/vaadin/tests/components/TooltipPositionTest.java (limited to 'client') diff --git a/client/src/com/vaadin/client/VTooltip.java b/client/src/com/vaadin/client/VTooltip.java index edd1273bf5..47a1b71228 100644 --- a/client/src/com/vaadin/client/VTooltip.java +++ b/client/src/com/vaadin/client/VTooltip.java @@ -210,6 +210,14 @@ public class VTooltip extends VOverlay { x = Window.getClientWidth() - offsetWidth - MARGIN + Window.getScrollLeft(); } + + if (tooltipEventMouseX != EVENT_XY_POSITION_OUTSIDE) { + // Do not allow x to be zero, for otherwise the tooltip + // does not close when the mouse is moved (see + // isTooltipOpen()). #15129 + int minX = Window.getScrollLeft() + MARGIN; + x = Math.max(x, minX); + } return x; } @@ -245,6 +253,14 @@ public class VTooltip extends VOverlay { y = Window.getScrollTop(); } } + + if (tooltipEventMouseY != EVENT_XY_POSITION_OUTSIDE) { + // Do not allow y to be zero, for otherwise the tooltip + // does not close when the mouse is moved (see + // isTooltipOpen()). #15129 + int minY = Window.getScrollTop() + MARGIN; + y = Math.max(y, minY); + } return y; } }); @@ -323,6 +339,7 @@ public class VTooltip extends VOverlay { setPopupPosition(tooltipEventMouseX, tooltipEventMouseY); } + private int EVENT_XY_POSITION_OUTSIDE = -5000; private int tooltipEventMouseX; private int tooltipEventMouseY; @@ -332,11 +349,13 @@ public class VTooltip extends VOverlay { } private int getEventX(Event event, boolean isFocused) { - return isFocused ? -5000 : DOM.eventGetClientX(event); + return isFocused ? EVENT_XY_POSITION_OUTSIDE : DOM + .eventGetClientX(event); } private int getEventY(Event event, boolean isFocused) { - return isFocused ? -5000 : DOM.eventGetClientY(event); + return isFocused ? EVENT_XY_POSITION_OUTSIDE : DOM + .eventGetClientY(event); } @Override diff --git a/uitest/src/com/vaadin/tests/components/TooltipPosition.java b/uitest/src/com/vaadin/tests/components/TooltipPosition.java new file mode 100644 index 0000000000..30222722d9 --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/TooltipPosition.java @@ -0,0 +1,70 @@ +/* + * Copyright 2000-2014 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.vaadin.tests.components; + +import com.vaadin.server.VaadinRequest; +import com.vaadin.ui.Button; +import com.vaadin.ui.UI; +import com.vaadin.ui.VerticalLayout; + +/** + * This UI is used for testing that a tooltip is not positioned partially + * outside the browser window when there is enough space to display it. + * + * @author Vaadin Ltd + */ +public class TooltipPosition extends AbstractTestUI { + + public static final int NUMBER_OF_BUTTONS = 5; + + @Override + protected void setup(VaadinRequest request) { + // These tooltip delay settings can be removed once #13854 is resolved. + getTooltipConfiguration().setOpenDelay(0); + getTooltipConfiguration().setQuickOpenDelay(0); + getTooltipConfiguration().setCloseTimeout(1000); + + VerticalLayout layout = new VerticalLayout(); + layout.setSpacing(true); + layout.setHeight(UI.getCurrent().getPage().getBrowserWindowHeight(), + Unit.PIXELS); + addComponent(layout); + for (int i = 0; i < NUMBER_OF_BUTTONS; i++) { + Button button = new Button("Button"); + button.setDescription(generateTooltipText()); + layout.addComponent(button); + } + } + + private String generateTooltipText() { + StringBuilder result = new StringBuilder(); + for (int i = 0; i < 50; i++) { + result.append("This is the line ").append(i) + .append(" of the long tooltip text.
"); + } + return result.toString(); + } + + @Override + public String getTestDescription() { + return "The tooltips of the buttons should not be clipped when there is enough space to display them."; + } + + @Override + public Integer getTicketNumber() { + return 15129; + } +} diff --git a/uitest/src/com/vaadin/tests/components/TooltipPositionTest.java b/uitest/src/com/vaadin/tests/components/TooltipPositionTest.java new file mode 100644 index 0000000000..4106374d64 --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/TooltipPositionTest.java @@ -0,0 +1,173 @@ +/* + * Copyright 2000-2014 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.vaadin.tests.components; + +import java.util.List; + +import org.junit.Assert; +import org.junit.Test; +import org.openqa.selenium.By; +import org.openqa.selenium.Dimension; +import org.openqa.selenium.Point; +import org.openqa.selenium.StaleElementReferenceException; +import org.openqa.selenium.WebDriver; +import org.openqa.selenium.WebDriver.Window; +import org.openqa.selenium.WebElement; +import org.openqa.selenium.interactions.Actions; +import org.openqa.selenium.support.ui.ExpectedCondition; + +import com.vaadin.testbench.elements.ButtonElement; +import com.vaadin.tests.tb3.MultiBrowserTest; + +/** + * Tests that the tooltip is positioned so that it fits in the displayed area. + * + * @author Vaadin Ltd + */ +public class TooltipPositionTest extends MultiBrowserTest { + + @Test + public void testRegression_EmptyTooltipShouldNotBeAppearedDuringInitialization() + throws Exception { + openTestURL(); + + waitForElementVisible(By.cssSelector(".v-tooltip")); + WebElement tooltip = driver.findElement(By.cssSelector(".v-tooltip")); + + Assert.assertTrue( + "This init tooltip with text ' ' is present in the DOM and should be entirely outside the browser window", + isOutsideOfWindow(tooltip)); + } + + @Test + public void testTooltipPosition() throws Exception { + openTestURL(); + for (int i = 0; i < TooltipPosition.NUMBER_OF_BUTTONS; i++) { + ButtonElement button = $(ButtonElement.class).get(i); + // Move the mouse to display the tooltip. + Actions actions = new Actions(driver); + actions.moveToElement(button, 10, 10); + actions.build().perform(); + waitUntil(tooltipToBeInsideWindow(By.cssSelector(".v-tooltip"), + driver.manage().window())); + + if (i < TooltipPosition.NUMBER_OF_BUTTONS - 1) { + // Remove the tooltip by moving the mouse. + actions = new Actions(driver); + actions.moveByOffset(300, 0); + actions.build().perform(); + waitUntil(tooltipNotToBeShown(By.cssSelector(".v-tooltip"), + driver.manage().window())); + } + } + } + + /* + * An expectation for checking that the tooltip found by the given locator + * is present in the DOM and entirely inside the browser window. The + * coordinate of the top left corner of the window is supposed to be (0, 0). + */ + private ExpectedCondition tooltipToBeInsideWindow( + final By tooltipLocator, final Window window) { + return new ExpectedCondition() { + + @Override + public Boolean apply(WebDriver input) { + List elements = findElements(tooltipLocator); + if (elements.isEmpty()) { + return false; + } + WebElement element = elements.get(0); + try { + if (!element.isDisplayed()) { + return false; + } + Point topLeft = element.getLocation(); + int xLeft = topLeft.getX(); + int yTop = topLeft.getY(); + if (xLeft < 0 || yTop < 0) { + return false; + } + Dimension elementSize = element.getSize(); + int xRight = xLeft + elementSize.getWidth() - 1; + int yBottom = yTop + elementSize.getHeight() - 1; + Dimension browserSize = window.getSize(); + return xRight < browserSize.getWidth() + && yBottom < browserSize.getHeight(); + } catch (StaleElementReferenceException e) { + return false; + } + } + + @Override + public String toString() { + return "the tooltip to be displayed inside the window"; + } + }; + }; + + /* + * An expectation for checking that the tooltip found by the given locator + * is not shown in the window, even partially. The top left corner of window + * should have coordinates (0, 0). + */ + private ExpectedCondition tooltipNotToBeShown( + final By tooltipLocator, final Window window) { + return new ExpectedCondition() { + + @Override + public Boolean apply(WebDriver input) { + List elements = findElements(tooltipLocator); + if (elements.isEmpty()) { + return true; + } + WebElement tooltip = elements.get(0); + try { + return isOutsideOfWindow(tooltip); + } catch (StaleElementReferenceException e) { + return true; + } + } + + @Override + public String toString() { + return "the tooltip not to be displayed inside the window"; + } + + }; + } + + private boolean isOutsideOfWindow(WebElement tooltip) { + if (!tooltip.isDisplayed()) { + return true; + } + // The tooltip is shown, at least partially, if + // its intervals of both horizontal and vertical coordinates + // overlap those of the window. + Point topLeft = tooltip.getLocation(); + Dimension tooltipSize = tooltip.getSize(); + Dimension windowSize = driver.manage().window().getSize(); + int xLeft = topLeft.getX(); + int yTop = topLeft.getY(); + int xRight = xLeft + tooltipSize.getWidth() - 1; + int yBottom = yTop + tooltipSize.getHeight() - 1; + boolean overlapHorizontally = !(xRight < 0 || xLeft >= windowSize + .getWidth()); + boolean overlapVertically = !(yBottom < 0 || yTop >= windowSize + .getHeight()); + return !(overlapHorizontally && overlapVertically); + } +} \ No newline at end of file diff --git a/uitest/src/com/vaadin/tests/components/menubar/MenuTooltipTest.java b/uitest/src/com/vaadin/tests/components/menubar/MenuTooltipTest.java index 24025b9f39..091f7be954 100644 --- a/uitest/src/com/vaadin/tests/components/menubar/MenuTooltipTest.java +++ b/uitest/src/com/vaadin/tests/components/menubar/MenuTooltipTest.java @@ -49,6 +49,7 @@ public class MenuTooltipTest extends MultiBrowserTest { Coordinates elementCoordinates = getCoordinates($(MenuBarElement.class) .first()); + sleep(1000); Mouse mouse = ((HasInputDevices) getDriver()).getMouse(); -- cgit v1.2.3 From d9829d6636e046b452fdf6f93194c55db9c3997c Mon Sep 17 00:00:00 2001 From: Sergey Budkin Date: Fri, 7 Nov 2014 12:38:05 +0200 Subject: One pixel discrepancy in headers and rows border with minimal width (#15118) Disabled old patch for VTreeTable, which caused the problem. Change-Id: I3fdf6b0890307b27e32ceff4492cb7d0bfc6b680 --- client/src/com/vaadin/client/ui/VScrollTable.java | 3 +- client/src/com/vaadin/client/ui/VTreeTable.java | 2 + .../components/treetable/MinimalWidthColumns.java | 44 ++++++++++++++++++++++ .../treetable/MinimalWidthColumnsTest.java | 31 +++++++++++++++ 4 files changed, 79 insertions(+), 1 deletion(-) create mode 100644 uitest/src/com/vaadin/tests/components/treetable/MinimalWidthColumns.java create mode 100644 uitest/src/com/vaadin/tests/components/treetable/MinimalWidthColumnsTest.java (limited to 'client') diff --git a/client/src/com/vaadin/client/ui/VScrollTable.java b/client/src/com/vaadin/client/ui/VScrollTable.java index 6c241f1033..14e4c658ad 100644 --- a/client/src/com/vaadin/client/ui/VScrollTable.java +++ b/client/src/com/vaadin/client/ui/VScrollTable.java @@ -5392,6 +5392,7 @@ public class VScrollTable extends FlowPanel implements HasWidgets, private Map cellToolTips = new HashMap(); private boolean isDragging = false; private String rowStyle = null; + protected boolean applyZeroWidthFix = true; private VScrollTableRow(int rowKey) { this.rowKey = rowKey; @@ -5497,7 +5498,7 @@ public class VScrollTable extends FlowPanel implements HasWidgets, * definition of zero width table cells. Instead, use 1px * and compensate with a negative margin. */ - if (width == 0) { + if (applyZeroWidthFix && width == 0) { wrapperWidth = 1; wrapperStyle.setMarginRight(-1, Unit.PX); } else { diff --git a/client/src/com/vaadin/client/ui/VTreeTable.java b/client/src/com/vaadin/client/ui/VTreeTable.java index 9b7e9702b2..9e5940a2f2 100644 --- a/client/src/com/vaadin/client/ui/VTreeTable.java +++ b/client/src/com/vaadin/client/ui/VTreeTable.java @@ -155,6 +155,8 @@ public class VTreeTable extends VScrollTable { public VTreeTableRow(UIDL uidl, char[] aligns2) { super(uidl, aligns2); + // this fix causes #15118 and doesn't work for treetable anyway + applyZeroWidthFix = false; } @Override diff --git a/uitest/src/com/vaadin/tests/components/treetable/MinimalWidthColumns.java b/uitest/src/com/vaadin/tests/components/treetable/MinimalWidthColumns.java new file mode 100644 index 0000000000..c4679f739b --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/treetable/MinimalWidthColumns.java @@ -0,0 +1,44 @@ +package com.vaadin.tests.components.treetable; + +import com.vaadin.annotations.Theme; +import com.vaadin.server.VaadinRequest; +import com.vaadin.tests.components.AbstractTestUI; +import com.vaadin.ui.TreeTable; + +@Theme("valo") +public class MinimalWidthColumns extends AbstractTestUI { + + /* + * (non-Javadoc) + * + * @see com.vaadin.tests.components.AbstractTestUI#setup(com.vaadin.server. + * VaadinRequest) + */ + @Override + protected void setup(VaadinRequest request) { + TreeTable tt = new TreeTable(); + tt.addContainerProperty("Foo", String.class, ""); + tt.addContainerProperty("Bar", String.class, ""); + + Object item1 = tt.addItem(new Object[] { "f", "Bar" }, null); + Object item2 = tt.addItem(new Object[] { "Foo2", "Bar2" }, null); + + tt.setParent(item2, item1); + + tt.setColumnWidth("Foo", 0); + tt.setColumnWidth("Bar", 50); + tt.setWidth("300px"); + addComponent(tt); + } + + @Override + protected Integer getTicketNumber() { + return Integer.valueOf(15118); + } + + @Override + protected String getTestDescription() { + return "There should be no 1px discrepancy between vertical borders in headers and rows"; + } + +} diff --git a/uitest/src/com/vaadin/tests/components/treetable/MinimalWidthColumnsTest.java b/uitest/src/com/vaadin/tests/components/treetable/MinimalWidthColumnsTest.java new file mode 100644 index 0000000000..46c2f397b6 --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/treetable/MinimalWidthColumnsTest.java @@ -0,0 +1,31 @@ +/* + * Copyright 2000-2014 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.vaadin.tests.components.treetable; + +import org.junit.Test; + +import com.vaadin.tests.tb3.MultiBrowserTest; + +public class MinimalWidthColumnsTest extends MultiBrowserTest { + + @Test + public void testFor1pxDifference() throws Exception { + openTestURL(); + sleep(500); + compareScreen("onepixdifference"); + } + +} -- cgit v1.2.3