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

TextField Placeholder now honors the right inset #229

Merged
merged 1 commit into from
Dec 23, 2020

Conversation

Blanco27
Copy link
Contributor

Hey,

we added a custom icon to a textfield and used additional inset to do so. Since left and top inset is already being honored, we thought it'd make sense to do the same with the right inset.

However, there's two questions:

  1. How do correctly format the source code (We used IDEA which should use .editorconfig, right?)
  2. Should we clip the placeholder or use an ellipsis (Currently we are clipping it)

@Blanco27 Blanco27 marked this pull request as draft December 22, 2020 11:09
@DevCharly
Copy link
Collaborator

Thx, must admit that I did not try long placeholder text...

1. How do correctly format the source code (We used IDEA which should use .editorconfig, right?)

Yes, FlatLaf's .editorconfig should define style used in FlatLaf.

But please do not reformat existing code.
This makes review hard and I do not merge such PRs.

When I add code to other projects, I simply look at existing code formatting and then write my code in that style.

2. Should we clip the placeholder or use an ellipsis (Currently we are clipping it)

Hmm, long text in text field is also simply clipped, but it is possible to scroll the text,
which is not possible for placeholder text...
So I think it would be better to use ellipsis (as in JLabel or JButton if they are too small).

Use method com.formdev.flatlaf.util.JavaCompatibility.getClippedString() for string clipping, which uses ellipsis.

@Bios-Marcel
Copy link
Contributor

I think it'd be good to have a CONTRIBUTION.md or a piece in the Readme that specifies the rules that we have to follow when making a PR.

So, in this case, what you are saying is, that the formatting is already correct, but just hasn't been applied in the past and therefore produces a big difference? Is there a way to avoid such a problem by specifying to only format edited lines or even disable formatting by default?

@DevCharly
Copy link
Collaborator

I think it'd be good to have a CONTRIBUTION.md or a piece in the Readme that specifies the rules that we have to follow when making a PR.

👍

So, in this case, what you are saying is, that the formatting is already correct, but just hasn't been applied in the past and therefore produces a big difference?

No, formatting of existing code is correct.

And on my machine, reformatting in IDEA with .editorconfig enabled does not reformat nearly the whole code (as in this PR).
There are only some differences. E.g. different line wrapping. But spaces are correct.

I do not use the formatter.
I write the code is so that it is easy to read.

Here is a comparison (from FlatTextFieldUI) of hand formatted code and IDEA formatted:

image

Is there a way to avoid such a problem by specifying to only format edited lines or even disable formatting by default?

Don't know why IDEA reformatted the whole file on @MioBlanco computer.
On mine it does not reformat it.
It there some option to "reformat on save" in IDEA?

If the placeholder can't be drawn fully, we clip it by adding an
ellipse.
@Blanco27 Blanco27 marked this pull request as ready for review December 23, 2020 08:30
DevCharly added a commit that referenced this pull request Dec 23, 2020
@DevCharly DevCharly merged commit d5002b1 into JFormDesigner:master Dec 23, 2020
@DevCharly DevCharly added this to the 0.47 milestone Dec 23, 2020
@Blanco27 Blanco27 deleted the placeholder_insets branch December 23, 2020 12:11
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.

3 participants