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

Year in Music 2023 #2668

Merged
merged 59 commits into from
Jan 3, 2024
Merged

Year in Music 2023 #2668

merged 59 commits into from
Jan 3, 2024

Conversation

MonkeyDo
Copy link
Member

@MonkeyDo MonkeyDo commented Dec 12, 2023

This year's YIM page rolled up into one big PR, just the way we like it !

As with previous years, it is my opinion that creating a test suite for this yearly feature is not particularly necessary and very time consuming for the projected lifetime of the feature.

Here is a list of what we are still currently missing, using the figma design document and the checklist document as a reference:

  • coming soon placeholder page -(monkey: haven't had time to set it up but should be easy, should have time in the next few days) Ran out of time
  • adding the newest YIM stats (genres, …) to the page props (lucifer)
  • displaying those new stats on the page (monkey)
  • mass email design implementation and testing (lucifer?)
  • testing and tweaking the shareable SVGs (monkey)
  • stats overview SVG (?) if we still want and have time to do that
  • prepare special sidebar +top navbar links to YIM23 (monkey, end of the month)
  • cover art composite image (mayhem)
  • "friends" section at the bottom: we could show a piece of info from 's friends' YIM report, as in the mockup, if that's not too much data to fetch Ran out of time

@pep8speaks
Copy link

pep8speaks commented Dec 12, 2023

Hello @MonkeyDo! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 450:131: E501 line too long (172 > 130 characters)
Line 544:131: E501 line too long (135 > 130 characters)
Line 545:131: E501 line too long (138 > 130 characters)

Comment last updated at 2024-01-03 11:32:04 UTC

MonkeyDo and others added 23 commits December 20, 2023 16:26
pesky "display: table" style left over from last year's YIM component
For some reason the height is 0 in Firefox. The other graphs work fine.
Garbled the previous one during copy paste
The utility we use to render SVGs (Canvg) does not take the `letter-spacing` property into account, and we need to customize the kerning manually using the `dx` attribute
For reference: canvg/canvg#668
cropped a little tight
In the ListenCard component, if we have no info at all to display any cover art, ensure there is a an empty thumbnail element to maintain text alignment with other listencards with cover art
Woopsie !
I needed to change the margin rather than the padding, which refers to the padding between each bar. When = 1, no bars are shown at all.
Improving mobile phone layout
@MonkeyDo MonkeyDo marked this pull request as ready for review December 27, 2023 15:27
@MonkeyDo MonkeyDo merged commit af119d7 into master Jan 3, 2024
4 checks passed
@MonkeyDo MonkeyDo deleted the yim-2023 branch January 3, 2024 13:20
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.

3 participants