JAVA - JFrame not looking proper when called from action listener

Question

I have a problem with one of my frames not looking as it should, when it is called upon the press of a button.

The frame looks as if it was rendered improperly, the label text in it is shortened, however when i move the same line of code outside the action listener, it works as it should.

I have a sort of main menu, with two buttons, only the Generate Menu works at the moment, it looks like this:

https://i.stack.imgur.com/pJBhA.png

The code for the action listener:

    runMenuButt.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent e) {
            System.out.println("Generate Menu pressed");
            mF.dispose();
            MenuGenerator.generateTheMenu();
        }
    });

The result looks wrong: https://i.stack.imgur.com/Pqk0L.png The frame is also unresponsive, clikcing X does not, while it should close the frame and the application.

However changing the code to:

    runMenuButt.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent e) {
            System.out.println("Generate Menu pressed");
            //mF.dispose();

        }
    });
    MenuGenerator.generateTheMenu();

Produces correct look: https://i.stack.imgur.com/vtkwn.png

The code for the "Main menu"

public static void openMainMenu() {
    Font menuFont = new Font("Courier",Font.BOLD,16);
    Dimension dim = Toolkit.getDefaultToolkit().getScreenSize();
    mF.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
    mF.setSize(465,230);
    mF.setLocation(dim.width/2-mF.getSize().width/2, dim.height/2-mF.getSize().height/2);
    mF.getContentPane().setBackground(Color.WHITE);

    Color blueSteel = new Color(70,107,176);
    JPanel p = new JPanel();
    p.setSize(600,50);
    p.setLayout(new GridBagLayout());
    GridBagConstraints gbc = new GridBagConstraints();
    p.setLocation((mF.getWidth() - p.getWidth()) /2, 20);
    p.setBackground(blueSteel);
    JLabel l = new JLabel("Welcome to the menu GENERATORRRR");
    l.setFont(menuFont);
    l.setForeground(Color.WHITE);
    p.add(l, gbc);

    JButton runMenuButt = new JButton("Generate Menu");
    runMenuButt.setLocation(20 , 90);
    JButton manageRecipButt = new JButton("Manage Recipients");
    manageRecipButt.setLocation(240 , 90);
    menuUtilities.formatButton(runMenuButt);
    menuUtilities.formatButton(manageRecipButt);

    mF.setResizable(false);
    mF.setLayout(null);
    mF.add(runMenuButt);
    mF.add(manageRecipButt);
    mF.add(p);
    mF.setVisible(true);

    runMenuButt.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent e) {
            System.out.println("Generate Menu pressed");
            //mF.dispose();

        }
    });
    MenuGenerator.generateTheMenu();

    manageRecipButt.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent e) {
            JOptionPane.showMessageDialog(null, "Not supported yet", "Function not yet available",JOptionPane.INFORMATION_MESSAGE);
        }
    });
    //System.out.println(mF.getContentPane().getSize());
}

And the status bar:

public class StatusBar {
private static JLabel statusLabel= new JLabel("Starting");
private static JFrame statusFrame = new JFrame("Generation Status");

public static void createStatusBar() {
    Font menuFont = new Font(Font.MONOSPACED,Font.BOLD,20);
    Dimension dim = Toolkit.getDefaultToolkit().getScreenSize();

    statusFrame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
    statusFrame.setSize(700,100);

    JPanel p = new JPanel();
    p.setPreferredSize(new Dimension(100,100));
    statusLabel.setFont(menuFont);
    p.add(statusLabel);

    statusFrame.add(p,BorderLayout.CENTER);
    statusFrame.setLocation(dim.width/2-statusFrame.getSize().width/2, dim.height/2-statusFrame.getSize().height/2);
    statusFrame.setVisible(true);
}

public static void setStatusBar(String statusText) {
    statusLabel.setText(statusText);
    statusLabel.paintImmediately(statusLabel.getVisibleRect());
    statusLabel.revalidate();
}

public static void closeStatusBar(){
    statusFrame.dispose();
}
}

I create the bar with this line: StatusBar.createStatusBar();

Why does the status bar not render properly when the MenuGenerator.generateTheMenu(); is called from the action listener?

Here is minimal code that reproduces this behavior for anyone who would like to test it: It also uses class for the StatusBar, which is already posted.

public class MinimalClass {
private static JFrame mF = new JFrame("Main Menu");

public static void main(String[] args) {
    openMainMenu();
}

public static void openMainMenu() {
    Dimension dim = Toolkit.getDefaultToolkit().getScreenSize();
    mF.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
    mF.setSize(465,230);
    mF.setLocation(dim.width/2-mF.getSize().width/2, dim.height/2-mF.getSize().height/2);
    mF.getContentPane().setBackground(Color.WHITE);

    JButton runMenuButt = new JButton("Generate Menu");
    runMenuButt.setLocation(20 , 90);
    runMenuButt.setSize(200 , 85);

    mF.setResizable(false);
    mF.setLayout(null);
    mF.add(runMenuButt);
    mF.setVisible(true);

    runMenuButt.addActionListener(new ActionListener() {
        public void actionPerformed(ActionEvent e) {
            System.out.println("Generate Menu pressed");
            mF.dispose();
            generateTheMenu();
        }
    });
}

public static void generateTheMenu() {
    System.setProperty("sun.java2d.cmm", "sun.java2d.cmm.kcms.KcmsServiceProvider");
    String rawMenuOutput = "";

    try {
        rawMenuOutput= getMenuInJavaNow();
    } catch (Exception e){
        System.out.println("Something went terribly wrong");
    }
    System.out.println(rawMenuOutput);
}

public static String getMenuInJavaNow() throws IOException {

    String rawMenuOutput = "Restaurant Menu" ;
    rawMenuOutput = rawMenuOutput + "Test line";
    String []menuOtpArr = new String [3];

    try {
        StatusBar.createStatusBar();
        TimeUnit.SECONDS.sleep(2);
        StatusBar.setStatusBar("Test1");
        TimeUnit.SECONDS.sleep(2);
        menuOtpArr[0]="Test line";
        StatusBar.setStatusBar("Test2");
        TimeUnit.SECONDS.sleep(2);
        menuOtpArr[1]="Test line";
        StatusBar.setStatusBar("Test3");
        TimeUnit.SECONDS.sleep(2);
        menuOtpArr[2]="Test line";
        StatusBar.setStatusBar("Test4");
        TimeUnit.SECONDS.sleep(2);
        StatusBar.closeStatusBar();
    } catch (Exception e) {
    }

    for (int i=0;i < menuOtpArr.length;i++) {
        rawMenuOutput = rawMenuOutput + "\n\n" +menuOtpArr[i];
    }
    return rawMenuOutput;
}
}

Thank you for your time

Answer 1

statusLabel.paintImmediately(statusLabel.getVisibleRect()); seems to masking a larger issue.

The problem is, Swing is single threaded (and NOT thread safe). This means that when you call TimeUnit.SECONDS.sleep(2); from within getMenuInJavaNow, which is called by generateTheMenu, which is called by the ActionListener, it's been called within the context of the Event Dispatching Thread.

This is putting the EDT to sleep, meaning that it isn't processing layout or paint requests (properly)

Start by having a read of Concurrency in Swing for more details

Now, you have a larger issue, how to solve it. For the answer to that question, we require a lot more context then is currently available.

The getMenuInJavaNow seems to be returning some values, to what end I'm not sure.

"A" solution, would be to use a SwingWorker (see Worker Threads and SwingWorker for more details). It provides the ability to execute long running tasks in the background, but also provides the means for sync updates back to the UI, for example...

import java.awt.BorderLayout;
import java.awt.Color;
import java.awt.Dimension;
import java.awt.EventQueue;
import java.awt.Font;
import java.awt.GridLayout;
import java.awt.Insets;
import java.awt.Toolkit;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.beans.PropertyChangeEvent;
import java.beans.PropertyChangeListener;
import java.util.List;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import javax.swing.JButton;
import javax.swing.JFrame;
import javax.swing.JLabel;
import javax.swing.JPanel;
import javax.swing.SwingWorker;

public class MinimalClass {

    private static JFrame mF = new JFrame("Main Menu");

    public static void main(String[] args) {
        EventQueue.invokeLater(new Runnable() {
            @Override
            public void run() {
                openMainMenu();
            }
        });
    }

    public static void openMainMenu() {
        Dimension dim = Toolkit.getDefaultToolkit().getScreenSize();
        mF.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
        mF.setLocationRelativeTo(null);
        mF.getContentPane().setBackground(Color.WHITE);

        JButton runMenuButt = new JButton("Generate Menu");
        runMenuButt.setMargin(new Insets(25, 25, 25, 25));

        JPanel buttons = new JPanel(new GridLayout(0, 2));
        buttons.add(runMenuButt);

        mF.add(buttons);
        mF.pack();
        mF.setVisible(true);

        runMenuButt.addActionListener(new ActionListener() {
            public void actionPerformed(ActionEvent e) {
                System.out.println("Generate Menu pressed");
                mF.dispose();
                generateTheMenu();
            }
        });
    }

    public static void generateTheMenu() {
        System.setProperty("sun.java2d.cmm", "sun.java2d.cmm.kcms.KcmsServiceProvider");

        StatusBar.createStatusBar();

        SwingWorker<String, String> worker = new SwingWorker<String, String>() {
            @Override
            protected String doInBackground() throws Exception {
                String rawMenuOutput = "Restaurant Menu";
                rawMenuOutput = rawMenuOutput + "Test line";
                String[] menuOtpArr = new String[3];

                try {
                    TimeUnit.SECONDS.sleep(2);
                    publish("1234567890123456789012345678901234567890");
                    TimeUnit.SECONDS.sleep(2);
                    publish("This is a test");
                    TimeUnit.SECONDS.sleep(2);
                    publish("More testing");
                    TimeUnit.SECONDS.sleep(2);
                    publish("Still testing");
                    TimeUnit.SECONDS.sleep(2);
                } catch (Exception e) {
                }

                for (int i = 0; i < menuOtpArr.length; i++) {
                    rawMenuOutput = rawMenuOutput + "\n\n" + menuOtpArr[i];
                }
                return rawMenuOutput;
            }

            @Override
            protected void done() {
                StatusBar.closeStatusBar();
            }

            @Override
            protected void process(List<String> chunks) {
                StatusBar.setStatusBar(chunks.get(chunks.size() - 1));
            }

        };

        worker.addPropertyChangeListener(new PropertyChangeListener() {
            @Override
            public void propertyChange(PropertyChangeEvent evt) {
                if (worker.getState() == SwingWorker.StateValue.DONE) {
                    try {
                        String result = worker.get();
                        System.out.println(result);
                        StatusBar.closeStatusBar();
                    } catch (InterruptedException | ExecutionException ex) {
                        ex.printStackTrace();
                    }
                }
            }
        });
        worker.execute();
    }

    public static class StatusBar {

        private static JLabel statusLabel = new JLabel("Starting");
        private static JFrame statusFrame = new JFrame("Generation Status");

        public static void createStatusBar() {
            Font menuFont = new Font(Font.MONOSPACED, Font.BOLD, 20);
            Dimension dim = Toolkit.getDefaultToolkit().getScreenSize();

            statusFrame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
            statusFrame.setSize(700, 100);

            JPanel p = new JPanel();
            p.setBackground(Color.RED);
            //          p.setPreferredSize(new Dimension(100, 100));
            statusLabel.setFont(menuFont);
            p.add(statusLabel);

            statusFrame.add(p, BorderLayout.CENTER);
            statusFrame.setLocationRelativeTo(null);
            statusFrame.setVisible(true);
        }

        public static void setStatusBar(String statusText) {
            statusLabel.setText(statusText);
        }

        public static void closeStatusBar() {
            statusFrame.dispose();
        }
    }
}

Observations...

• static is not your friend, especially in cases like this. You really, really, really need to learn to live without it.
• setLayout(null) is not doing you any favours, especially in the long run. Take the time to go through Laying Out Components Within a Container and start making proper use of layout managers, they might seem "complicated", but they will save you from a lot of hair loss
• Avoid using setPreferred/Minimum/MaximumSize where ever possible, you are robing the component of the ability to provide useful rendering hints which may change across platforms and rendering pipelines

Just a quick follow up question, what is the difference between done and addPropertyListener ? Is there any? Isnt it redundant to use both?

The example here is pretty basic, for me I've used done to handle what the SwingWorker "knows" needs to be done, it doesn't however, know what is to be done with the result.

I've used the PropertyChangeListener to deal with that instead - the point - it's an example.

And I also noticed, that I dont need to actually publish, as calling StatusBar.setStatusBar(""); works as well. Is it necessary to use publish?

In a word YES. Swing is NOT thread safe, calling StatusBar.setStatusBar("") directly can lead to some weird and unexpected results. publish pushes the call into the Event Dispatching Thread, making it safe to update the UI from within.

I have the code for generating the String I want to set as the StatusBar Title in another class, not in the generateTheMenu, therefore it is more convenient for me to simply call .setStatusBar. The not minimal code I have is actually something like this

This is where things like interfaces come in really handy. You "string" generating class "could" either return the resulting text OR you could pass a reference to a interface implementation which is used to "display" it. This way, your SwingWorker could act as a consumer for the String and pass it through the publish method.

There are a number of really important concepts to understand.

• You want to decouple your code. This makes it easier to change certain parts of the code without affecting the other parts
• You want to be able to "code to interface, not implementation". This goes hand in hand with the first comment. Basically, you want to "hide" the implementation details as much as possible - lots of different reasons for it, but it helps keep your code lean, helps make the follow more understandable and stops one part of the code from accessing another it really has no responsibility to do so (is the string generation really responsible for updating the status bar? IMHO - not really)
• There is also a swagger of design patterns available to make solving issues easier. I've already mentioned the concept of "produce/consumer", but this is just one

The "Event Dispatching Thread"

• The Event Dispatch Thread
• Java Event-Dispatching Thread explanation
• Swing threading and the event-dispatch thread