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

make printing palatable #1579

Merged
merged 9 commits into from
Nov 9, 2021
Merged

make printing palatable #1579

merged 9 commits into from
Nov 9, 2021

Conversation

jchorl
Copy link
Contributor

@jchorl jchorl commented Nov 7, 2021

There's a fair amount of hacked up css surgery going on here. I added comments for what each part does. I'm super flexible on where this code should go, how we should organize it, etc. This is just a first pass. Please provide feedback!

Here's an old & new printed report:
old.pdf
new.pdf

  • This comment contains a description of changes (with reason)
  • CHANGELOG.md has been updated

@jchorl
Copy link
Contributor Author

jchorl commented Nov 8, 2021

@ewels I think this is the last of my PRs :P

You can see from the examples what it does. Np if you want to play around with it, it feels extremely hacky. Fwiw, these were the configs I was using locally. Now that we have css overrides, I can just apply these myself, so no rush ;)

It mostly feels bad that it makes multiqc much more fragile. If the html changes, the print styling will almost certainly break...

I wouldn't be offended if you didn't want to merge.

Copy link
Member

@ewels ewels left a comment

Choose a reason for hiding this comment

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

Love this! I have always neglected PDF exports and sort of as an extension, printing. Now that I see what you've done here I'm wondering why.. Very simple addition and makes a huge improvement to the output 🖨️ ✨

If the html changes, the print styling will almost certainly break...

Yeah, but I can't see that this would be any worse than the current state of not having any print styling.. So not really a big issue as far as I can see (also the template rarely changes these days).

@ewels
Copy link
Member

ewels commented Nov 9, 2021

I was a bit confused at first as I couldn't see some of the effects when testing locally. Then I realised that not all the classes had been propagated through all of the templates. I guess you're mostly using -t simple, but I see no reason why the default template can't also look nice when printed. So I ported (most of) the new class names and markup over to all templates.

I've also had a bit of a play and added a bunch of additional CSS, mostly for stuff in the default interactive template. For example, making tables dumb and entirely visible and so on. Looking great now!

CHANGELOG.md Outdated Show resolved Hide resolved
@ewels
Copy link
Member

ewels commented Nov 9, 2021

Black CI is failing but seems to be due to an issue with the docker container. Passes without issue locally ✅

@ewels ewels merged commit 25e0788 into MultiQC:master Nov 9, 2021
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