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

Plot outputs incorrectly sized inside scaled outputs #4135

Closed
gadenbuie opened this issue Sep 27, 2024 · 1 comment · Fixed by #4139
Closed

Plot outputs incorrectly sized inside scaled outputs #4135

gadenbuie opened this issue Sep 27, 2024 · 1 comment · Fixed by #4139

Comments

@gadenbuie
Copy link
Member

When plot outputs appear inside scaled regions of the DOM, Shiny reports the output area in absolute pixels (i.e. the size of the output area after the scaling is applied). The returned image, however, ends up being smaller than desired because it doesn't take into account the scaling.

You can see this happening in the superzip example.

image

I've applied a red outline to the plot output area and a blue outline to the actual image. Ideally these would overlap and the image would take up the full output area.

CSS
.shiny-plot-output {
    outline: 1px solid red;
}

.shiny-plot-output > img {
    outline: 1px solid blue;
}

In this example, on app load, the histCentile output reports size 259 x 180px and receives an image with those dimensions. But the output appears in an absolute panel (#controls) that has zoom: 0.9 applied. Those are indeed the absolute dimensions of the #histCentile container after the browser has scaled the output container. Once placed, the image ends up with the absolute size of 233.09 x 162px. The image should instead be 287.78 x 200px (taking into account the scaling) to end up with the correct size.

@gadenbuie
Copy link
Member Author

gadenbuie commented Sep 30, 2024

The error first appeared in shinycoreci 2024-08-29. This is most likely when GitHub rolled out the 20240825.1 MacOS image -- or generally updated their runners to use Chrome 128 (from 127). This is almost certainly because Chrome 128 changed their implementation of zoom to be in line with standards.

The implementation of the previously non-standard CSS zoom property has been updated to align with the new standard. This changes various JavaScript APIs to align with the specification, changes zoom to apply to iframe content documents, and changes it to apply to all inherited length properties where previously it only changed the inherited font-size.

Thanks to this useful comment providing context: select2/select2#6338 (comment).

Also, from the Chromium dev mailing list:

This is working as expected; here's the relevant spec languageweb platform test and wpt.fyi results.

You may find the newly-added Element.currentCSSZoom property useful for handling these values. If you think there's missing functionality, please file a spec issue.

jcheng5 added a commit that referenced this issue Oct 1, 2024
CSS zoom property affects el.getBoundingClientRect() but not
el.offsetWidth/Height. When reporting sizes of outputs from
client to server, we need to back out the CSS zoom because
those sizes are used as CSS width/height, which will be
affected by zoom.

(Note that something similar happens with CSS transforms but
we don't have a good way to deal with them)
jcheng5 added a commit that referenced this issue Oct 1, 2024
CSS zoom property affects el.getBoundingClientRect() but not
el.offsetWidth/Height. When reporting sizes of outputs from
client to server, we need to back out the CSS zoom because
those sizes are used as CSS width/height, which will be
affected by zoom.

(Note that something similar happens with CSS transforms but
we don't have a good way to deal with them)
jcheng5 added a commit that referenced this issue Dec 8, 2024
* Fix #4135: Plot outputs incorrectly sized inside scaled outputs

CSS zoom property affects el.getBoundingClientRect() but not
el.offsetWidth/Height. When reporting sizes of outputs from
client to server, we need to back out the CSS zoom because
those sizes are used as CSS width/height, which will be
affected by zoom.

(Note that something similar happens with CSS transforms but
we don't have a good way to deal with them)

* Squelch TS error

* `yarn build` (GitHub Actions)

* Add TODO

Co-authored-by: Garrick Aden-Buie <garrick@adenbuie.com>

* Rebuild JS

---------

Co-authored-by: jcheng5 <jcheng5@users.noreply.github.com>
Co-authored-by: Garrick Aden-Buie <garrick@adenbuie.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant