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

[Bug]: Zoom in Safari is not working as expected #20888

Closed
Spielboerg opened this issue Feb 2, 2023 · 6 comments · Fixed by #21069
Closed

[Bug]: Zoom in Safari is not working as expected #20888

Spielboerg opened this issue Feb 2, 2023 · 6 comments · Fixed by #21069

Comments

@Spielboerg
Copy link
Contributor

Spielboerg commented Feb 2, 2023

Describe the bug

The CSS zoom is not working correctly in Safari if rem values are used. See a minimal reproduction here:
https://codepen.io/spielboerg/pen/GRBPRRE

Here is the comparison between Safari & Google Chrome:
Safari Bug

I have already filed a bug report with Apple about this. Until this bug is fixed, it might be better if the zoom was done with transform: scale() like for Firefox.

To Reproduce

  1. Open the official Storybook: https://storybookjs.netlify.app/official-storybook/?path=/story/addons-actions--arg-types-example
  2. Change the padding of the button in the devTools to 1rem
  3. Increase the zoom in Storybook via SCR-20230202-g58 (not the native browser zoom via CMD/Ctrl + +/-)
  4. See that the button is not scaled properly (top: normal zoom, bottom: zoomed in):
    Storybook

System

Environment Info:

  System:
    OS: macOS 13.2
    CPU: (10) arm64 Apple M1 Max
  Binaries:
    Node: 16.17.0 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 8.18.0 - /usr/local/bin/npm
  Browsers:
    Chrome: 109.0.5414.119
    Firefox: 109.0
    Safari: 16.3
  npmPackages:
    @storybook/addon-docs: ^6.5.10 => 6.5.16 
    @storybook/addon-essentials: ^6.5.16 => 6.5.16 
    @storybook/addon-links: ^6.5.16 => 6.5.16 
    @storybook/addons: ^6.5.10 => 6.5.16 
    @storybook/builder-webpack5: ^6.5.16 => 6.5.16 
    @storybook/manager-webpack5: ^6.5.16 => 6.5.16 
    @storybook/preset-create-react-app: ^4.1.2 => 4.1.2 
    @storybook/react: ^6.5.16 => 6.5.16 
    @storybook/testing-library: ^0.0.13 => 0.0.13 
    @storybook/theming: ^6.5.10 => 6.5.16

Additional context

No response

@ndelangen
Copy link
Member

I've labelled this as a good first contribution, because it's a pretty contained feature.
I can support whoever wants to work on this. Please reach out via discord: https://discord.gg/storybook

@blacktoast
Copy link

Hello, Do you mind if I try to fix it? As It is my first time on this project, could you recommend any particular document or codes to focus on?
I also entered a discord

@Spielboerg
Copy link
Contributor Author

@blacktoast Maybe in the next 2 weeks I will have time to try it as well, but please go ahead. So far I have found these two related files:

import { global } from '@storybook/global';
export function browserSupportsCssZoom(): boolean {
try {
// @ts-expect-error (we're testing for browser support)
return global.document.implementation.createHTMLDocument('').body.style.zoom !== undefined;
} catch (error) {
return false;
}
}

setIframeInnerZoom(scale: number) {
try {
if (browserSupportsCssZoom()) {
Object.assign(this.iframe.contentDocument.body.style, {
zoom: 1 / scale,
minHeight: `calc(100vh / ${1 / scale})`,
});
} else {
Object.assign(this.iframe.contentDocument.body.style, {
width: `${scale * 100}%`,
height: `${scale * 100}%`,
transform: `scale(${1 / scale})`,
transformOrigin: 'top left',
});
}
} catch (e) {
this.setIframeZoom(scale);
}
}

I'm not sure how to change the test in browserSupportsCssZoom() without adding an exception for Safari, since the existing test is pretty straight forward. But maybe you or @ndelangen have an idea how to solve this problem.

@blacktoast
Copy link

I don't have any ideas either, other than adding an exception for Safari in browserSupportsCssZoom().

@ndelangen
Copy link
Member

@blacktoast that sounds like a good solution to me! I'd welcome a PR making that change!

Please ping me, if you open a PR, I'll take a look ❤️

@shilman
Copy link
Member

shilman commented Feb 17, 2023

¡Ay Caramba!! I just released https://github.com/storybookjs/storybook/releases/tag/v7.0.0-beta.50 containing PR #21069 that references this issue. Upgrade today to the @next NPM tag to try it out!

npx sb@next upgrade --prerelease

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants