-
Notifications
You must be signed in to change notification settings - Fork 228
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
next: Support display the comment of the SGF file #365
Conversation
@OlivierBlanvillain could you review this one? Thanks! Looks so useful :) |
* | ||
* @return true when process comment scroll | ||
*/ | ||
public boolean onMouseWheelMoved(MouseWheelEvent e) { |
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.
I would rename it to something more descriptive like processCommentMouseWheelMoved
.
/** | ||
* Process Comment Mouse Wheel Moved | ||
* | ||
* @return true when process comment scroll |
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.
@return true when the scroll event was processed by this method
* @param full | ||
* @return | ||
*/ | ||
private int drawCommnet(Graphics2D g, int x, int y, int w, int h, boolean full) { |
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.
Typo: s/drawCommnet/drawComment/
* | ||
* @return the next node, null if there is no next node | ||
*/ | ||
public BoardHistoryNode nextNode() { |
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.
Unused?
Could you rebase this PR on top of next? When I reached these lines https://github.com/featurecat/lizzie/pull/364/files#diff-d7874015cfce6652bd9e035733aaaef1R89 I realized I already reviewed parts of this code... (To be clear: I'm not done with the review, I will continue/restart once the PR is rebased) |
@OlivierBlanvillain |
int fontSize = (int)(Math.min(getWidth(), getHeight()) * 0.98 * 0.03); | ||
try { | ||
fontSize = Lizzie.config.uiConfig.getInt("comment-font-size"); | ||
} catch (JSONException e) { |
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.
This seems out of place to me. I feel that in this part of the code we should not have to think about JSON or default value, instead simply access something like Lizzie.config.uiConfig.commentFontSize
, the rest should be handled as it is for other config options.
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.
Changed to config.
commentPane.setFont(font); | ||
commentPane.setText(comment); | ||
commentPane.setSize(w, cHeight); | ||
createCommentImage((comment != null && !comment.equals(this.cachedComment)) ? true : false, w, cHeight); |
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.
expr ? true : false
should be the same as expr
...
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.
Wow, what is the code! Removed.
@@ -79,7 +79,7 @@ public void drawTree(Graphics2D g, int posx, int posy, int startLane, int maxpos | |||
g.setColor(curcolor); | |||
|
|||
// Draw main line | |||
while (cur.next() != null && posy + YSPACING < maxposy) { | |||
while (cur.next() != null && ((posy + YSPACING + DOT_DIAM) < maxposy)) { // Fix oval cover issue sometimes |
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.
Can you explain a bit more what this is fixing?
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.
Sometimes the variation node covers the comment panel.
But this bug is previous version. I'll test current version for this bug.
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.
It seems no bug now. Restored.
Looks good, thanks! |
Closes: #350
This pull request based on #364
Support for displaying comments of the sgf files.
Default is close, can press key 'T' to toggle, and add a item 'show-comment' in the config file.
Allow use a hidden item 'comment-font-size' in the config to forced specify the comment font size, for example:
"comment-font-size": 20