thp / urlwatch

Watch (parts of) webpages and get notified when something changes via e-mail, on your phone or via other means. Highly configurable.
https://thp.io/2008/urlwatch/
Other
2.81k stars 352 forks source link

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

Closed kongomongo closed 8 months ago

kongomongo commented 9 months ago

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 commented 9 months ago

Seems like the first part at least is https://github.com/thp/urlwatch/pull/730

kongomongo commented 9 months ago

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 commented 9 months ago

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 commented 8 months ago

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

trevorshannon commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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 commented 8 months ago

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

777 and #778 have now been merged.