elamperti / OpenWebScrobbler

🎧 An open source web scrobbler for Last.fm
https://openscrobbler.com/
GNU General Public License v2.0
319 stars 38 forks source link

format album duration properly on Scrobble Album page #249

Closed mishmish-dev closed 5 days ago

mishmish-dev commented 3 weeks ago

Currently the total duration on Scrobble Album page is rendered incorrectly if the album is longer than 1 hour and the browser in a different timezone than UTC. Screenshot: it shows duration to be 3 hours instead of 1, because I'm in UTC+2

This is happening because the code uses format() from date-fns, and there is no easy way to make it timezone-agnostic for H:mm:ss format.

This commit fixes that.

mishmish-dev commented 3 weeks ago

It would be good to unify this with the duration indicator in ScrobbleItem (although that one is simpler), perhaps moving this helper function to somewhere like src/utils/datetime.ts

Yes, I will try to do it.

mishmish-dev commented 3 weeks ago

@elamperti Done, please check it out. There're two microscopic changes in behavior:

I also created a helper function for formatting scrobble timestamp and moved it to the new utils file, to unclutter ScrobbleItem.tsx a little bit.

mishmish-dev commented 3 weeks ago

Sorry, didn't check my code with yarn lint, fixed these

elamperti commented 3 weeks ago

No worries! Seems like the prettier check is failing now, yarn prettier:fix should be enough to fix it

mishmish-dev commented 3 weeks ago

Added unit tests for formatDuration() helper. The other function in utils.datetime module is harder to test, as it depends on today's date and browser's timezone, I left it aside.

mishmish-dev commented 3 weeks ago

You're welcome! Not sure what is the problem with E2E tests, they worked for me locally.