-
Notifications
You must be signed in to change notification settings - Fork 143
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
Scale up magic number #1319
Scale up magic number #1319
Conversation
e09fd1a
to
44f3ae5
Compare
From the look and feel perspective of this fix => i am good to go with the fix. From code perspective, i am not sure. |
Thank you for reviewing the code. Could you please clarify what the issue is with the code? |
5171181
to
44f3ae5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Kuba3105 you could also adapt the other assign of extra
in line 354 since it's also problematic. That one you can test by adapting the Snippet206
and creating a radio or check button without text, like this:
...
Button button = new Button(shell, SWT.CHECK);
button.setImage(image);
// button.setText("Button");
button.setBackground(display.getSystemColor(SWT.COLOR_RED)); //for better visibility during testing
...
I have already adapt the other assign (see the commit 5171181). I will submit a PR soon. |
You should simply add those changes to this PR (squash commits so there's always 1 commit in the PR). No need to submit another PR since it's all in the same method :-) |
be7c7fd
to
60d8a23
Compare
Use DPIUtil Contributes to vi-eclipse/Eclipse-Platform#72
60d8a23
to
1106560
Compare
The modification of the magic number allows for scaling up or down through DPIUtil. This only affects Radio or Check Buttons. There is no visual difference between using the magic number and DPIUtil.
These Screenshots are from the genral preferences.
Without the change on 100% zoom
With the change on 100% zoom
Without the change on 200% zoom
With the change on 200% zoom
Without the change on 100% zoom, move it to 200%.
With the change on 100% zoom, move it to 200%.
Contributes to vi-eclipse/Eclipse-Platform#72