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

The browser zoom out is inconsistent when using bpmn keyboard controls binding. #1362

Closed
FredaPinto opened this issue Oct 22, 2020 · 28 comments
Assignees
Labels
bug Something isn't working

Comments

@FredaPinto
Copy link

Hi There,

I have added below line of code for keyboard controls.
keyboard: { bindTo: document },
Issue is happening when user uses browser (command and + ),(command and -)the (cmd and -) has inconsistent behavior only canvas zooms out but not whole page.
Any help or suggestion would be highly appreciated.Thank you !

Steps to Reproduce:-

  1. Open a new bpmn canvas from https://demo.bpmn.io/new.
  2. Use (command,+) from keyboard and (cmd, _)keys to zoom out and in.

3.You will notice for zoom in the whole page zooms when using keyboard controls but for zoom out only canvas zooms out not whole page.

Environment:-

  • Browser: [e.g.Google Chrome Version 85.0]
  • OS: [e.g. Mac Catalina 10.15.6]
  • Library version: [e.g. 2.0.0]
@FredaPinto FredaPinto added the bug Something isn't working label Oct 22, 2020
@pinussilvestrus
Copy link
Contributor

image

Are these your environment details?

If no, would be appreciated if you could add them. It would help us to understand whether it's an OS/browser related issue.

@pinussilvestrus pinussilvestrus added the help wanted Extra attention is needed label Oct 25, 2020
@FredaPinto
Copy link
Author

Hi Niklas,

Yes they are my environment details,and i also checked it on Firefox 75.0 (64 -bit )version i could see same issue.
Let me know if you need any additional details.
Thank you !

@pinussilvestrus
Copy link
Contributor

What are you focusing on while you're using the keyboard shortcuts? The Canvas, one of the Components of the BPMN Modeler (Palette, Context Pad, ...), or are you focusing outside of the viewport?

@pinussilvestrus
Copy link
Contributor

For me it works as expected, zooming in and zooming out just zooms the Canvas

Kapture 2020-10-27 at 14 03 18

@barmac
Copy link
Member

barmac commented Oct 27, 2020

On Chrome 84, Cmd+[+/=] does not allow me to zoom just the canvas. It does not matter whether I clicked on the canvas or not.

bug

@FredaPinto
Copy link
Author

FredaPinto commented Oct 27, 2020

@barmac thats so helpful,thanks for capturing the screen recording.

@FredaPinto
Copy link
Author

@pinussilvestrus my focus is on canvas itself,but still the same issue happens.

@pinussilvestrus
Copy link
Contributor

pinussilvestrus commented Oct 27, 2020

Thanks for sharing @barmac! Interesting that it works for me on Chrome 86

@FredaPinto
Copy link
Author

@pinussilvestrus i upgraded my chrome to 86.0 still see same issue.So im attaching zip file for you to check.Thanks !
demo.mov.zip

@pinussilvestrus
Copy link
Contributor

@barmac could you double-check whether this a general problem with bpmn-js or simply related to the demo?

@barmac
Copy link
Member

barmac commented Nov 3, 2020

I will do that.

@barmac
Copy link
Member

barmac commented Nov 3, 2020

Chrome 86 with vanilla bpmn-js: Cmd + [+] is handled by the browser while Cmd + [-] is zooming out the canvas.

@FredaPinto
Copy link
Author

any updates ?would be nice to know if there is any update or workaround.Thanks !

@pinussilvestrus
Copy link
Contributor

@barmac any guess? I'm still unable to reproduce it, although we seem to use the same setup (Chrome 86 + MacOS).

@barmac
Copy link
Member

barmac commented Nov 6, 2020

I got it. It's the matter of the keyboard layout. If you press button with + on an international/US/PL keyboard, = is actually triggered. This is not the case on a German keyboard 🥇 .

So to fix this, I believe we should add mapping for = key. Then, if user has a country-specific keyboard layout, they could use some custom extensions if they don't want = to trigger zoom.

barmac added a commit to bpmn-io/diagram-js that referenced this issue Nov 6, 2020
This is required for the feature to work on international
keyboard layout.

Related to bpmn-io/bpmn-js#1362
@barmac
Copy link
Member

barmac commented Nov 6, 2020

This will be fixed via bpmn-io/diagram-js#498

@barmac barmac added in progress Currently worked on and removed help wanted Extra attention is needed labels Nov 6, 2020
@barmac barmac self-assigned this Nov 6, 2020
@nikku
Copy link
Member

nikku commented Nov 6, 2020

How can this be? I thought the browser would pass on the actual key being pressed?

@barmac
Copy link
Member

barmac commented Nov 6, 2020

It does until you press the key together with Control or Command. Then, the event always contains the basic key value, so = instead of +, 0 instead of ( etc.

@barmac
Copy link
Member

barmac commented Nov 6, 2020

By basic I mean the value without Shift.

@nikku
Copy link
Member

nikku commented Nov 6, 2020

Well then, is there a way to handle this one properly via key codes then? The implication of your suggested change would be that = zooms on regional keyboards, i.e. German, too, right?

@nikku
Copy link
Member

nikku commented Nov 6, 2020

Then, if user has a country-specific keyboard layout, they could use some custom extensions if they don't want = to trigger zoom.

This is exactly what I'd like to avoid, if that is possible.

@barmac
Copy link
Member

barmac commented Nov 6, 2020

If we switch to keyCode, zoom in will be triggered for accent letters on the German keyboard: ´´ (the first one to the left from Backspace).

@FredaPinto
Copy link
Author

@barmac Thanks for taking this in progress, so will this issue be addressed in specific next release ?

@barmac
Copy link
Member

barmac commented Nov 12, 2020

If we merge the fix, then it should be incorporated in the next issue.

@nikku nikku closed this as completed in 9792dad Nov 12, 2020
@bpmn-io-tasks bpmn-io-tasks bot removed the in progress Currently worked on label Nov 12, 2020
@nikku
Copy link
Member

nikku commented Nov 12, 2020

Fixed with v7.4.1 of the library.

nikku added a commit that referenced this issue Nov 19, 2020
@FredaPinto
Copy link
Author

@nikku just wanted to confirm v7.4.1 is it released ? and which is the exact module that this library refers too ?
Thanks in advance.

@barmac
Copy link
Member

barmac commented Nov 23, 2020

The v7.4.1 of bpmn-js is already available via npm and it contains the fix as revealed in the changelog.

@FredaPinto
Copy link
Author

Thanks @barmac .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

No branches or pull requests

4 participants