Print long topic to PDF, redux, again

feedback

(Barry van Oudtshoorn) #39

Looks great!

I’ve noticed on Firefox that I sometimes get the browser-native print dialog pop up immediately, then your new popup window once I close the print dialog, and then the desired print dialog. I also noticed that the popup-blocker kicked in – so is the keypress perhaps not prevent propagation of the event, and is it perhaps opening the popup after an async call, rather than as a direct result of the keypress?


(Rafael dos Santos Silva) #40

Exactly. I couldn’t find a way to properly stop propagation on the print event. (it doesn’t look possible)


(Barry van Oudtshoorn) #41

I don’t know if it’s possible on the print event, but it’s possible on the key combo. (I didn’t know there was a print event!) Note that you can’t use keypress – that only works in Firefox, as the other browsers seem to handle the default print in keydown.

Here’s the super-simple approach:

https://jsfiddle.net/0kch2s63/2/

It works correctly in both Firefox and Chrome. Unfortunately in Edge you still get the printing popup – but the event is handled appropriately. Not sure about Safari – I don’t have a Mac on me at present. :slight_smile:


(Rafael dos Santos Silva) #42

This was happening indeed in earlier versions. Since this line got added, it doesn’t happen anymore with me on latest Firefox at Ubuntu Linux.


(Rafael dos Santos Silva) #43

I tried an approach were we could re-use the CSS we have:

Not sure if it’s the best way, but this way we doesn’t bloat the crawler version for bots and keep the style DRY.


One of the pages of our longest topic (157 pages long) on meta with quotes and oneboxes:


(Jeff Atwood) #44

@falco can you summarize the final result here?


(Rafael dos Santos Silva) #45

As requested, there is no UI for printing.

So any user can start to print by pressing CTRL+P when on a long topic. (This shortcut is show in our keyboard help)

For example, going to The State of JavaScript on Android in 2015 is... poor and pressing CTRL+P will open a popup with a print dialog asking to print 64 pages.


(Tobias Eigen) #46

Nice work! I love this!

I don’t suppose there is a way to exclude certain (or all) html customizations from the javascriptless or print view, is there? Right now it shows up as a bulleted table of contents because it’s not including the accompanying css, which we don’t want.

Example:


(cpradio) #47

Sure you can! Or at least in theory. I haven’t had a chance to put it in practice yet though.

@media print {
  .menu-primary-container { display: none }
}

You can also hide such things from the crawler using

body.crawler {
  .menu-primary-container { display: none }
}

Edit: and by crawler, I mean the crawler view the print window seems to utilize. Google will still see your links as I don’t think it parses CSS to know what content it should index (I could be wrong on that though).


(Kane York) #48

(This is off topic)
I think Google does do this, I remember some posts talking about how they detected font-color: #fffffe on keyword stuffing as being too invisible to see.


(Angus McLeod) #49

There is actually a slight inconsistency in the crawler.html.erb as it includes theme html, but doesn’t include theme css. (see here; compare with application.html.erb which includes everything in discourse_stylesheet.html.erb).

This means any custom html added by a theme will appear in the print (or other crawler-based views), but cannot be styled, or removed via styling.

I’ve made a pr to add theme styles to the crawler view.

cc @Falco


(Sam Saffron) #50

I wonder if what we want here is a specific css theme piece for crawler view, uneasy just adding the blanket app css here cause people need to think about this if the want to customize css in crawler view


(Angus McLeod) #51

Yes, I was thinking the same thing.

Although, including the theme css is arguably better than current state, which just includes the un-styled theme html.


(Sam Saffron) #52

Not necessarily cause it could potentially cause visual breakage, nobody tests it


(Rafael dos Santos Silva) #53

I do agree here.

This is certainly better!

I wonder with our current structure, how hard is to pull only the CSS that affects posts here @awesomerobot?


(Kane York) #54

Also note that the HTML structure in crawler view is completely different to that of normal topic pages. I think that improving the no-js/crawler view to look a bit more like the normal rendering is a good idea, but it’s also kinda a prereq to doing that.

(They also don’t render small-actions at all, which is a much more important thing to fix.)


(Kris) #55

A theme with a dark background would probably fail, plus I’m not sure someone with a dark theme would want to always print it that way (likely harder to read on paper and eats through all your ink). So we’d want some logic/modification (maybe we strip out color and keep other styles?), which leads back to:

To me it seems like the easiest path here stylistically is to do something with our no-JS view which already seems light on printing and easy on reading.


(Sam Saffron) #56

Sorry, just to clarify, is your vote to add “crawler.scss” file as an optional theme asset? Or something else?


(Kris) #57

Ah I didn’t read back far enough, yeah that was a bit unclear — I was suggesting sticking with the crawler view for print instead of trying to apply theme styles automatically.

So, a crawler.scss file as an optional is a good idea for flexibility, but we could also improve the default crawler view and print stylesheet a bit.


(Angus McLeod) #58

Sorry to bother, but just to clarify, did you decide on a next step here?

I’ll withdraw my current PR, but I’m happy to help (if appropriate) with whatever the solution is to the issue with having custom html but no custom css in the crawler view.