Skip to content

Serial/network monitor and editor console dynamic font size hotkeys and improved updating #8704

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
Mar 26, 2019
Merged

Serial/network monitor and editor console dynamic font size hotkeys and improved updating #8704

merged 7 commits into from
Mar 26, 2019

Conversation

Pieter12345
Copy link
Contributor

  • Add CTRL +/- and CTRL scroll shortcuts to increase/decrease the serial/network monitor and editor console text size in the same way the editor text can currently be adjusted.
  • Make the serial/network monitor text size update when the editor font size is changed.
  • Make the editor console text size update when the editor font size is changed. Before this, the size was already updated, but not applied to existing text in the editor. This lead to the following bug:
    afbeelding

These changes combined cause the editor, editor console, serial monitor and network monitor to share their font in such a way that the font size can be updated using CTRL +/-/scroll in these elements and the new font size immediately applies to all these elements on change.

Fixes #8615.
Outdates #8698 (this is added to this PR).

@Pieter12345
Add editor zoom "CTRL =" shortcut
3ad08e1 
Add "CTRL =" as additional shortcut to increase the editor font size. This shortcut should be added because the '+' and '=' characters are often on the same key on the keyboard and having to press SHIFT as well is not intuitive for all users (especially since many common applications support "CTRL =").
@Pieter12345
Make text monitor output area font size dynamically adjustable
c0720ea 
Add CTRL +/- and CTRL scroll shortcuts to increase/decrease serial/network monitor output text size. This font size is shared with the editor and adjusting either will update both.
Partially fixes arduino#8615
@Pieter12345
Fix compiler/IDE warnings in monitor classes + Code cleanup
265b56b 
No functional changes:
- Fix compiler/IDE warnings in monitor classes.
- Replace listeners to use lambda classes where convenient in monitor classes.
- Code cleanup.
@Pieter12345
Properly update editor console font size on change
784b69e 
Make the editor console font properly update with editor font size changes. Before this commit, only newly added text would have an adjusted size, making it possible to have multiple text sizes in the editor console.
@Pieter12345
Add CTRL +/-/scroll shortcuts to adjust editor console font size
dd4a037 
Add CTRL +/- and CTRL scroll shortcuts to increase/decrease editor console output text size. This font size is shared with the editor and serial/network monitor. Adjusting the font size on any of these will update them all.
Partially fixes arduino#8615.
@Pieter12345
Merge branch 'ctrl-equals-editor-zoom' into serial-monitor-zoom
6dae7ce
@Pieter12345 Pieter12345 mentioned this pull request Mar 26, 2019
Closed
@facchinm
Copy link
Member

@ArduinoBot build this please

@ArduinoBot
Copy link
Contributor

✅ Build completed.

Please test this code using one of the following:

⬇️ http://downloads.arduino.cc/javaide/pull_requests/arduino-PR-8704-BUILD-848-linux32.tar.xz
⬇️ http://downloads.arduino.cc/javaide/pull_requests/arduino-PR-8704-BUILD-848-linux64.tar.xz
⬇️ http://downloads.arduino.cc/javaide/pull_requests/arduino-PR-8704-BUILD-848-windows.zip
⬇️ http://downloads.arduino.cc/javaide/pull_requests/arduino-PR-8704-BUILD-848-macosx.zip
⬇️ http://downloads.arduino.cc/javaide/pull_requests/arduino-PR-8704-BUILD-848-linuxarm.tar.xz

ℹ️ The linuxarm build is still experimental and may not be always available.

cmaglie
cmaglie reviewed Mar 26, 2019
View reviewed changes
@Pieter12345
Move editor font resize listener code to Base
2d593c4 
- Move editor font resize listener code to the Base class.
- Cache and share the same listener for all components.
@cmaglie cmaglie merged commit 61bbc38 into arduino:master Mar 26, 2019
@cmaglie cmaglie added this to the Release 1.8.10 milestone Mar 26, 2019
@cmaglie
Copy link
Member

cmaglie commented Mar 26, 2019

Now I have a failing test here:

    [javac] Compiling 41 source files to <http://jenkins.arduino.cc:8080/job/arduino-linux64/ws/app/test-bin>
    [javac] <http://jenkins.arduino.cc:8080/job/arduino-linux64/ws/app/test/processing/app/UpdateTextAreaActionTest.java>:17: error: constructor AbstractTextMonitor in class AbstractTextMonitor cannot be applied to given types;
    [javac]       super(boardPort);
    [javac]       ^
    [javac]   required: Base,BoardPort
    [javac]   found: BoardPort
    [javac]   reason: actual and formal argument lists differ in length
    [javac] 1 error

I'm wondering if passing Base in the constructor is the correct thing to do.

@Pieter12345
Copy link
Contributor Author

@cmaglie Ah I see, for some reason I didn't have app/test added as a source directory. Usually, I would use Mockito to create a mock object for the Base class and pass this in the test. However, this project does not have Mockito as a dependency. How do you think about adding Mockito as a dependency to support JUnit tests and increase testability? Another option would be to allow a null Base in the constructor and do not add these listeners in this case.

@cmaglie
Copy link
Member

cmaglie commented Mar 28, 2019

Either options looks good to me (Mockito and checking for null-ness of Base).

I was wondering about a third option: now AbstractTextMonitor depends on Base because it has to call this method base.addEditorFontResizeListeners(...), what about inverting the dependency, for example we could add a addMouseWheelListener(..) method to AbstractTextMonitor and call it from the outside (where the monitor is created) to set the wheel listener.
Also, now that I'm thinking more about it, AbstractTextMonitor is a JFrame that is in turn a Component, it should already have this method.

cmaglie added a commit to cmaglie/Arduino that referenced this pull request Mar 28, 2019
@cmaglie
Remove Base dependency from AstractTextMonitor
8d53698 
See arduino#8704 (comment)
cmaglie added a commit to cmaglie/Arduino that referenced this pull request Mar 28, 2019
@cmaglie
Remove Base dependency from AstractTextMonitor
134e31b 
See arduino#8704 (comment)
@cmaglie cmaglie mentioned this pull request Mar 28, 2019
Merged
facchinm pushed a commit that referenced this pull request Jul 18, 2019
@cmaglie @facchinm
Remove Base dependency from AstractTextMonitor
2f68d2a 
See #8704 (comment)
@per1234 per1234 added the Component: IDE user interface The Arduino IDE's user interface label Apr 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cmaglie cmaglie cmaglie left review comments

Assignees
No one assigned
Labels
Component: IDE user interface The Arduino IDE's user interface
Projects
None yet
Milestone
Release 1.8.10
Development

Successfully merging this pull request may close these issues.

Font-size change with CTRL +/-/mouse wheel in Serial monitor and Console/messages pane
5 participants
@Pieter12345 @facchinm @ArduinoBot @cmaglie @per1234