Painting a rectangle on a button click using JPanel

Question

I am trying to paint a rectangle using JPanel. The problem is, when I click on a menu item it should paint a new rectangle. BUT, when I do this, only part of it is painted. Here is what I mean:

    rectangleMenuItem.addActionListener(new ActionListener() {
        @Override
        public void actionPerformed(ActionEvent event) {
            squares.addSquare(10, 10, 100, 100);
        }
    });

does this:

but when I put the squares.addSquare(...) OUTSIDE of the ActionListener, it gives the correct shape (just not when I want it to)

    squares.addSquare(10, 10, 100, 100);
    rectangleMenuItem.addActionListener(new ActionListener() {
        @Override
        public void actionPerformed(ActionEvent event) {
            //nothing on click...
        }
    });

and here is the correct rectangle:

any idea why it doesn't draw correctly when I put it in the ActionListener? Thanks.

Full Code

class UMLWindow extends JFrame {
Squares squares = new Squares();

private static final long serialVersionUID = 1L;

public UMLWindow() {
    addMenus();
}

public void addMenus() {

    getContentPane().add(squares);
    //squares.addSquare(10, 10, 100, 100);

    JMenuBar menubar = new JMenuBar();

    JMenu file = new JMenu("File");
    file.setMnemonic(KeyEvent.VK_F);

    JMenu shapes = new JMenu("Shapes");
    file.setMnemonic(KeyEvent.VK_F);

    JMenuItem rectangleMenuItem = new JMenuItem("New Rectangle");
    rectangleMenuItem.addActionListener(new ActionListener() {
        @Override
        public void actionPerformed(ActionEvent event) {
            squares.addSquare(10, 10, 100, 100);
        }
    });

    shapes.add(rectangleMenuItem);

    menubar.add(shapes);

    setJMenuBar(menubar);

    setTitle("UML Editior");
    setSize(300, 200);
    setLocationRelativeTo(null);
}


}

class Squares extends JPanel {
private static final long serialVersionUID = 1L;

private List<Rectangle> squares = new ArrayList<Rectangle>();

public void addSquare(int x, int y, int width, int height) {
    Rectangle rect = new Rectangle(x, y, width, height);
    squares.add(rect);
}

@Override
protected void paintComponent(Graphics g) {
    super.paintComponent(g);
    this.setOpaque(true);
    this.setBackground(Color.WHITE);
    Graphics2D g2 = (Graphics2D) g;
    for (Rectangle rect : squares) {
        g2.draw(rect);
    }
}

}

Answer 1

Swing uses a passive rendering engine, so it doesn't know when it should update/repaint a component unless you tell it.

Start by calling repaint from within your addSquare method...

public void addSquare(int x, int y, int width, int height) {
    Rectangle rect = new Rectangle(x, y, width, height);
    squares.add(rect);
    repaint();
}

This will make a request to the repaint manager informing it the current component needs to be repainted.

You should also be providing a sizing hint for your component, so layout managers to set it's size to 0x0.

Consider overriding the getPreferredSize method of the Squares class and return an appropriate default size...

@Override
public Dimension getPreferredSize() {
    return new Dimension(300, 200);
}

This will also mean that you can call pack on the main window and it will "pack" itself around the content, very neat.

Also, don't call methods which will either change the state of the component or other components or schedule an additional repaint request, as this could setup an infinite loop of repaints which will drain your PC of resources....

@Override
protected void paintComponent(Graphics g) {
    super.paintComponent(g);
    // This is bad...
    //this.setOpaque(true);
    // This is bad...
    //this.setBackground(Color.WHITE);
    Graphics2D g2 = (Graphics2D) g;
    for (Rectangle rect : squares) {
        g2.draw(rect);
    }
}

Things like opacity and color should be defined outside of the paint cycle. Also, components that extend from JPanel are opaque by default ;)