- Notifications
You must be signed in to change notification settings - Fork33.8k
Fix zoom-out on an image [#131080]#134706
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| @@ -59,7 +63,7 @@ | |||
| ]; | |||
| const settings = getSettings(); | |||
| const isMac = settings.isMac; | |||
| const isMac = settings.isMac || isMacNavigator(); | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Why isn'tsettings.isMac on its own enough?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@mjbvz VS Code outside of Electron also works in the browser, right?
Running in a browser, even on MacOS,settings.isMac will return false. I think we should add browser side checking as well. Besides as I tested, this fixes the zoom out bug in Chrome.
function isMac(): boolean {if (typeof process === 'undefined') {return false;}return process.platform === 'darwin';}It's function body ofsettings.isMac value.
I assume,process isn't available on the browser side.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Ok, let's removeisMac then so there's only source of truth here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@mjbvz Ok, so do you think that we can only stay when checking isMacNavigator and isMac can be thrown out completely?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Yes, removesettings.isMac to use the new function instead. I believesettings.isMac is only used inside the webview
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@mjbvz Updated!
Thanks! This will be in the next insiders build |
This PRfixes#131080.