search

FreshRSS / Extensions

A repository containing all the official FreshRSS extensions
GNU Affero General Public License v3.0
321 stars 52 forks source link

Update ReadingTime Extension for Safari Support #171

Closed VeryDampTowel closed 7 months ago

VeryDampTowel commented 10 months ago

Change li.style.width from min-content to hardcoded 40px.

Apparently Safari doesn't support min-content so the reading time is not shown in Safari.

I figure 40px is plenty of room for most reading times, but feel free to change it up.

Reference: https://stackoverflow.com/a/7085104

Alkarex commented 10 months ago

Ping @sal0max

Frenzie commented 10 months ago

If you follow the link to the fiddle in the link you'll note min-width does seem to work on tables in modern Safari, although given the diff is about min-content it's less clear how relevant that is regardless.

sal0max commented 9 months ago

The Stack Overflow link is about Safai not supporting the min-width attribute. Your PR is changing the min-content value of the width attribute, @parispjones

So, yeah.. we should change something, here. I tested some combinations, and li.style.width = "40px"; seems sufficient in all cases. Agreed. To keep the code clean, we should remove li.style.minWidth = "40px";, however.

VeryDampTowel commented 9 months ago

@sal0max You are correct. Sorry for the poor initial phrasing. I removed minWidth so we should be good now.

Alkarex commented 9 months ago

Please also bump the version number https://github.com/FreshRSS/Extensions/blob/master/xExtension-ReadingTime/metadata.json

VeryDampTowel commented 9 months ago

Done!

Alkarex commented 9 months ago

Arg, I should have merged this PR before https://github.com/FreshRSS/Extensions/pull/169 Could you try to re-apply your changes here @math-GH to fix the conflicts?

Alkarex commented 7 months ago

Ping @math-GH

math-GH commented 7 months ago

Ping @math-GH

Done. Thanks for the reminder.