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

Terminal, Output and Syntax Coloring options UI cleanup. #6660

Merged
merged 1 commit into from Nov 10, 2023

Conversation

mbien
Copy link
Member

@mbien mbien commented Nov 5, 2023

  • made three font chooser dialogs wider so that the font columns render better
  • adjusted component gaps and layouts for consistency
  • basic code cleanup

best way to see the difference is to toggle between the terminal and output tab (misc options) in NB 20, then again with this dev build.

(sorry for the bright orange, this is just to mark my local dev build)

font-chooser-after
terminal-options-after
output-options-after

 - made font pickers wider so that the font columns render better
 - adjusted component gaps and layouts for consistency
 - basic code cleanup
@mbien mbien added Code cleanup Editor UI User Interface ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) labels Nov 5, 2023
@mbien mbien added this to the NB21 milestone Nov 5, 2023
}
try {
in.close();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a nuance, caught my attention to get a reader closed in AWT, made me worry if we do write on that stream on a different thread.

Removing the otherwise unused in variable and use previewInputOutput.getIn().close() would make this more clear.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah this looks weird but I believe getIn() does more than just opening a stream:

If a reader is ever requested, an input line is added to the tab and used to read one line at a time.

this sounds like it makes a visual difference and is the reason why it is opened that early and closed once it displayed something - but i could be wrong.

Copy link
Contributor

@lkishalmi lkishalmi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@mbien
Copy link
Member Author

mbien commented Nov 10, 2023

master is green, lets merge something :)

@mbien mbien merged commit 73237de into apache:master Nov 10, 2023
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:dev-build [ci] produce a dev-build zip artifact (7 days expiration, see link on workflow summary page) Code cleanup Editor UI User Interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants