8

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
3
  • 1
    whatever you do, never sleep on the EDT
    – kleopatra
    Commented Dec 8, 2011 at 9:45
  • 1
    It seems that while the "long task" is executing the JButton still pile up events and as soon as the "long task" is finished got executed.
    – Alex
    Commented Dec 8, 2011 at 10:57
  • 1
    all kind of nasty stuff can happen when you block the EDT, there is nothing much to understand because nasty implies unpredictable as well - simply don't and be a happy coder :-)
    – kleopatra
    Commented Dec 8, 2011 at 11:41

6 Answers 6

8

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
2
  • 1
    MultiClickThreshold is not ok as the "long task" can be longer than the time specified for threshold so a new even can be triggered while the "long task" is in progress
    – Alex
    Commented Dec 8, 2011 at 10:47
  • 3
    @Alex, true.. but, the goal might be just to inhibit end-users who habitually double-click everything because they aren't aware that buttons only need to be pressed once. That is, in my eyes, the main goal of "setMutliClickThreshold."
    – ryvantage
    Commented Aug 1, 2013 at 16:22
2

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);
1
  • I'd really like to hear your feedback on my answer I just left.
    – ryvantage
    Commented May 16, 2014 at 22:03
1

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.

2
  • flags? removing listeners? Astonished ;-) As we both know, that's brittle. A solid state-model of Action's enablement (as the OP basically wanted but didn't quite reach) is the way to go.
    – kleopatra
    Commented Dec 8, 2011 at 9:50
  • Agree. Action based way even better but would require more changes in existing code.
    – StanislavL
    Commented Dec 8, 2011 at 10:46
1

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

1
  • 1
    correct ... except that you wouldn't touch the button directly, but bind the enablement of an associated Action
    – kleopatra
    Commented Dec 8, 2011 at 9:51
1

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.

1
  • Upon my own implementation, I can see that that there's a (small) defect: this solution assumes that SwingWorker::isDone() is always a good way to determine if it's ok to run again, which is not always the case (in my situation).
    – ryvantage
    Commented May 16, 2014 at 22:52
0

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.

5
  • But task.setEnabled(true); is executed in Event Dispatch Thread
    – Alex
    Commented Dec 8, 2011 at 10:45
  • Yes, I was just confused with new Thread(). Then the issue is same as you have guessed, the piled up events are executed after button is enabled in second case while they are executed before button is enabled in first case. Commented Dec 8, 2011 at 11:04
  • Ok, but why piles up in second case and not in first - where is the glitch ?
    – Alex
    Commented Dec 8, 2011 at 11:24
  • Lets say you click button two times before sleep() is executed. Now one click is yet to be processed. In first case because you again enable button in invokeLater, the click will be processed (while button is disabled) and will have no effect. In second case the button is enabled and then pending event is processed. Commented Dec 8, 2011 at 11:30
  • So pending event will be processed even if the button is disabled...?!?!
    – Alex
    Commented Dec 8, 2011 at 14:29

Not the answer you're looking for? Browse other questions tagged or ask your own question.