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

feat(core): support beforeprint and afterprint hooks #1080

Merged

Conversation

epelc
Copy link
Contributor

@epelc epelc commented Jun 6, 2019

Extends the PrintHook service to also register beforeprint and afterprint event handlers to synchronously update styles and prevent layout switching races.

The PrintHook service was good at handling show/hide but layout changes between row/column were having many race issues and more often than not they were not working while printing due to the underlying matchMedia api being async.

Related #603

See #603 (comment) for proposal and more info.

CC @CaerusKaru @ThomasBurleson


The main issue I had figuring this out was actually the deactivation of printing and getting the PrintHook service to restore the original media queries. I ended up figuring out a bit how the deactivations are stored and that essentially the following events happen.

  • beforeprint fires
  • many mql with matches: false trigger
    • these are added to the deactivations array.
    • Had to keep track of extra isPrintingBeforeAfterEvent boolean since beforeprint fires before these mql changes
  • print mql change fires with matches: true

This kind of explains the flow a little better. Just leaving this and the above to document how this all works since it was kind of a pain to figure out.
image

@ThomasBurleson ThomasBurleson self-assigned this Jun 7, 2019
@ThomasBurleson ThomasBurleson added the P2 Issue that is important to resolve as soon as possible label Jun 7, 2019
@epelc epelc force-pushed the fix/print-layout-switch-beforeprint branch 2 times, most recently from 569d6a2 to 6a370ae Compare June 7, 2019 04:19
@epelc
Copy link
Contributor Author

epelc commented Jun 7, 2019

I fixed the lint issues and added a check for isPlatformBrowser before registering the window before/after print event listeners to support server side rendering(falls back to previous ssr fallback after mql print).

epelc added a commit to epelc/flex-layout that referenced this pull request Jun 7, 2019
@epelc epelc force-pushed the fix/print-layout-switch-beforeprint branch from 6a370ae to 443df46 Compare September 27, 2019 15:50
@epelc
Copy link
Contributor Author

epelc commented Sep 27, 2019

@CaerusKaru @ThomasBurleson This is updated for the latest 8.0.0-beta27 now.

If anyone needs to test easily I've deployed a separate installable build branch. Update your package.json to use

  "dependencies": {
    "@angular/flex-layout": "github:epelc/flex-layout#fork-publish-beta27-pr1080"
}

Please let me know if there are any changes I can make to help get this merged. It is very important for my projects and I know 9.x will be here soon so it'd be nice to have it in before then.

@CaerusKaru CaerusKaru added this to the 9.0.0-beta.28 milestone Oct 13, 2019
@epelc epelc force-pushed the fix/print-layout-switch-beforeprint branch from 443df46 to 64101d6 Compare October 14, 2019 14:21
@epelc
Copy link
Contributor Author

epelc commented Oct 14, 2019

@CaerusKaru I switched over to the document defaultView

@CaerusKaru
Copy link
Member

@epelc Looks like some issues with your cast of DOCUMENT to Document. Perhaps @alan-agius4 can provide some insight?

Also would it be possible to add a unit test for this new feature?

@epelc epelc force-pushed the fix/print-layout-switch-beforeprint branch from 64101d6 to d8ab973 Compare October 15, 2019 19:11
Extends the PrintHook service to also register beforeprint and afterprint event handlers to synchronously update styles and prevent layout switching races.

Related angular#603
@epelc epelc force-pushed the fix/print-layout-switch-beforeprint branch from d8ab973 to a55a801 Compare October 15, 2019 19:39
@epelc
Copy link
Contributor Author

epelc commented Oct 15, 2019

@CaerusKaru the document should be fixed build/test work. I switched it to any which is what is used elsewhere in the project for document injections.

I'm not really sure how to write tests for this. I tried looking for the print-hook tests and there is only one small one in the media-marshaller.spec.ts which doesn't really check much. I haven't had much experience with testing in angular so I'm not quite sure how to fake all the events needed. It doesn't look like anything is testing the interceptEvents or collectActivations which are the primary important additions to ensure that print mode is still activated until afterprint fires.

@CaerusKaru CaerusKaru merged commit 8302998 into angular:master Nov 6, 2019
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Dec 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes P2 Issue that is important to resolve as soon as possible pr: needs review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants