-
Notifications
You must be signed in to change notification settings - Fork 259
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
More Impactful Current Line Number Font. #447
base: master
Are you sure you want to change the base?
Conversation
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.
Generally I think this is a fine addition, but I have some suggestions for changes. Also, I think some lines might be > 120 characters and thus failing the linting?
} | ||
g.drawString(number, rhs-width,y); |
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.
The g.drawString()
call could still be here instead of in the if- and else blocks if we create a new x
variable and assign it a vale in the if/else blocks. I'd kind-of prefer a single g.drawString()
here.
} | ||
g.drawString(number, rhsBorderWidth, y); |
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.
Same changes here as in the LTR case
@@ -387,6 +392,8 @@ protected void paintComponent(Graphics g) { | |||
|
|||
// Paint line numbers | |||
boolean ltr = getComponentOrientation().isLeftToRight(); | |||
Font baseFont = g.getFont(); | |||
Font currentLineNumberFont = baseFont.deriveFont(Font.BOLD).deriveFont(baseFont.getSize() + currentLineNumberFontIncreaseSize); // Forcing Bold Style on the current line number |
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.
If the new field is its own font then we don't have to derive the new font on each call to paintComponent()
.
// This was required since the current line number has a higher font size so the previous method can cause the first digit of the | ||
// number to go out of bounds of the line number list. | ||
// Maybe there is some better solution to this situation. | ||
cellWidth += fontMetrics.charWidth('9')*(count+1) + (fontMetrics.charWidth('9') * (int)currentLineNumberFontIncreaseSize); |
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.
Assuming we just move to a currentLineNumberFont
variable, we can just pick the larger of the two FontMetrics
values here to simplify things a bit:
Font font = getFont();
FontMetrics fontMetrics = font.getFontMetrics();
int charWidth = fontMetrics.charWidth('9');
if (currentLineNumberFont != null) {
int charWidth2 = currentLineNumberFont.getFontMetrics().charWidth('9');
charWidth = Math.max(charWidth, charWidth2);
}
...
cellWidth += charWidth * (count + 1) + 3;
Might even move that charWidth
calculation logic into a private helper method just to make the code more readable,.
Today I installed Fedora 36 which comes with gnome's new Text Editor.
Take a look
Notice the difference between the current line number and others.
See how the current line number is popping out because of higher font size and bold style.
First I implemented this feature on my copy of the library, with more impactful look that feels more like a 3d effect.
To achieve this I also needed to alter the
updateCellWidths()
function to include more size for the popping current line number.And that's it, the end results are amazing.