Skip to content

Only zoom after first click, after the webview editor is active. - #81933

Closed
Caige Nichols (caiges) wants to merge 8 commits into
microsoft:masterfrom
caiges:no-zoom-on-first-click
Closed

Only zoom after first click, after the webview editor is active.#81933
Caige Nichols (caiges) wants to merge 8 commits into
microsoft:masterfrom
caiges:no-zoom-on-first-click

Conversation

@caiges

@caiges Caige Nichols (caiges) commented Oct 4, 2019

Copy link
Copy Markdown

This changes the image preview to zoom only after an initial click event.

@caiges Caige Nichols (caiges) changed the title Only zoom after first click, after the webview editor is active. Only zoom after first click, after the webview editor is active. Fixes #81877 Oct 4, 2019
@caiges Caige Nichols (caiges) changed the title Only zoom after first click, after the webview editor is active. Fixes #81877 Only zoom after first click, after the webview editor is active. Oct 4, 2019
@caiges

Caige Nichols (caiges) commented Oct 4, 2019

Copy link
Copy Markdown
Author

Fixes #81877

@caiges Caige Nichols (caiges) mentioned this pull request Oct 4, 2019
Closed
mjbvz
Matt Bierner (mjbvz) reviewed Oct 4, 2019
View reviewed changes
Comment thread extensions/image-preview/media/main.js
@caiges

Caige Nichols (caiges) commented Oct 8, 2019

Copy link
Copy Markdown
Author

There looks to be two duplicate PRs attempting to address this issue: #82074 and #81935

@msftclas

Microsoft Contribution License Agreements (msftclas) commented Oct 10, 2019 •
edited
Loading

Copy link
Copy Markdown

CLA assistant check
All CLA requirements met.

@MartinBrathen

MartinBrathen commented Oct 11, 2019

Copy link
Copy Markdown
Contributor

Hi,

I tested your code and found a bug:

When giving the preview focus by either clicking the tab, or with ctrl + number key, you then have to click twice inside the preview in order to zoom.

I ran into this bug myself while trying solve the same issue without using the postMessage(...) method. I was not able to fix the bug without using it.

@caiges

Caige Nichols (caiges) commented Oct 11, 2019

Copy link
Copy Markdown
Author

Yeah, I think passing state via postMessage is necessary. Do you want to consolidate our efforts?

@MartinBrathen

MartinBrathen commented Oct 11, 2019 •
edited
Loading

Copy link
Copy Markdown
Contributor

Sure!

I'm quite new to working on large github projects so I don't know how that would work. What do you have in mind?

@mjbvz

Matt Bierner (mjbvz) commented Oct 14, 2019

Copy link
Copy Markdown
Contributor

Closing this in favor of #82074

This change looks like a good approach overall but I think that #82074 also handles a few other cases. Sorry that I didn't unmark the original issue as help wanted when the first PR came in to let others know you were working on it

@caiges
Caige Nichols (caiges) deleted the no-zoom-on-first-click branch October 14, 2019 22:04
@github-actions github-actions Bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@mjbvz Matt Bierner (mjbvz) Awaiting requested review from mjbvz

Assignees

@mjbvz Matt Bierner (mjbvz)

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@caiges @msftclas @MartinBrathen @mjbvz