X Tutup
Skip to content

Fix zoom-out on an image [#131080]#134706

Merged
mjbvz merged 1 commit intomicrosoft:mainfrom
cyntler:fix-safari-zoom-out
Oct 15, 2021
Merged

Fix zoom-out on an image [#131080]#134706
mjbvz merged 1 commit intomicrosoft:mainfrom
cyntler:fix-safari-zoom-out

Conversation

@cyntler
Copy link
Contributor

@cyntler cyntler commented Oct 9, 2021

This PR fixes #131080.


const settings = getSettings();
const isMac = settings.isMac;
const isMac = settings.isMac || isMacNavigator();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why isn't settings.isMac on its own enough?

Copy link
Contributor Author

@cyntler cyntler Oct 11, 2021

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 of settings.isMac value.
I assume, process isn't available on the browser side.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, let's remove isMac then so there's only source of truth here

Copy link
Contributor Author

@cyntler cyntler Oct 12, 2021

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?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, remove settings.isMac to use the new function instead. I believe settings.isMac is only used inside the webview

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mjbvz Updated!

@mjbvz mjbvz added this to the October 2021 milestone Oct 11, 2021
@mjbvz mjbvz merged commit a12e9cd into microsoft:main Oct 15, 2021
@mjbvz
Copy link
Collaborator

mjbvz commented Oct 15, 2021

Thanks! This will be in the next insiders build

@cyntler cyntler deleted the fix-safari-zoom-out branch October 15, 2021 22:09
@github-actions github-actions bot locked and limited conversation to collaborators Nov 29, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unable to zoom out on an image

2 participants

X Tutup