vaadin / vaadin-button-flow

Vaadin Flow Java API for vaadin/vaadin-button Web Component
https://vaadin.com/components/vaadin-button
Other
0 stars 7 forks source link

re-enable a DisableOnClick button #115

Closed tbee closed 5 years ago

tbee commented 5 years ago

I have the DisableOnClick of a button set to true, and in its event listener I check some textfields and decide to show a popup with an error. In that case the button needs to be enabled again, but setEnabled(true) does not do that. There is no documentation which explains how to re-enable the button.

maticpetek commented 5 years ago

Code example :

Button button = new Button("Click me",
               event -> {
                mySlowMethod();
                event.getSource().setEnabled(true);
                });
       button.setDisableOnClick(true);
maticpetek commented 5 years ago

Reference on discussion - https://vaadin.com/forum/thread/17503191/17504128

tbee commented 5 years ago

Also reference on discussion - https://vaadin.com/forum/thread/17481667/re-enable-a-disableonclick-button

T3rm1 commented 5 years ago

So why is no one going to fix this issues? It's a blocker and should be hotfixed asap.

sandronm commented 5 years ago

This bug is really painful for our project. I hope it will be fixed asap :-)

pleku commented 5 years ago

I think I have a pretty good guess what the issue is - will take a look at it tomorrow or during the weekend.

pleku commented 5 years ago

The issue was as suspected but there is no "good way" for fixing it without new API for Flow.

Looking at possible workarounds that can be backported, but don't make me feel too bad for applying those.e There are some alternatives for this.

sandronm commented 5 years ago

What are the possible workarounds now?

fschon commented 5 years ago

Having to manually setDisableOnClick(true) and then re-enable it for every button to prevent double clicks is very tedious. Shouldn't the default behavior be that the button is effectively disabled until the server click event code is complete, and then delete second (or more) click and re-enable the button? If the code had disabled the button in the event then leave the button disabled.

T3rm1 commented 5 years ago

Yes, I think this should be the default, too.

ZheSun88 commented 5 years ago

Hi @fschon and @T3rm1 , Thanks for the feedback on this fix. As this ticket has been closed now, I would suggest you fire a new one to discuss this, so that it is easier for us to tracking the changes and also easier for you to get help. We'd like to hear your voice on this issue, thanks again.

nemanjakmno commented 4 years ago

setDisableOnClick on flow button not working on fast double or triple clicks. Vaadin version 14.1.27. Managed to print up to 3 fast clicks on disabled button. Are you planning to fix this any soon and provide some good solution for e.g. timeout on button disable on click.

I managed to achieve this somehow but still not very reliable:

private static void setDisableOnClickTimeoutAsyncEnable(ClickEvent<?> event , Long timeout){

    Component c = event.getSource();

    c.getElement().setEnabled(false);

    c.getUI().ifPresent(ui -> ui.access(() 
            -> ui.push()));

    CompletableFuture.runAsync(()->{
        try {
            Thread.sleep(timeout!=null && timeout > 0 ? timeout : DISABLE_ON_CLICK_TIMEOUT_DEFAULT);
        }catch (InterruptedException ex) {
        }finally {
            c.getUI().ifPresent(ui -> ui.access(
                    ()-> {
                        c.getElement().setEnabled(true);
                        ui.push();
                    }
            ));

        }

    });

    //additional check with simple thread
    new Thread(()->{
        try {
            Thread.sleep(timeout!=null && timeout > 0 ? (2 * timeout) : 
                (2 * DISABLE_ON_CLICK_TIMEOUT_DEFAULT));
        }catch (InterruptedException e) {
        }finally {
            if(!c.getElement().isEnabled()) {
                System.out.println("WARN: setDisableOnClickTimeoutAsyncEnable: "
                        + "enabling button via thread. This should never happen! ");
                c.getUI().ifPresent(ui -> ui.access(
                        ()-> c.getElement().setEnabled(true)
                    ));
            }
        }

    }).start();

}
pleku commented 4 years ago

@fschon @T3rm1 @kolle1986 the solution for both, not having to use setDisableOnClick(true) &setEnabled(true)andsetDisableOnClick(true)` not being able to prevent "too fast" clicks, could be what is proposed in #98 so please +1 and comment on that instead. I think it is a valid concern and should get attention.