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

Report: print doesn't cut off expanded audit details #1870

Merged
merged 1 commit into from
Mar 21, 2017

Conversation

zixu-zhao
Copy link

@zixu-zhao zixu-zhao commented Mar 16, 2017

Hi,

I apologize I mess up the git tree of the previous brunch. I just delete the older pull request and brunch, and create a new one. This one would be clear. I also improve the code to support shortcut expanding.

Thanks,
Zixu Zhao

Original request message:

did some researches of the issue: Print ->Save as PDF : Pages truncated #1780, and here is my solution.

The main problem I found is, once the window.print() is called. The printing process will take current page height immediately. In fact, the function _setupExpandDetailsWhenPrinting works as expected in my tests, i.e. all details are labeled open=true when triggering print event.

So the only problem is, for example, the page height is 2112px before the expanding, and the printing take it as 3 pages pdf file; however, after the expanding, the page height is 9230px, and it requires 11 pages pdf file. Then, the last 8 pages are truncated in this case.

I found two possibilities.

First, I could solve it by adding a min-height: 10000px under the @media print . It is obviously a bad idea. However, it works, so it confirms my hypophysis above.

Second, since my searching work implies that there is no way to change the page height dynamically in CSS or after window.print() is called, I would like to solve the problem through calling expand before calling window.print(). I left the collapse listener for the event when printing complete.

Closes #1780

@zixu-zhao
Copy link
Author

zixu-zhao commented Mar 16, 2017

@ebidel Could we move the PR #1860 here? I apologize I mess up the git tree of the previous brunch.

window.addEventListener('afterprint', _ => {
details.map(detail => detail.open = false);
});
} else {
// Note: while FF has media listeners, it doesn't fire when matching 'print'.
window.matchMedia('print').addListener(mql => {
details.map(detail => detail.open = mql.matches);
if(!mql.matches) {
Copy link
Contributor

Choose a reason for hiding this comment

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

spacing: if (!mql.matches) {

@ebidel
Copy link
Contributor

ebidel commented Mar 21, 2017

Tried this and it looks great! Just the one style comment.

@ebidel ebidel changed the title solve page truncated including short cut Report: print doesn't cut off expandde audit details Mar 21, 2017
@zixu-zhao
Copy link
Author

Hi, I updated the style of if (!mql.matches) { and squashed the commits.

@ebidel ebidel changed the title Report: print doesn't cut off expandde audit details Report: print doesn't cut off expanded audit details Mar 21, 2017
@ebidel ebidel merged commit 16c2b99 into GoogleChrome:master Mar 21, 2017
@zixu-zhao zixu-zhao deleted the fix/pageTruncated branch March 21, 2017 23:53
ebidel added a commit that referenced this pull request Mar 29, 2017
* Report: add legend to decipher iconography left to us by the ancient ones (#1841)

* Report: print doesn't cut off expanded audit details (#1870)

* Report: increase icon size for a11y (#1856)

* Tweak report colors so that we are WCAG2AA valid.

* Biggin icons

* CLI: add update-notifier. Fixes #1805 (#1890)

* Fixes #1907 - move update-notifier to root (#1908)

* CLI: remove npm prepublish (#1889)
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 this pull request may close these issues.

2 participants