From 84fbff9e6645d0133c7b9f214389e215c79c818f Mon Sep 17 00:00:00 2001 From: Artur Signell Date: Mon, 29 Aug 2016 22:11:23 +0300 Subject: [PATCH] Fix animation end listeners so they are always removed (#17903) Fixes ComboBox suggestion popup so that it will not automatically close when clicking the popup button, if the user happened to double click on the button earlier. Ported from 7.7 Change-Id: I6cd8c7744ca4c52a7bd52ab12c23fb55522f0611 --- .../java/com/vaadin/client/AnimationUtil.java | 27 +++++++++++ .../com/vaadin/client/widgets/Overlay.java | 28 ++++++----- .../combobox/ComboBoxValoDoubleClick.java | 48 +++++++++++++++++++ 3 files changed, 91 insertions(+), 12 deletions(-) create mode 100644 uitest/src/main/java/com/vaadin/tests/components/combobox/ComboBoxValoDoubleClick.java diff --git a/client/src/main/java/com/vaadin/client/AnimationUtil.java b/client/src/main/java/com/vaadin/client/AnimationUtil.java index 652cfd9432..bbefcffbdd 100644 --- a/client/src/main/java/com/vaadin/client/AnimationUtil.java +++ b/client/src/main/java/com/vaadin/client/AnimationUtil.java @@ -19,6 +19,7 @@ import com.google.gwt.core.client.JavaScriptObject; import com.google.gwt.dom.client.Element; import com.google.gwt.dom.client.NativeEvent; import com.google.gwt.dom.client.Style; +import com.vaadin.client.AnimationUtil.AnimationEndListener; /** * Utility methods for working with CSS transitions and animations. @@ -65,6 +66,7 @@ public class AnimationUtil { var callbackFunc = $entry(function(e) { listener.@com.vaadin.client.AnimationUtil.AnimationEndListener::onAnimationEnd(Lcom/google/gwt/dom/client/NativeEvent;)(e); }); + callbackFunc.listener = listener; elem.addEventListener(@com.vaadin.client.AnimationUtil::ANIMATION_END_EVENT_NAME, callbackFunc, false); @@ -84,6 +86,31 @@ public class AnimationUtil { elem.removeEventListener(@com.vaadin.client.AnimationUtil::ANIMATION_END_EVENT_NAME, listener, false); }-*/; + /** + * Removes the given animation listener. + * + * @param element + * the element which has the listener + * @param animationEndListener + * the listener to remove + * @return true if the listener was removed, false + * if the listener was not registered to the given element + */ + public static native boolean removeAnimationEndListener(Element elem, + AnimationEndListener animationEndListener) + /*-{ + if(elem._vaadin_animationend_callbacks) { + var callbacks = elem._vaadin_animationend_callbacks; + for(var i=0; i < callbacks.length; i++) { + if (callbacks[i].listener == animationEndListener) { + elem.removeEventListener(@com.vaadin.client.AnimationUtil::ANIMATION_END_EVENT_NAME, callbacks[i], false); + return true; + } + } + return false; + } + }-*/; + /** For internal use only. May be removed or replaced in the future. */ public static native void removeAllAnimationEndListeners(Element elem) /*-{ diff --git a/client/src/main/java/com/vaadin/client/widgets/Overlay.java b/client/src/main/java/com/vaadin/client/widgets/Overlay.java index 35a5b46576..0b1d317a05 100644 --- a/client/src/main/java/com/vaadin/client/widgets/Overlay.java +++ b/client/src/main/java/com/vaadin/client/widgets/Overlay.java @@ -21,7 +21,6 @@ import java.util.List; import com.google.gwt.animation.client.Animation; import com.google.gwt.core.client.GWT; -import com.google.gwt.core.client.JavaScriptObject; import com.google.gwt.dom.client.Document; import com.google.gwt.dom.client.Element; import com.google.gwt.dom.client.IFrameElement; @@ -396,8 +395,6 @@ public class Overlay extends PopupPanel { current = null; } - private JavaScriptObject animateInListener; - private boolean fitInWindow = false; private boolean maybeShowWithAnimation() { @@ -422,16 +419,18 @@ public class Overlay extends PopupPanel { if (animationName.contains(ADDITIONAL_CLASSNAME_ANIMATE_IN)) { // Disable GWT PopupPanel animation if used setAnimationEnabled(false); - animateInListener = AnimationUtil.addAnimationEndListener( - getElement(), new AnimationEndListener() { + AnimationUtil.addAnimationEndListener(getElement(), + new AnimationEndListener() { @Override public void onAnimationEnd(NativeEvent event) { String animationName = AnimationUtil .getAnimationName(event); if (animationName.contains( ADDITIONAL_CLASSNAME_ANIMATE_IN)) { - AnimationUtil.removeAnimationEndListener( - getElement(), animateInListener); + boolean removed = AnimationUtil + .removeAnimationEndListener( + getElement(), this); + assert removed : "Animation end listener was not removed"; removeStyleDependentName( ADDITIONAL_CLASSNAME_ANIMATE_IN); } @@ -720,6 +719,10 @@ public class Overlay extends PopupPanel { public void onAnimationEnd(NativeEvent event) { if (AnimationUtil.getAnimationName(event).contains( ADDITIONAL_CLASSNAME_ANIMATE_IN)) { + boolean removed = AnimationUtil + .removeAnimationEndListener( + getElement(), this); + assert removed : "Animation end listener was not removed"; reallyHide(autoClosed); } } @@ -746,11 +749,12 @@ public class Overlay extends PopupPanel { .getAnimationName(event); if (animationName.contains( ADDITIONAL_CLASSNAME_ANIMATE_OUT)) { - AnimationUtil - .removeAllAnimationEndListeners( - getElement()); - // Remove both animation styles just in - // case + boolean removed = AnimationUtil + .removeAnimationEndListener( + getElement(), this); + assert removed : "Animation end listener was not removed"; + + // Remove both animation styles just in case removeStyleDependentName( ADDITIONAL_CLASSNAME_ANIMATE_IN); removeStyleDependentName( diff --git a/uitest/src/main/java/com/vaadin/tests/components/combobox/ComboBoxValoDoubleClick.java b/uitest/src/main/java/com/vaadin/tests/components/combobox/ComboBoxValoDoubleClick.java new file mode 100644 index 0000000000..7b32b5e570 --- /dev/null +++ b/uitest/src/main/java/com/vaadin/tests/components/combobox/ComboBoxValoDoubleClick.java @@ -0,0 +1,48 @@ +/* + * 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.combobox; + +import com.vaadin.annotations.Theme; +import com.vaadin.server.VaadinRequest; +import com.vaadin.tests.components.AbstractTestUI; +import com.vaadin.v7.ui.ComboBox; + +@Theme("valo") +public class ComboBoxValoDoubleClick extends AbstractTestUI { + + // Quite impossible to autotest reliably as there must be a click to open + // the popup and another click during the opening animation to reproduce the + // bug. Manually a double click is just about the right timing. + @Override + protected void setup(VaadinRequest request) { + ComboBox cb = new ComboBox("Double-click Me"); + for (int i = 0; i < 100; i++) { + cb.addItem("Item-" + i); + } + addComponent(cb); + } + + @Override + public String getTestDescription() { + return "ComboBox should remain usable even after double-clicking (affects only Valo theme with $v-overlay-animate-in)."; + } + + @Override + protected Integer getTicketNumber() { + return 17903; + } + +} -- 2.39.5