Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve toolbar border #568

Merged
merged 1 commit into from Mar 19, 2020
Merged

Improve toolbar border #568

merged 1 commit into from Mar 19, 2020

Conversation

weisJ
Copy link
Contributor

@weisJ weisJ commented Mar 19, 2020

Description

Change the container of the toolbar from Box to JToolBar and use an empty border for the toolbar itself.

Motivation and Context

  1. Generally toolbars are placed in a component that uses BorderLayout. When doing so it gives the LaF more context on how to paint the toolbar.

  2. With the toolbar being placed in a Box container the toolbar only paints it border along the space it consumes resulting in an odd visual e.g.

    • On Metal the background is different midway through the topbar (zoom in to see it).
      metal_before

    • On Nimbus the border stops midway.
      nimbus_before

  3. JToolBar uses the exact same layoutmanager as Box resulting in the same layout as before.

  4. By giving the actual toolbar portion an EmptyBorder doubly painted borders are avoided.

How Has This Been Tested?

Layout has not been affected. See screenshots

Screenshots (if appropriate):

IntelliJ (Looks the same on the other darklaf themes):
intellij

Motif:
motif

Metal:
metal

Nimbus:
nimbus

Windows:
windows

Windows Classic:
windows_classic

Types of changes

  • Visual improvement

Checklist:

  • My code follows the code style of this project.
  • I have updated the documentation accordingly. Not needed.

Comment on lines 588 to 589
Box toolPanel = new Box(BoxLayout.X_AXIS);
JToolBar toolPanel = new JToolBar();
toolPanel.setFloatable(false);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, this indeed looks odd.

However, it looks like a single toolbar is enough, so the following should work:

        JMeterToolBar toolPanel = JMeterToolBar.createToolbar(true);
        this.toolbar = toolPanel;
        GuiPackage guiInstance = GuiPackage.getInstance();
        guiInstance.setMainToolbar(toolbar);

        toolPanel.add(Box.createRigidArea(new Dimension(5, 15)));

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I though about doing exactly this but didn’t know whether it was on purpose to have the right side of the top panel separated from the left (from a component perspective not the visuals).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no idea either, and it is not documented :-/

It looks like that was made for historical reasons only.
If we want to make the toolbar extendable, then we probably need to add explicit APIs so the plugins could contribute to the relevant sections within the toolbar.

So far I'm inclined to have a single component (which is often easier for layout) rather than a nested structure.

@pmouawad , any idea here?

@vlsi vlsi merged commit 1c734dd into apache:master Mar 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants