user1521881
user1521881

Reputation: 67

unwanted space between jpanels

I am trying to create a JPanel which has 3 components arranged in a line. It should have a coloured box, a label and then a delete button.

I have got a JPanel set as a GridLayout which stores a JPanel for the coloured box, a JLabel for the label and a JButton with a custom ImageIcon.

The problem is that there is a blank space between the coloured box and the label. I have highlighted the borders of each component and no component seems to be overstretched.

Here is a screenshot of what I mean:

enter image description here

Here is the code I am working with: Label class:

public class Label extends JPanel {

    JButton btnDeleteObject;
    // Delete icon
    ImageIcon delIcon = new ImageIcon("Delete.png"); 
    Image img = delIcon.getImage();
    Image newimg = img.getScaledInstance(28, 28,  java.awt.Image.SCALE_SMOOTH);  
    ImageIcon scaledDelIcon = new ImageIcon(newimg);

    Color labelColour;

    public Label(String labelName, Color labelColour) {
        this.labelColour = labelColour;

        setLayout(new BoxLayout(this, BoxLayout.X_AXIS));

        addComponents(labelName);

    }   

    private void addComponents(String labelName) {

        JPanel innerContainer = new JPanel(new GridLayout(1, 3));
        JLabel name = new JLabel(labelName);

        LabelColourBox cBox = new LabelColourBox();

        name.setMaximumSize(new Dimension(80, 40));
        name.setPreferredSize(new Dimension(80, 40));
        name.setSize(new Dimension(80, 40));

        name.setBorder(BorderFactory.createLineBorder(Color.blue));

        setBorder(BorderFactory.createLineBorder(Color.black));

//      name.setBorder(new EmptyBorder(5, 5, 5, 5));

        // Add action to delete button for Icon
        Action action = new AbstractAction("Button Label", scaledDelIcon) {
            // This method is called when the button is pressed
            public void actionPerformed(ActionEvent evt) {
                System.out.println("delete");
            }
        };

        btnDeleteObject = new JButton(action);

        // Remove label, background
        btnDeleteObject.setText("");
        btnDeleteObject.setContentAreaFilled(false);

        setAlignmentX(LEFT_ALIGNMENT);

        innerContainer.add(cBox);
        innerContainer.add(name);
        innerContainer.add(btnDeleteObject);

        add(innerContainer);

    }
}

and here is the LabelColourBox:

public class LabelColourBox extends JPanel{

    public LabelColourBox() {
        setLayout(new BoxLayout(this, BoxLayout.X_AXIS));
    }


    @Override
    protected void paintComponent(Graphics g) {

        super.paintComponent(g);
        setBorder(BorderFactory.createLineBorder(Color.green));

        setMaximumSize(new Dimension(40, 40));
        setPreferredSize(new Dimension(40, 40));
        setSize(new Dimension(40, 40));

        g.setColor(Color.RED);
        g.fillRect(0, 0, 40, 40);
    }


}

Upvotes: 1

Views: 1833

Answers (2)

MadProgrammer
MadProgrammer

Reputation: 347314

As Guillaume has pointed, there are some serious issues with your code.

NEVER make any changes to ANY UI component from within any paintXxx method

protected void paintComponent(Graphics g) {
    super.paintComponent(g);

    // This is INCREDIBLY bad
    setBorder(BorderFactory.createLineBorder(Color.green));
    setMaximumSize(new Dimension(40, 40));
    setPreferredSize(new Dimension(40, 40));
    setSize(new Dimension(40, 40));
    //------------

    g.setColor(Color.RED);
    g.fillRect(0, 0, 40, 40);
}

Doing so will cause a repaint request to be sent to the repaint manager, which will eventually call your paintComponent which will trigger a repaint request ... and say good bye to your CPU as it cycles up to 100% and your program becomes unresponsive.

You should avoid calling setPreferred/Minimum/MaximumSize as much as possible. Let the individual components work it out. If you're customizing a component, override these methods, it will stop some way-ward developer from changing the size of the component under you.

You should avoid absolute values, especially when painting.

g.fillRect(0, 0, 40, 40);

This is very dangerous. What if this component gets added to a BorderLayout? It will only ever paint a square in the top left corner of your component...not really useful, instead use getWidth and getHeight as a bases for the area you need to effect. You also need to take into consideration the components Insets to make sure you content is being painted within the usable visual bounds.

This also means you can change the size of the component without needing to rewrite whole sections of code :P

As to your program. Use a more flexible layout manager. GridLayout creates a uniform grid in which to layout out its components...

enter image description here

public class Main {

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

    public Main() {
        EventQueue.invokeLater(new Runnable() {
            @Override
            public void run() {
                try {
                    UIManager.setLookAndFeel(UIManager.getSystemLookAndFeelClassName());
                } catch (ClassNotFoundException ex) {
                } catch (InstantiationException ex) {
                } catch (IllegalAccessException ex) {
                } catch (UnsupportedLookAndFeelException ex) {
                }

                JFrame frame = new JFrame();
                frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
                frame.setLayout(new GridLayout(3, 1));
                frame.add(new Label("One", Color.BLACK));
                frame.add(new Label("Two", Color.BLACK));
                frame.add(new Label("Three", Color.BLACK));
                frame.pack();
                frame.setVisible(true);
            }
        });
    }

    public class Label extends JPanel {

        JButton btnDeleteObject;
        // Delete icon
        ImageIcon delIcon = new ImageIcon("Delete.png");
        Image img = delIcon.getImage();
        Image newimg = img.getScaledInstance(28, 28, java.awt.Image.SCALE_SMOOTH);
        ImageIcon scaledDelIcon = new ImageIcon(newimg);
        Color labelColour;

        public Label(String labelName, Color labelColour) {
            this.labelColour = labelColour;

//            setLayout(new BoxLayout(this, BoxLayout.X_AXIS));
            // Not sure why you want to do this, but this would
            // be a more useful layout manager in the this context
            // Personally, I would just layout the components directly onto
            // this container...
            setLayout(new BorderLayout());

            addComponents(labelName);

        }

        private void addComponents(String labelName) {

//            JPanel innerContainer = new JPanel(new GridLayout(1, 3));
            JPanel innerContainer = new JPanel(new GridBagLayout());
            JLabel name = new JLabel(labelName);

            LabelColourBox cBox = new LabelColourBox();

//            name.setMaximumSize(new Dimension(80, 40));
//            name.setPreferredSize(new Dimension(80, 40));
//            name.setSize(new Dimension(80, 40));

            name.setBorder(BorderFactory.createLineBorder(Color.blue));

            setBorder(BorderFactory.createLineBorder(Color.black));

//      name.setBorder(new EmptyBorder(5, 5, 5, 5));

            // Add action to delete button for Icon
            Action action = new AbstractAction("Button Label", scaledDelIcon) {
                // This method is called when the button is pressed
                public void actionPerformed(ActionEvent evt) {
                    System.out.println("delete");
                }
            };

            btnDeleteObject = new JButton(action);

            // Remove label, background
            btnDeleteObject.setText("");
            btnDeleteObject.setContentAreaFilled(false);

            setAlignmentX(LEFT_ALIGNMENT);

            GridBagConstraints gbc = new GridBagConstraints();
            gbc.gridx = 0;
            gbc.gridy = 0;
            gbc.fill = GridBagConstraints.BOTH;

            innerContainer.add(cBox, gbc);
            gbc.gridx++;
            gbc.weightx = 1;
            gbc.anchor = GridBagConstraints.WEST;
            innerContainer.add(name, gbc);
            gbc.gridx++;
            gbc.weightx = 0;
            innerContainer.add(btnDeleteObject, gbc);

            add(innerContainer);

        }

        public class LabelColourBox extends JPanel {

            public LabelColourBox() {
                setLayout(new BoxLayout(this, BoxLayout.X_AXIS));
            }

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

            @Override
            public Dimension getMinimumSize() {
                return getPreferredSize();
            }

            @Override
            public Dimension getMaximumSize() {
                return getPreferredSize();
            }

            @Override
            protected void paintComponent(Graphics g) {

                super.paintComponent(g);

                // DON'T DO THIS, THIS IS A VERY BAD IDEA, THIS WILL RESULT
                // IN A NEW REPAINT REQUEST BEING SENT TO THE REPAINT MANAGER
                // CAUSE THIS METHOD TO BE CALLED AND NEW REPAINT REQUEST BEING...
                // you get the idea..
                //setBorder(BorderFactory.createLineBorder(Color.green));
                //setMaximumSize(new Dimension(40, 40));
                //setPreferredSize(new Dimension(40, 40));
                //setSize(new Dimension(40, 40));

                g.setColor(Color.RED);
                g.fillRect(0, 0, getWidth() - 1, getHeight() - 1);
                g.setColor(Color.GREEN);
                g.drawRect(0, 0, getWidth() - 1, getHeight() - 1);
            }
        }
    }
}

Upvotes: 2

Guillaume Polet
Guillaume Polet

Reputation: 47617

  1. Don't call either setSize/setPreferredSize/setMaximumSize/setMinimumSize/setBounds/setLocation, use an appropriate LayoutManager, that is there job.
  2. Don't call these methods in paintComponent. paintComponent() = paint the component, therefore you should perform only painting operations (drawLine, drawRect, ...).
  3. GridLayout is usually not a very flexible layout as it lays out all the child component with the same size, based on the biggest preferred size. So take a look at BorderLayout, GridBagLayout, or even BoxLayout and FlowLayout in such simple case. If you can use third-party libs, MigLayout is your friend.

Upvotes: 4

Related Questions