Skip to content

dash-preview: swap browser window's width and height if landscape #254

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.

Sign up for GitHub

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

Jump to bottom
Merged
antoinerg merged 1 commit into from
Oct 1, 2019
Merged

dash-preview: swap browser window's width and height if landscape #254

antoinerg merged 1 commit into from
Oct 1, 2019

Conversation

@antoinerg
Copy link
Collaborator

@antoinerg antoinerg commented Oct 1, 2019 •
edited
Loading

This PR is a follow-up to #250 which makes the browser size equivalent to the page size to make sure figures have correct dimensions and positioning. Although it works well as reported by @ycaokris, it didn't take into account the landscape option which led to badly laid out PDFs.

The fix is to simply swap width and height of the browser size when printing in landscape mode.

@etpinard
Copy link
Contributor

etpinard commented Oct 1, 2019

Looks good to me 💃

@ycaokris
Copy link

ycaokris commented Oct 1, 2019

Tested with docker image quay.io/plotly/image-exporter:d5d6a741047dc56fa5a63e713725e8fd40bc710e for confined sized snapshot
https://dash-nightly.plotly.host/ddk-manufacture-spc-dashboard/snapshot-1569944213-f1e08e4a?print=true

The outmost container is set to {'width': '11.69in', 'height': '8.23in'} to validate we have same pdf layout for

  1. pageSize {"width":"297000","height":"211000"} as inner key(inside pdf_options), landscape false
  2. pageSize {"width":"297000","height":"211000"} as top-level key, landscape false
  3. pageSize "A4" as inner key, landscape true
  4. pageSize "A4" as outer key, landscape true

Result as follows:

  1. dash_spc_customized_w_h_container_landscape_inner_false.pdf
curl -H "Content-Type: application/json" -X POST -o dash_spc_customized_w_h_container_landscape_inner_false.pdf -d '{"url":"https://dash-nightly.plotly.host/ddk-manufacture-spc-dashboard/snapshot-1569944213-f1e08e4a?print=true", "timeout":"30", "pdf_options": {"marginsType": 1, "landscape": false, "pageSize":{"width":"297000","height":"211000"}}}' http://localhost:9091/dash-preview
  1. dash_spc_customized_w_h_container_landscape_outer_false.pdf
curl -H "Content-Type: application/json" -X POST -o dash_spc_customized_w_h_container_landscape_outer_false.pdf -d '{"url":"https://dash-nightly.plotly.host/ddk-manufacture-spc-dashboard/snapshot-1569944213-f1e08e4a?print=true", "timeout":"30", "pdf_options": {"marginsType": 1, "landscape": false}, "pageSize":{"width":"297000","height":"211000"}}' http://localhost:9091/dash-preview
  1. dash_spc_customized_w_h_container_A4_landscape_inner_true.pdf
curl -H "Content-Type: application/json" -X POST -o dash_spc_customized_w_h_container_A4_landscape_inner_true.pdf -d '{"url":"https://dash-nightly.plotly.host/ddk-manufacture-spc-dashboard/snapshot-1569944213-f1e08e4a?print=true", "timeout":"30", "pdf_options": {"marginsType": 1, "landscape":true, "pageSize": "A4"}}' http://localhost:9091/dash-preview
  1. dash_spc_customized_w_h_container_A4_landscape_outer_true.pdf
curl -H "Content-Type: application/json" -X POST -o dash_spc_customized_w_h_container_A4_landscape_outer_true.pdf -d '{"url":"https://dash-nightly.plotly.host/ddk-manufacture-spc-dashboard/snapshot-1569944213-f1e08e4a?print=true", "timeout":"30", "pdf_options": {"marginsType": 1, "landscape":true}, "pageSize": "A4"}' http://localhost:9091/dash-preview

All tests are giving me proper sized snapshot. This means browser size for landscape is swapped for adaptation. Thanks @antoinerg

The only remaining problem which might need me a tweak is height rounding of container size. A standard A4 page orca has returned to me is measured 11.69 in x 8.26 in(as below). However I have to set container size a bit shorter as 8.23 in in order to get all components fit in,
Screen Shot 2019-10-01 at 12 02 50 PM

a one-to-one size (by setting container 11.69 x 8.26 will start excluding graphs as like

dash_spc_customized_w_h_container_landscape_A4_false_826inx1169in_A4.pdf

This could be replicated on above container by 1v1 matching pdf size to be ~8.23in x 11.69in

and got
Screen Shot 2019-10-01 at 12 21 41 PM

curl -H "Content-Type: application/json" -X POST -o dash_spc_customized_w_h_container_test_1_to_1.pdf -d '{"url":"https://dash-nightly.plotly.host/ddk-manufacture-spc-dashboard/snapshot-1569944213-f1e08e4a?print=true", "timeout":"30", "pdf_options": {"marginsType": 1, "landscape": false}, "pageSize":{"width":"296926","height":"209042"}}' http://localhost:9091/dash-preview

Let me know if we need further test for it. Otherwise everything looks good.

@antoinerg
Copy link
Collaborator Author

antoinerg commented Oct 1, 2019

Otherwise everything looks good.

Ok so I'll merge this

@antoinerg antoinerg merged commit 46eb9d2 into 3.3-release Oct 1, 2019
@antoinerg antoinerg deleted the dash-preview-landscape branch October 1, 2019 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@antoinerg @etpinard @ycaokris