gwtbootstrap3 / gwtbootstrap3-extras

Extra (third party) wrappers for GwtBootstrap3
Apache License 2.0
43 stars 89 forks source link

select.client.ui.SelectBase: change handler added on every call to SelectBase.onLoad #355

Open psefranek opened 5 years ago

psefranek commented 5 years ago

After a little bit of testing i found out that jQuery change handler (handler for native javascript event of type "change") is added to underlying select element every time method SelectBase.onLoad is called. However this handler is not removed during call to SelectBase.onUnload (which itself calls private method SelectBase.unbindSelectEvents). This leads to undesired behavior that one user interaction fires ChangeEvent many times (as many as SelectBase.onUnload was called).

To fix this issue my advice is to add this one line to method org.gwtbootstrap3.extras.select.client.ui.SelectBase.unbindSelectEvents. This line would be: $wnd.jQuery(e).off("change"); (or we can use constant instead of string literal "change").

I am also providing example source code documenting described issue. Run it with browser's console opened and see the logs. Then wait few seconds to let timers complete. After that click on the Select widget and you can clearly see that ValueChangeHandler is called twice.

Also in the browser' console window we can see that event handler for "change" event is still here even if SelectBase.onUnload was called.

Here is example source code:

EntryPoint class

import org.gwtbootstrap3.extras.select.client.ui.MultipleSelect;
import org.gwtbootstrap3.extras.select.client.ui.Option;
import org.gwtbootstrap3.extras.select.client.ui.Select;

import com.google.gwt.core.client.EntryPoint;
import com.google.gwt.dom.client.Element;
import com.google.gwt.event.logical.shared.ValueChangeEvent;
import com.google.gwt.event.logical.shared.ValueChangeHandler;
import com.google.gwt.user.client.Timer;
import com.google.gwt.user.client.Window;
import com.google.gwt.user.client.ui.RootPanel;
import com.google.gwt.user.client.ui.SimplePanel;

public class TestEntrypoint implements EntryPoint {

    @Override
    public void onModuleLoad() {
        RootPanel rp = RootPanel.get("mainDiv");

        SimplePanel content = new SimplePanel();
        rp.add(content);

        Select s = new LoggingSelect();
        Option o1 = new Option();
        o1.setText("first");
        o1.setValue("first");
        s.add(o1);
        Option o2 = new Option();
        o2.setText("second");
        o2.setValue("second");
        s.add(o2);

        s.addValueChangeHandler(new ValueChangeHandler<String>() {
            @Override
            public void onValueChange(ValueChangeEvent<String> event) {
                Window.alert(event.getValue());
            }
        });

        MultipleSelect ms = new LoggingMultipleSelect();

        content.setWidget(s);

        new Timer() {
            @Override
            public void run() {
                content.setWidget(ms);
            }
        }.schedule(1_500);

        new Timer() {
            @Override
            public void run() {
                content.setWidget(s);
            }
        }.schedule(3_000);
    }

    private static class LoggingSelect extends Select {

        @Override
        protected void onLoad() {
            logHandler("Select BEFORE load", getElement());
            super.onLoad();
            logHandler("Select AFTER load", getElement());
        }       

        @Override
        protected void onUnload() {
            logHandler("Select BEFORE unload", getElement());
            super.onUnload();
            logHandler("Select AFTER unload", getElement());
        }

    }

    private static class LoggingMultipleSelect extends MultipleSelect {

        @Override
        protected void onLoad() {
            logHandler("MultipleSelect BEFORE load", getElement());
            super.onLoad();
            logHandler("MultipleSelect AFTER load", getElement());
        }

        @Override
        protected void onUnload() {
            logHandler("MultipleSelect BEFORE unload", getElement());
            super.onUnload();
            logHandler("MultipleSelect AFTER unload", getElement());
        }

    }

    private static native void logHandler(String logMsg, Element e) /*-{
        console.log(logMsg, $wnd.jQuery._data(e, "events"));
    }-*/;

}

GWT module

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE module PUBLIC "-//Google Inc.//DTD Google Web Toolkit 2.8.2//EN" 
    "http://www.gwtproject.org/doctype/2.8.2/gwt-module.dtd">
<module rename-to="test">
    <inherits name="com.google.gwt.user.User" />
    <inherits name="org.gwtbootstrap3.GwtBootstrap3" />
    <inherits name="org.gwtbootstrap3.extras.select.Select"/>   

    <entry-point class="com.example.client.TestEntrypoint" />   
</module>

index.html

<!doctype html>
<html>
    <head>
        <script type="text/javascript" src="test/test.nocache.js"></script>
    </head>
    <body>
        <div id="mainDiv"></div>
    </body>
</html>

Maven dependencies

<dependencies>
        <dependency>
            <groupId>com.google.gwt</groupId>
            <artifactId>gwt-user</artifactId>
            <version>2.8.2</version>
        </dependency>

        <dependency>
            <groupId>com.google.gwt</groupId>
            <artifactId>gwt-dev</artifactId>
            <version>2.8.2</version>
        </dependency>

        <dependency>
            <groupId>org.gwtbootstrap3</groupId>
            <artifactId>gwtbootstrap3</artifactId>
            <version>0.9.4</version>
        </dependency>

        <dependency>
            <groupId>org.gwtbootstrap3</groupId>
            <artifactId>gwtbootstrap3-extras</artifactId>
            <version>0.9.4</version>
        </dependency>
  </dependencies>
eric-eldard commented 5 years ago

Experiencing the exact same issue w/ same dependency versions. @psefranek – added your fix in a wrapper class and it works great. Thanks!