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.

Sign up for GitHub

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

Jump to bottom

different line height / vertical spacing / between urlwatch 2.22 and 2.28 #774

Closed
kongomongo opened this issue Nov 9, 2023 · 11 comments
Closed

different line height / vertical spacing / between urlwatch 2.22 and 2.28 #774

kongomongo opened this issue Nov 9, 2023 · 11 comments

Comments

@kongomongo
Copy link
Contributor

Hi there,

I noticed the following after upgrading debian buster to bullseye. I upgraded from urlwatch 2.22 to 2.28 in the process, did not change anything else and the following changed in my HTML output (rendered by Outlook):

2.22:
image

2.28:
image

Is this on purpose? Is this something I can change?

This is the HTML diff:

image

It seems this CSS is hardcoded, any way to change it via YAML?

Many thanks!
@thp
Copy link
Owner

thp commented Nov 14, 2023

Seems like the first part at least is #730

@kongomongo
Copy link
Contributor Author

Yes the coloring was changed in #730 but also the line-height: 1.5 was added which I am really not a fan of. I like my list of changes compact.

What do you suggest? Expose that as a variable? Make a "compact" setting?

@kongomongo
Copy link
Contributor Author

Sorry, found another difference: When using iOS (dark mode enabled) mail app, also looks way different

2.22:
image

2.28:
image

Unfortunately this is extremely hard to read ...

Should this go to #730 ?
What do you think @thp ?

@thp
Copy link
Owner

thp commented Dec 1, 2023

cc @trevorshannon what's your take on this? Any way we can have both, ideally without introducing new options?

@trevorshannon
Copy link
Contributor

Dark mode in email clients is notoriously hard to nail. I'll try to look into the differences between gmail and apple mail sometime. There must be a way to support both. It kinda seems like iOS is double-inverting by inverting the css-specified dark mode background color of #121212 to something like #ededed

As for line heights, I think that the higher line height is definitely better in the HTML table view (as shown in #730), so I bet we can bring line height back to 1em just for plain text output.

@trevorshannon trevorshannon mentioned this issue Dec 2, 2023
Merged
@trevorshannon
Copy link
Contributor

I made PR #777 to adjust the rendered line height for unified diff reports. Note I was not able to see the "spaced out" unified diff in Gmail--it must ignore line-height on <body> or something like that (I don't have Outlook). I did verify correct line height by rendering the HTML in Chrome, but feel free to test it out yourself using my branch @kongomongo

@trevorshannon trevorshannon mentioned this issue Dec 2, 2023
Merged
@trevorshannon
Copy link
Contributor

Ok, and #778 should fix the dark mode bug in Apple Mail on iOS. Again, I could not test on an actual device, but rather used litmus.com to do so.

@kongomongo
Copy link
Contributor Author

As for line heights, I think that the higher line height is definitely better in the HTML table view (as shown in #730), so I bet we can bring line height back to 1em just for plain text output.

I'm sorry but what is HTML table view? Never heard of it.

@kongomongo
Copy link
Contributor Author

kongomongo commented Dec 2, 2023 •
edited
Loading

I made PR #777 to adjust the rendered line height for unified diff reports. Note I was not able to see the "spaced out" unified diff in Gmail--it must ignore line-height on <body> or something like that (I don't have Outlook). I did verify correct line height by rendering the HTML in Chrome, but feel free to test it out yourself using my branch

Works perfectly 👍

Ok, and #778 should fix the dark mode bug in Apple Mail on iOS. Again, I could not test on an actual device, but rather used litmus.com to do so.

Also works! 👍 Looks like this on iOS:

image

@trevorshannon
Copy link
Contributor

trevorshannon commented Dec 2, 2023 •
edited
Loading

Ok, I'm glad that resolves it for you!

I'm sorry but what is HTML table view? Never heard of it.

In the urlwatch config (typically urlwatch.yaml) you can specify a diff value for the html reporter. The email reporter makes use of this value when generating the email, provided you have set html: true within the email reporter config. The possible options for diff are unified (which is what your screenshots show) and table (which is what the screenshots in #730 show). They are analogous to GitHub's "unified" and "split" diff views, respectively.

You can also see a side-by-side comparison of a unified diff and a table diff in PR #777

@thp
Copy link
Owner

thp commented Dec 3, 2023

Thanks @trevorshannon for the quick and easy-to-review fixes, and @kongomongo for the report and verification :)

#777 and #778 have now been merged.

@thp thp closed this as completed Dec 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@thp @trevorshannon @kongomongo