Add zoom, zoom in, & zoom out shortcuts

[bonez0607]

Purpose / Goal

Added zoom shortcuts specified in issue #107

Hello,

This is my first real PR - so I hope I'm doing things correctly...

I added a shortcut to 'click' the zoom button. From there the user can then use shortcuts to zoom in or out. I assumed this was the desired effect since the buttons aren't mounted until the zoom button is clicked, correct?

I'm on a mac and was having issues (even with existing shortcuts) with e.key == [appropriate key] && e.altKey working. However when I used the symbol created when pressing both alt and the event key it worked as expected.

For example e.key == '=' && e.altKey didn't work but e.key == '≠' did for the alt + '+' shortcut.

If I need to change to the previous formatting of e.key && e.altKey let me know and I can change back. I checked the shortcuts in Chrome, Firefox and Safari.

There were also a couple of quick mis commented items that I fixed.

Type

Please mention the type of PR

• Bug Fix
• Refactoring / Technology upgrade
• New Feature
• Documentation
• Other : | Please Specify |

[amitguptagwl]

I added a shortcut to 'click' the zoom button. From there the user can then use shortcuts to zoom in or out. I assumed this was the desired effect since the buttons aren't mounted until the zoom button is clicked, correct?

Good point. I didn't think about that. We can probably move zoom in/out buttons to js functions. And the buttons in zoom-action.tag.html can just call those functions. In this way, a user needs not to open the zoom action bar to use the shortcuts.

For example e.key == '=' && e.altKey didn't work but e.key == '≠' did for the alt + '+' shortcut.

Need to be cross-checked but great if it works.

[bonez0607]

That makes sense in regard to having them work without needing the action bar open. I can update it to reflect that.

Is there a specific js file you’d like the script to be in?

Lastly, would I need to open up a new PR or just amend this one?

[amitguptagwl]

Is there a specific js file you’d like the script to be in?

Probably common-actions.js

Lastly, would I need to open up a new PR or just amend this one?

this PR would be fine. I'll squash the changes before merging.

Important: Please check following once you complete the PR

1. Import 2 images.
2. Label 1st image. and zoom in/out
3. again label it and zoom in/out
4. check console if there is no error
5. check if there is any change in labels position when zoom in/out

In case of confusion, you can share the screenshot or GIF for clear understanding.

[bonez0607]

Alright I think I got everything working how it's supposed to. Just want to clarify some things.

• 1. Import 2 images.

• 2. Label 1st image. and zoom in/out

3. again label it and zoom in/out

Just want to clarify that the tags are supposed to scale with the image, correct?

• 4. check console if there is no error

No errors show in console

5. check if there is any change in labels position when zoom in/out
   label scales with image and maintains position - See gif

[amitguptagwl]

Yes! the labeled shapes are supposed to scale along with the image. But we've previously noticed that (due to a bug which is fixed now) if you label an image, change the zoom level, relabel and zoom again then the positions of label gets changed.

[bonez0607]

Ok, the zoom functions have been added to a file called js/common-actions.js. And cleaned up from the other files.

[amitguptagwl]

So keyboard shortcuts along with direct button click are working fine?

[amitguptagwl]

Thanks. So keyboard shortcuts along with direct button click are working fine?

…

On Sun, 21 Oct 2018 at 20:02, Joe B. ***@***.***> wrote: Ok, the zoom functions have been added to a file called common-actions.js. And cleaned up from the other files. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#122 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHVgKNnnZDpR8xbi4WZVjaXTp0wLK_CFks5unIVogaJpZM4Xbki4> .

-- Cheers *Amit Gupta*

[amitguptagwl]

@bonez0607 Have you get the chance to test it from buttons as well as from shortcuts?

[bonez0607]

Sorry, it's was a busy weekend. I can take a look to double check tomorrow night.

[amitguptagwl]

Sure no problem.

[bonez0607]

So the zoom shortcuts work and the buttons work w/out an error. However, when using the keyboard shortcut the percentage increments/decrements by 20% rather than the 10% change when using the buttons. I checked to make sure nothing was being called twice, as that seemed like a potential culprit.

[amitguptagwl]

I hope you must have console log to check if that function is being called twice. and You must have prevented event propagation.

[bonez0607]

Was able to track down the offending code. Checked in Chrome, Firefox, and Safari - buttons and keyboard are working.

[amitguptagwl]

great. Now you have to remove the console.log statement from your code. I'll suggest you review your code one more time, just to avoid any bug in future.

[bonez0607]

Doh! ok - should be good.

[amitguptagwl]

Thanks for the changes. I'll separately check your changes after 2 days before merge.