Print fix #19651

sjtrimble · 2017-10-11T06:52:48Z

Improved printing format

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] angular.io application / infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: #19599

What is the new behavior?

Cleaner printing format

Does this PR introduce a breaking change?

[ ] Yes
[x] No

mary-poppins · 2017-10-11T07:04:33Z

You can preview ec2fd09 at https://pr19651-ec2fd09.ngbuilds.io/.

gkalpak

To make the content take up the whole width, you also need a style like:

@media print {
  .mat-sidenav-content {
    margin: 0 !important;
  }
}

gkalpak · 2017-10-11T07:26:39Z

aio/src/styles/1-layouts/_print-layout.scss

+@media print {
+
+    .no-print {
+        display: none;


Why are some elements hidden with the no-print class and others are explcitly mentioned in the rule below?

Because it is still a WIP and I wanted to ask to see if we had a preference on how to handle :)

Why are some elements still hidden with the no-print class and others are explicitly mentioned in the rule below? 😛

gkalpak · 2017-10-11T07:45:33Z

aio/src/styles/1-layouts/_print-layout.scss

+    aio-nav-menu,
+    md-sidenav,
+    md-sidenav.sidenav.mat-sidenav,
+    .toc-container,


Why does this have the no-print class and also be included here?

sjtrimble · 2017-10-12T22:03:02Z

I tried moving as much as I could to the no-print class so it can be used in the future.

@gkalpak thanks for the sidenav content margin hint! :)

mary-poppins · 2017-10-12T22:15:17Z

You can preview df70ef6 at https://pr19651-df70ef6.ngbuilds.io/.

gkalpak

Overall looks good 👍
I left a couple of comments.

I wish we could do a better job with tabbed code panes (e.g. as seen near the bottom of guide/displaying-data), but I don't think it is even possible with CSS only 😞

gkalpak · 2017-10-18T22:37:11Z

aio/src/styles/1-layouts/_print-layout.scss

+@media print {
+
+    .no-print {
+        display: none;


Why are some elements still hidden with the no-print class and others are explicitly mentioned in the rule below? 😛

gkalpak · 2017-10-18T22:40:01Z

aio/src/styles/1-layouts/_print-layout.scss

+    }
+
+    md-sidenav.sidenav.mat-sidenav,
+    .toc-container,


Does this has to be here? Doesn't it already have the no-print class?

The md-sidenav.sidenav.mat-sidenav is the only one I can't figure out how to do without. It works fine when the sidenav is closed, but not when open.

gkalpak · 2017-10-18T22:42:39Z

aio/src/app/embedded/code/code.component.ts

@@ -31,7 +31,7 @@ const defaultLineNumsCount = 10; // by default, show linenums over this number
  selector: 'aio-code',
  template: `
    <pre class="prettyprint lang-{{language}}">
-      <button *ngIf="!hideCopy" class="material-icons copy-button"
+      <button *ngIf="!hideCopy" class="material-icons copy-button no-print"


I can still see copy buttons 😕

Fixed this :)

gkalpak · 2017-10-18T22:46:39Z

aio/src/styles/1-layouts/_print-layout.scss

+    }
+
+    md-sidenav-container.sidenav-container.has-floating-toc {
+        width: 100%;


Wouldn't this be covered by the rule right above?

sjtrimble · 2017-11-01T19:01:15Z

@gkalpak Will you take a look again :) I made a few more changes and addressed all but 1 of your comments. I'm also not sure what to do about the md-tabs :/

mary-poppins · 2017-11-01T19:09:52Z

You can preview 1391bf3 at https://pr19651-1391bf3.ngbuilds.io/.

gkalpak

A few more comments/questions to go!
Also:

Once build(aio): update to @angular/core@5.0.0-rc.5 #19702 is merged, the md- prefix of material components needs to be replaced with mat-.
As I said before, there is not much we can do for tabs. Can you at least add a special style for .mat-tab-label-active elements to show what the active tab is. (E.g. make the text bold or something.)

gkalpak · 2017-11-01T21:11:53Z

aio/src/styles/1-layouts/_print-layout.scss

+    }   
+
+    img {
+        max-width: 100% !important;


Where does this make a difference? Can you point out an example where this is needed/desired?

I don't think there is an example currently but it would prevent any image that is specifically sized in pixels from being cut off on a printed page.

gkalpak · 2017-11-01T21:13:50Z

aio/src/styles/1-layouts/_print-layout.scss

+    }
+
+    p {
+        widows: 4;


Wow, I had never heard of this one!

gkalpak · 2017-11-01T21:18:47Z

aio/src/styles/1-layouts/_print-layout.scss

+    }
+
+    .content code {
+        border: 0.5px solid $lightgray;


This looks bad inside code-examples:
Could we not apply this style inside code-example/code-pane elements?

gkalpak · 2017-11-01T21:20:41Z

aio/src/styles/1-layouts/_print-layout.scss

+        margin: 0 auto;
+        width: 95%;
+        background-color: white;
+        page-break-inside: avoid;


Is this necessary here? What does it do?

ngbot · 2018-01-16T15:38:55Z

Hello? Don't want to hassle you. Sure you're busy. But this PR has some merge conflicts that you probably ought to resolve.
That is... if you want it to be merged someday...

ngbot · 2018-01-16T15:42:19Z

Hello? Don't want to hassle you. Sure you're busy. But this PR has some merge conflicts that you probably ought to resolve.
That is... if you want it to be merged someday...

IgorMinar · 2018-02-06T23:50:17Z

@sjtrimble can you please finish this PR? It's currently listed as something we'd like to get done for v6 (March 2018)

sjtrimble · 2018-02-09T00:27:39Z

Ready for review @IgorMinar and @gkalpak :)

mary-poppins · 2018-02-09T00:34:23Z

You can preview 651d236 at https://pr19651-651d236.ngbuilds.io/.

IgorMinar

I tested this on various pages and it looks good to me. Definitely a huge improvement compared to the current state.

But let's wait for George to lgtm this as well as he's more familiar with the implementation of our stylesheets.

IgorMinar · 2018-02-09T05:48:52Z

Can you please squash the changes to get the lint CI status to a green state?

mary-poppins · 2018-02-09T18:22:24Z

You can preview f51aaa8 at https://pr19651-f51aaa8.ngbuilds.io/.

printfix

mary-poppins · 2018-02-09T22:32:15Z

You can preview 60a2b5e at https://pr19651-60a2b5e.ngbuilds.io/.

sjtrimble · 2018-02-09T22:32:38Z

@gkalpak Take a look one last time :) I rebased and cleaned up the dupe aio-code code borer bit.

ngbot · 2018-02-10T11:22:13Z

I see that you just added the PR action: merge label, but the following checks are still failing:
status "continuous-integration/travis-ci/pr" is failing

If you want your PR to be merged, it has to pass all the CI checks.

If you can't get the PR to a green state due to flakes or broken master, please try rebasing to master and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help.

printfix PR Close #19651

This reverts commit b54ad05.

printfix PR Close angular#19651

angular-automatic-lock-bot · 2019-09-13T07:52:32Z

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.}

sjtrimble added the state: WIP label Oct 11, 2017

googlebot added the cla: yes label Oct 11, 2017

gkalpak reviewed Oct 11, 2017

View reviewed changes

gkalpak mentioned this pull request Oct 11, 2017

angular.io pages don't print well #19599

Closed

sjtrimble force-pushed the print-fix branch from ec2fd09 to df70ef6 Compare October 12, 2017 22:02

gkalpak reviewed Oct 18, 2017

View reviewed changes

sjtrimble force-pushed the print-fix branch from df70ef6 to 1391bf3 Compare November 1, 2017 18:58

gkalpak requested changes Nov 1, 2017

View reviewed changes

jasonaden added the comp: aio label Nov 14, 2017

gkalpak mentioned this pull request Dec 14, 2017

Angular.io documentation printer friendly or PDF. #21012

Closed

IgorMinar mentioned this pull request Jan 24, 2018

angular.io improvements #21710

Closed

34 tasks

IgorMinar added this to the v6-candidates milestone Feb 6, 2018

IgorMinar assigned sjtrimble Feb 6, 2018

sjtrimble force-pushed the print-fix branch from 94e1d62 to 651d236 Compare February 9, 2018 00:26

IgorMinar self-requested a review February 9, 2018 05:46

IgorMinar added action: review The PR is still awaiting reviews from at least one requested reviewer target: patch This PR is targeted for the next patch release and removed state: WIP labels Feb 9, 2018

IgorMinar approved these changes Feb 9, 2018

View reviewed changes

gkalpak removed the action: review The PR is still awaiting reviews from at least one requested reviewer label Feb 9, 2018

sjtrimble force-pushed the print-fix branch 2 times, most recently from d1a9f6d to f51aaa8 Compare February 9, 2018 18:15

fix(aio): improve printing styles

60a2b5e

printfix

sjtrimble force-pushed the print-fix branch from f51aaa8 to 60a2b5e Compare February 9, 2018 22:18

gkalpak added action: merge The PR is ready for merge by the caretaker and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Feb 10, 2018

gkalpak mentioned this pull request Feb 12, 2018

feat(aio): enable print stylesheets #22171

Closed

matsko requested review from IgorMinar and gkalpak February 14, 2018 23:15

matsko closed this in b54ad05 Feb 14, 2018

matsko pushed a commit that referenced this pull request Feb 14, 2018

fix(aio): improve printing styles (#19651)

1e3e0fa

printfix PR Close #19651

matsko added a commit that referenced this pull request Feb 15, 2018

Revert "fix(aio): improve printing styles (#19651)"

ba9cd5b

This reverts commit b54ad05.

vicb pushed a commit to vicb/angular that referenced this pull request Feb 15, 2018

fix(aio): improve printing styles (angular#19651)

7078fbf

printfix PR Close angular#19651

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 13, 2019

Print fix #19651

Print fix #19651

Uh oh!

Conversation

sjtrimble commented Oct 11, 2017

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Uh oh!

mary-poppins commented Oct 11, 2017

Uh oh!

gkalpak left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sjtrimble commented Oct 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mary-poppins commented Oct 12, 2017

Uh oh!

gkalpak left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sjtrimble commented Nov 1, 2017

Uh oh!

mary-poppins commented Nov 1, 2017

Uh oh!

gkalpak left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ngbot bot commented Jan 16, 2018

Uh oh!

ngbot bot commented Jan 16, 2018

Uh oh!

IgorMinar commented Feb 6, 2018

Uh oh!

sjtrimble commented Feb 9, 2018

Uh oh!

mary-poppins commented Feb 9, 2018

Uh oh!

IgorMinar left a comment

Choose a reason for hiding this comment

Uh oh!

IgorMinar commented Feb 9, 2018

Uh oh!

mary-poppins commented Feb 9, 2018

Uh oh!

mary-poppins commented Feb 9, 2018

Uh oh!

sjtrimble commented Oct 12, 2017 •

edited

Loading