GwtMaterialDesign / gwt-material

A Google Material Design wrapper for GWT
https://gwtmaterialdesign.github.io/gmd-core-demo/
Apache License 2.0
411 stars 124 forks source link

MaterialCollection: the state of inner MaterialSwitches doesn't change when the user clicks on the switch #192

Closed gilberto-torrezan closed 8 years ago

gilberto-torrezan commented 8 years ago

That can be simulated at the demo app. At the "Clickable Items with Secondary Controls" section, when the user clicks directly on the switch (instead of any other part of the MaterialCollectionItem), the change event is triggered, the toast appears, but the switch doesn't switch states (from on to off or from off to on).

When clicked on any part of the MaterialCollectionItem, the switch behave accordingly.

Tested on Firefox and Chrome.

BenDol commented 8 years ago

Ah good catch.

BenDol commented 8 years ago

Its activating the switch then deactivating it straight away. Will just stop propagation on the collection item click.

kevzlou7979 commented 8 years ago

Yeah I agree just stop propagation then it solves the problem. I will update the showcase site.

BenDol commented 8 years ago

Hasn't been resolved yet, will submit the fix later tonight.

kevzlou7979 commented 8 years ago

@BenDol Ah ok no problem.

Kusig commented 8 years ago

Debugged this as well. Found that the ClickListener ist multiple times registered because dependent on the usage, the onLoad is called multiple times. I guess it should register only once ever.

@Override protected void onLoad() { super.onLoad();

    addClickHandler(new ClickHandler() {
        @Override
        public void onClick(ClickEvent event) {
            setValue(!getValue());
            event.preventDefault();
        }
    });
}
BenDol commented 8 years ago

Yeah needs to be changed.

Kusig commented 8 years ago

I've created pull request #199 about. Don't know if this is the only place where this might be a problem. Probably there are other components having the same problem.

BenDol commented 8 years ago

There shouldn't be other components with this problem. Was something that slipped between the cracks haha, but create an issue if you find one.

Kusig commented 8 years ago

sorry, I'm still fighting with git ... Pull Request #200

BenDol commented 8 years ago

Has this been resolved?

kevzlou7979 commented 8 years ago

Is there any update for this before we can release 1.4.1?

Kusig commented 8 years ago

This has been resolved from my point of view. with the Pull Request #204. It is working fine so far.

gilberto-torrezan commented 8 years ago

Well this isn't fixed for me (I'm on release_1.5 branch). The proper fix is to add a event.stopPropagation(); to the main ClickHandler on MaterialSwitch (line 84).