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

Remove etched borders. #569

Merged
merged 1 commit into from
Mar 22, 2020
Merged

Remove etched borders. #569

merged 1 commit into from
Mar 22, 2020

Conversation

weisJ
Copy link
Contributor

@weisJ weisJ commented Mar 19, 2020

Description

Remove the usage of BorderFactory#createEtchedBorder.

Motivation and Context

Currently EtchedBorder is used for almost (but weirdly not all) titled borders.
They are undesirable for multiple reasons:

  1. The Laf can't provide a custom etched border. Instead:
  2. Etched borders simply use a lighter and darker version of the background colour, which most of the time looks out of place (especially with regards to "newer" design standards).

When using BorderFactory#createTitledBorder without specifying the border explicitly the a LaF-provided border is used. Because it can provide a border which looks good together with the rest of the LaF the result looks a lot more consistent.

At the places where simply BorderFactory#createEtchedBorder is used it is replaced by BorderFactory.createLineBorder("") which simply uses an unmodified version of the border provided by the laf.

How Has This Been Tested?

See screenshots. Only visuals are updated.

Screenshots (if appropriate):

All other lafs not shown look the same as before, because they use an EtchedBorder by default.

Before:

Metal

metal_before

Nimbus

nimbus_before

IntelliJ

intellij_before

After:

Metal

metal

Nimbus

nimbus

IntelliJ

intellij

Types of changes

  • UI improvements.

Checklist:

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

@vlsi
Copy link
Collaborator

vlsi commented Mar 19, 2020

@pmouawad , @FSchumacher , any idea here?

Frankly speaking, I'm inclined to merge this.

As you probably have seen, I've recently removed some of the borders, and I appreciate @weisJ taking time to unify the UI.

I don't quite like how Nimbus LaF defaults to LoweredBorder by default, however, that is their choice. I don't think JMeter should compensate that by adding explicit etched everywhere.

If one desires to use Nimbus, but keep borders etched rather than lowered, then we could probably add a property that would call uiDefaults.put("TitledBorder.border", ...); to augment the LaF in question.

@vlsi
Copy link
Collaborator

vlsi commented Mar 19, 2020

I did click through different components, and it looks good to me.

@vlsi vlsi merged commit c28f220 into apache:master Mar 22, 2020
This pull request was closed.
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.

2 participants