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

Image preview should not zoom on first click if unfocused #81877

Closed
mjbvz opened this issue Oct 2, 2019 · 7 comments · Fixed by #83826
Closed

Image preview should not zoom on first click if unfocused #81877

mjbvz opened this issue Oct 2, 2019 · 7 comments · Fixed by #83826
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug verification-found Issue verification failed verified Verification succeeded
Milestone

Comments

@mjbvz
Copy link
Collaborator

mjbvz commented Oct 2, 2019

Repo

  1. Open an image to the side of a text editor
  2. Start with the text editor focused
  3. Now click into the image editor to focus it

Bug
The first click ends up zooming the image. Clicking should only zoom when the editor is already focused

@mjbvz mjbvz added help wanted Issues identified as good community contribution opportunities good first issue Issues identified as good for first-time contributors labels Oct 2, 2019
@mjbvz mjbvz self-assigned this Oct 2, 2019
@mjbvz
Copy link
Collaborator Author

mjbvz commented Oct 2, 2019

Here's the relevant code about click handling in the image preview:

container.addEventListener('click', (/** @type {MouseEvent} */ e) => {

@mjbvz mjbvz added the bug Issue identified by VS Code Team member as probable bug label Oct 2, 2019
@mjbvz mjbvz changed the title Image preview Image preview should not zoom on first click if unfocused Oct 2, 2019
@jeanp413
Copy link
Contributor

jeanp413 commented Oct 5, 2019

If focus is required for zoom, then the cursor when hovering over the image preview should be the default arrow cursor when unfocused, currently it is the zoom-in cursor. Also zooming with the mouse wheel should not be allowed when unfocused. Thoughts?

@mjbvz mjbvz removed good first issue Issues identified as good for first-time contributors help wanted Issues identified as good community contribution opportunities labels Oct 9, 2019
@mjbvz
Copy link
Collaborator Author

mjbvz commented Oct 14, 2019

Fixed by #82074

@mjbvz mjbvz closed this as completed Oct 14, 2019
@mjbvz mjbvz added this to the October 2019 milestone Oct 14, 2019
@alexr00
Copy link
Member

alexr00 commented Oct 30, 2019

I'm still seeing the first click zoom. Steps:

  1. Open a typescript file.
  2. Open a gif
  3. Move gif to the right so that both typescript and gif are visible and side by side
  4. Click in typescript.
  5. Click in gif
    Expected: no zoom
    Actual: it zooms on the click in the gif in (5).

@alexr00 alexr00 reopened this Oct 30, 2019
@alexr00 alexr00 added the verification-found Issue verification failed label Oct 30, 2019
@MartinBrathen
Copy link
Contributor

@alexr00 I'm not able to reproduce. Have tried on both insiders 1.40.0 and OSS.

Does this only occur with gifs for you?

@mjbvz
Copy link
Collaborator Author

mjbvz commented Oct 31, 2019

I can repo this. You may need to click back and forth multiple times trigger the error

@MartinBrathen
Copy link
Contributor

I'm still not able to. Will try more tomorrow.

A possible cause might be that the message event is fired before the mousedown event on the first click. This could be tested by checking the order of execution for the corresponding event handlers in /vscode/extensions/image-preview/media/main.js. I'll do it if I manage to reproduce the bug.

@kieferrm kieferrm modified the milestones: October 2019, November 2019 Oct 31, 2019
@roblourens roblourens added the verified Verification succeeded label Dec 4, 2019
@vscodebot vscodebot bot locked and limited conversation to collaborators Dec 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug verification-found Issue verification failed verified Verification succeeded
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants