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

Feature: print email #1804

Merged
merged 5 commits into from
Aug 7, 2019
Merged

Conversation

sourenaraya
Copy link
Contributor

Fixes #579

new dependency introduced, not sure if that is ok.
Ctrl+P works.

sourenaraya and others added 2 commits June 11, 2019 19:08
Hybrid method used: hide everything except mail body using CSS, adjust iframe height before printing using JS

Signed-off-by: Souren Araya <[email protected]>
@sourenaraya
Copy link
Contributor Author

any update on review?

package.json Outdated Show resolved Hide resolved
templates/index.php Outdated Show resolved Hide resolved
@ChristophWurst
Copy link
Member

Let's wait for @jancborchardt's review

@sourenaraya
Copy link
Contributor Author

Sorry for asking this again, but, any update on this?

@jancborchardt
Copy link
Member

Sorry, I was recently on vacation and have to get up to speed again. :) Will check it soon. @sourenaraya just for clarification: There’s no interface change to check for, only Ctrl-P and browser menu print functionality, correct?

@sourenaraya
Copy link
Contributor Author

Yep, just Ctrl+P as usual.

@jancborchardt
Copy link
Member

Works very nicely, good job @sourenaraya! Only details:

  • In Firefox: If you print via the menu → Print functionality you get a preview. If you scroll there, the mail-message-header (with subject, sender and recipient) is still sticky so floating above the text. Maybe it should be changed to position: relative; for printing? It does come out fine in the PDF
  • Also in Firefox/Ubuntu, when printing to PDF, is it possible to preset the file name to something, like the mail title? For me the filename is always the same as last time unless I rename.
  • In the printout, the date of the email is not included. Should we have this? Might be something for a follow-up too.

In general nothing blocking and we could also merge it like this 👍 :)

@sourenaraya
Copy link
Contributor Author

sourenaraya commented Jul 31, 2019

i've set position: relative to mail-message-header, thanks.

  • print-to-pdf function: AFAK there is no way to suggest filename to browser. Its always mozilla.pdf, or something set in about:config
  • date in the printout: date is not present on plaintext, as well as on richtext emails, anywhere. First of all, we have to somehow include it in backend responses, or have a way to retrieve date information from backend. If there is any, i am not aware of it.

Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

Works very well, 👍 from me. :) Great job @sourenaraya! 🚀

Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

There are a few edge cases with complex HTML emails but other than it works as advertised :)

Thanks a lot!

@ChristophWurst ChristophWurst merged commit aafa4b8 into nextcloud:master Aug 7, 2019
@ChristophWurst
Copy link
Member

@sourenaraya please don't forget to go to https://www.bountysource.com/issues/50806693-print-stylesheet and claim your bounty :)

@sourenaraya
Copy link
Contributor Author

Issue still marked as open on the bountysource 😂

@ChristophWurst
Copy link
Member

Ugh. Bountysource is so buggy. Sorry about that. Tried to get support via https://twitter.com/ChristophWurst/status/1159344197436104704. Let's see 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Print stylesheet [$65 awarded]
3 participants