Alex
Alex

Reputation: 5904

Disable JButton, while background job, to avoid multiple clicks

I need to stop user making multiple clicks on a JButton while the first click still execute.

I was able to came with a solution for this issue but I do not completelly understand why it's working.

Bellow I posted the code (trimmed to a minimum) that works and the one that does not work.

In first example (good) if you run it and click the button multiple times only one action is considered as for the second example (bad) if you click the mouse multiple times you get action executed at least twice.

The second (bad) example simply does not use invokeLater() method.

Where the difference in behaviour cames from?

import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.util.logging.Level;
import java.util.logging.Logger;
import javax.swing.*;

public class TestButtonTask {

    public static void main(String[] args) {

        final JFrame frame = new JFrame("Test");
        frame.setDefaultCloseOperation(WindowConstants.DISPOSE_ON_CLOSE);

        final JButton task = new JButton("Test");

        task.addActionListener(new ActionListener() {

            @Override
            public void actionPerformed(ActionEvent e) {
                long t = System.currentTimeMillis();
                System.out.println("Action received");

                task.setText("Working...");
                task.setEnabled(false);

                SwingUtilities.invokeLater(new Thread() {

                    @Override
                    public void run() {
                        try {
                            sleep(2 * 1000);
                        } catch (InterruptedException ex) {
                            Logger.getLogger(TestButtonTask.class.getName()).log(Level.SEVERE, null, ex);
                        }

                        SwingUtilities.invokeLater(new Runnable() {

                            public void run() {
                                task.setEnabled(true);
                                task.setText("Test");
                            }
                        });

                    }
                });
            }
        });

        frame.add(task);
        frame.pack();
        frame.setVisible(true);
    } //end main
} //end class

And now the "wrong" code

import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.util.logging.Level;
import java.util.logging.Logger;
import javax.swing.*;

public class TestButtonTask {

    public static void main(String[] args) {

        final JFrame frame = new JFrame("Test");
        frame.setDefaultCloseOperation(WindowConstants.DISPOSE_ON_CLOSE);

        final JButton task = new JButton("Test");

        task.addActionListener(new ActionListener() {

            @Override
            public void actionPerformed(ActionEvent e) {
                long t = System.currentTimeMillis();
                System.out.println("Action received");

                task.setText("Working...");
                task.setEnabled(false);

                SwingUtilities.invokeLater(new Thread() {

                    @Override
                    public void run() {
                        try {
                            sleep(2 * 1000);
                        } catch (InterruptedException ex) {
                            Logger.getLogger(TestButtonTask.class.getName()).log(Level.SEVERE, null, ex);
                        }

                        //SwingUtilities.invokeLater(new Runnable() {

                            //public void run() {
                                task.setEnabled(true);
                                task.setText("Test");
                            //}
                        //});

                    }
                });
            }
        });

        frame.add(task);
        frame.pack();
        frame.setVisible(true);
    } //end main
} //end class

After info provided by @kleopatra and @Boris Pavlović here is the code I created that seems to work pretty decent.

import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.beans.PropertyChangeEvent;
import java.beans.PropertyChangeListener;
import java.util.logging.Level;
import java.util.logging.Logger;
import javax.swing.*;

public class TestButtonTask {

    public static void main(String[] args) {

        final JFrame frame = new JFrame("Test");
        frame.setDefaultCloseOperation(WindowConstants.DISPOSE_ON_CLOSE);

        final JButton task = new JButton("Test");

        task.addActionListener(new ActionListener() {

            @Override
            public void actionPerformed(ActionEvent e) {
                task.setText("Working...");
                task.setEnabled(false);

                SwingWorker worker = new SwingWorker<Void, Void>() {

                    @Override
                    protected Void doInBackground() throws Exception {
                        try {
                            Thread.sleep(3 * 1000);
                        } catch (InterruptedException ex) {
                            Logger.getLogger(TestButtonTask.class.getName()).log(Level.SEVERE, null, ex);
                        }

                        return null;
                    }                    
                };

                worker.addPropertyChangeListener(new PropertyChangeListener() {
                    @Override
                    public void propertyChange(PropertyChangeEvent evt) {
                        System.out.println("Event " + evt + " name" + evt.getPropertyName() + " value " + evt.getNewValue());
                        if ("DONE".equals(evt.getNewValue().toString())) {
                            task.setEnabled(true);
                            task.setText("Test");
                        }
                    }
                });

                worker.execute();
            }
        });

        frame.add(task);
        frame.pack();
        frame.setVisible(true);
    } //end main
} //end class

Upvotes: 8

Views: 13361

Answers (6)

ryvantage
ryvantage

Reputation: 13479

After years of dealing with the frustration of this problem, I've implemented a solution that I think is the best.

First, why nothing else works:

  1. JButton::setMutliclickThreshold() is not really an optimal solution, because (as you said) there is no way to know how long to set the threshold. This is only good to guard against double-click happy end-users because you have to set an arbitrary threshold.
  2. JButton::setEnabled() is an obviously fragile solution that will only make life much more difficult.

So, I've created the SingletonSwingWorker. Now, Singletons are called anti-patterns, but if implemented properly, they can be a very powerful. Here is the code:

public abstract class SingletonSwingWorker extends SwingWorker {

    abstract void initAndGo();

    private static HashMap<Class, SingletonSwingWorker> workers;
    public static void runWorker(SingletonSwingWorker newInstance) {
        if(workers == null) {
            workers = new HashMap<>();
        }
        if(!workers.containsKey(newInstance.getClass()) || workers.get(newInstance.getClass()).isDone()) {
            workers.put(newInstance.getClass(), newInstance);
            newInstance.initAndGo();
        }
    }
}

This will enable you to create classes which extend SingletonSwingWorker and guarantee only one instance of that class will be executable at one time. Here is an example implementation:

public static void main(String[] args) {
    final JFrame frame = new JFrame();
    JButton button = new JButton("Click");
    button.setMultiClickThreshhold(5);
    button.addActionListener(new ActionListener() {
        @Override
        public void actionPerformed(ActionEvent e) {
            DisplayText_Task.runWorker(new DisplayText_Task(frame));
        }
    });

    JPanel panel = new JPanel();
    panel.add(button);
    frame.add(panel);
    frame.pack();
    frame.setLocationRelativeTo(null);
    frame.setVisible(true);
    frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
}

static class DisplayText_Task extends SingletonSwingWorker {

    JFrame dialogOwner;
    public DisplayText_Task(JFrame dialogOwner) {
        this.dialogOwner = dialogOwner;
    }

    JDialog loadingDialog;
    @Override
    void initAndGo() {
        loadingDialog = new JDialog(dialogOwner);
        JProgressBar jpb = new JProgressBar();
        jpb.setIndeterminate(true);
        loadingDialog.add(jpb);
        loadingDialog.pack();
        loadingDialog.setVisible(true);
        execute(); // This must be put in the initAndGo() method or no-workie
    }

    @Override
    protected Object doInBackground() throws Exception {
        for(int i = 0; i < 100; i++) {
            System.out.println(i);
            Thread.sleep(200);
        }
        return null;
    }

    @Override
    protected void done() {
        if(!isCancelled()) {
            try {
                get();
            } catch (ExecutionException | InterruptedException e) {
                loadingDialog.dispose();
                e.printStackTrace();
                return;
            }
            loadingDialog.dispose();
        } else
            loadingDialog.dispose();
    }

}

In my SwingWorker implementations, I like to load a JProgressBar, so I always do that before running doInBackground(). With this implementation, I load the JProgressBar inside the initAndGo() method and I also call execute(), which must be placed in the initAndGo() method or the class will not work.

Anyways, I think this is a good solution and it shouldn't be that hard to refactor code to refit your applications with it.

Very interested in feedback on this solution.

Upvotes: 1

kleopatra
kleopatra

Reputation: 51524

Okay, here's a code snippet using an Action

  • it disable's itself on performed
  • it spawns a task, at the end of which is enables itself again. Note: for simplicity here the task is simulated by a Timer, real-world would spawn a SwingWorker to do the background work, listening to its property changes and enable itself on receiving a done
  • set as the button's action

The code:

    Action taskAction = new AbstractAction("Test") {

        @Override
        public void actionPerformed(ActionEvent e) {
            System.out.println("Action received ");
            setEnabled(false);
            putValue(NAME, "Working...");
            startTask();
        }

        // simulate starting a task - here we simply use a Timer
        // real-world code would spawn a SwingWorker
        private void startTask() {
            ActionListener l = new ActionListener() {
                @Override
                public void actionPerformed(ActionEvent e) {
                    putValue(NAME, "Test");
                    setEnabled(true);

                }
            };
            Timer timer = new Timer(2000, l);
            timer.setRepeats(false);
            timer.start();
        }};

     JButton task = new JButton(taskAction);

Upvotes: 2

mKorbel
mKorbel

Reputation: 109815

you have two choises

1) JButton#setMultiClickThreshhold

2) you have to split this idea to the two separated actions inside actionListener or Action

  • 1st. step, JButton#setEnabeld(false);
  • 2nd. step, then call rest of code wrapped to the javax.swing.Action (from and dealyed by javax.swing.Timer), SwingWorker or Runnable#Thread

Upvotes: 8

Ashwinee K Jha
Ashwinee K Jha

Reputation: 9307

Note that when you are modifying anything in GUI your code must run on Event Dispatch thread using invokeLater or invokeAndWait if you are in another thread. So second example is incorrect as you are trying to modify enabled state from another thread and it can cause unpredictable bugs.

Upvotes: 0

Boris Pavlović
Boris Pavlović

Reputation: 64622

The right way is using a SwingWorker. When user click the button before submmiting a job to the SwingWorker the state of the button should be changed to disabled JButton#setEnabled(false). After the SwingWorker finished the job state of the button should be reset to enabled. Here's Oracle's tutorial on SwingWorker

Upvotes: 1

StanislavL
StanislavL

Reputation: 57381

There are two more ways.

You can define a flag. Set it when action start and reset back after the end. Check the flags in the actionPerformed. If inProgress==true just do nothing.

Another way is to remove the listener and assign it back after the action ends.

Upvotes: 1

Related Questions