Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add ability to zoom via scroll wheel #263

Merged
merged 8 commits into from
Mar 1, 2023

Conversation

mfoulks3200
Copy link
Contributor

Added a convenience feature to adjust the view zoom by holding shift and scrolling.

@awesomefly
Copy link

awesomefly commented Nov 2, 2022 via email

@codecov-commenter
Copy link

codecov-commenter commented Nov 2, 2022

Codecov Report

Merging #263 (eca4192) into develop (d74690e) will increase coverage by 0.08%.
The diff coverage is 9.09%.

@@              Coverage Diff              @@
##             develop     #263      +/-   ##
=============================================
+ Coverage      70.44%   70.53%   +0.08%     
- Complexity      1536     1553      +17     
=============================================
  Files            146      146              
  Lines           8680     8780     +100     
  Branches        1405     1423      +18     
=============================================
+ Hits            6115     6193      +78     
- Misses          1973     1994      +21     
- Partials         592      593       +1     
Impacted Files Coverage Δ
...a/com/tagtraum/perf/gcviewer/view/AboutDialog.java 84.28% <ø> (ø)
...va/com/tagtraum/perf/gcviewer/view/GCDocument.java 33.97% <ø> (ø)
...a/com/tagtraum/perf/gcviewer/view/GCViewerGui.java 80.00% <ø> (ø)
...om/tagtraum/perf/gcviewer/view/ModelChartImpl.java 77.74% <9.09%> (-1.99%) ⬇️
...gtraum/perf/gcviewer/ctrl/impl/GcSeriesLoader.java 74.15% <0.00%> (-2.25%) ⬇️
...tagtraum/perf/gcviewer/imp/DataReaderSun1_6_0.java 88.53% <0.00%> (-1.53%) ⬇️
...gtraum/perf/gcviewer/imp/DataReaderIBM_J9_R28.java 83.09% <0.00%> (-0.71%) ⬇️
...perf/gcviewer/imp/DataReaderUnifiedJvmLogging.java 81.40% <0.00%> (-0.46%) ⬇️
.../tagtraum/perf/gcviewer/model/AbstractGCEvent.java 86.47% <0.00%> (+0.39%) ⬆️
... and 2 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@chewiebug
Copy link
Owner

Hi @mfoulks3200

Thank you very much for your contribution! I'll add synchronization of the zoom factor dropdown (percentage number) with the changes triggered by the mousewheel event. Then I'll merge the pull request.

One question: You chose "shift" as the modifier combined with mouse wheel to do the zooming. I am used to "control" for zooming (e.g. in all browsers). Is there a particular reason for you chosing "shift"? Would it be ok for you to change to "control" instead of "shift"?

@vlsi Thank you very much for your review!

Best regards
Jörg

@mfoulks3200
Copy link
Contributor Author

Hi Jorg,

No worries on the key shortcut, change it to whatever you feel fits best. Shift was just more ergonomic on my mac keyboard.

@vlsi
Copy link

vlsi commented Dec 15, 2022

I think for mac, cmd (meta) + scroll would do better than shift

@mfoulks3200
Copy link
Contributor Author

I've added those changes

@@ -186,7 +186,7 @@ private void maybePopup(MouseEvent e) {
addMouseWheelListener(new MouseWheelListener() {
@Override
public void mouseWheelMoved(MouseWheelEvent e) {
if ((e.getModifiersEx() & InputEvent.SHIFT_DOWN_MASK) != 0) {
if ((e.getModifiersEx() & InputEvent.META_DOWN_MASK) != 0||(e.getModifiersEx() & InputEvent.CTRL_DOWN_MASK) != 0) {
Copy link

Choose a reason for hiding this comment

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

I would rather make the modifier OS-dependent.
In other words, META for macOS, and CTRL for the rest.

Just in case, ctrl+click means "right click" in macOS

Copy link
Owner

Choose a reason for hiding this comment

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

I have pushed a commit to adding the os dependent code using OSXSupport.isOSX().

@chewiebug chewiebug merged commit 561d379 into chewiebug:develop Mar 1, 2023
@chewiebug
Copy link
Owner

The pull request is finally merged.

Unfortunately, Travis thinks that I have no build credits any more and won't build for the moment. Their plan page for my account says different
grafik

I hope, I can fix this soon, to enable automated builds again.

@chewiebug
Copy link
Owner

By the way: @mfoulks3200 thanks again for your contribution and @vlsi for your review comments!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants