SongInfoStyle
Add customizable style for SongInfo, right align metadata text on player card on Home page and limit the max size of metadata
What does this change intend to accomplish?
Metadata on player cards can take up a lot of space, reduce that max space and wrap text as necessary
See images in first comment for a diff
Checklist
- [x] Have you tested your changes and ensured they work?
- [x] Have you checked to ensure there aren't other open Pull Requests for the same update/change?
- [x] If applicable, have you updated the documentation/manual?
- [x] If applicable, have you updated the CHANGELOG?
- [x] Does your submission pass linting & tests? You can test on localhost using
./scripts/test - [x] Have you written new tests for your core features/changes, as applicable?
- [x] If this is a UI change, have you tested it across multiple browser platforms?
- [x] If this is a UI change, have you tested across multiple viewport sizes (ie. desktop versus mobile)?
Old:
New:
On mobile, note how much taller it is due to the width limiting. The intention is to keep the sources/stream data from fighting for space on mobile:
Old:
New:
I'll also note: These are largely stylistic choices, I know the maxwidth thing assisted with some fighting for space but the right text align just looks better on player cards imo so you don't have a blob of text floating in the middle of the card
Was testing on a unit capable of multi-zone play, found more bugs, fixed them I've made it so that the zone names are dynamically moved around to make space for the longer ones as needed to not cover or otherwise impact metadata Note that it is still possible to cover up metadata with a zone name, but only if you have a single extremely long word
Old:
New:
Looking into ways of potentially limiting max size in previous screenshots
Looking into ways of potentially limiting max size in previous screenshots
Figuring this out is nontrivial and has minimal dividends so I am no longer pursuing it
what github issue does this address?
None to my knowledge, it was a change added in #703 that I added to attempt to prevent zones overlapping with metadata (generally the metadata is long, I tried and failed to do the same with a long zone)
not a huge fan of the wrapping change, and I dont' think you've necessarily made the case that a) this is a problem we should spend time solving, and b) this is the best solution to that problem. making these boxes variable height is the quickest way to make this look messy imo.
I'm not sure I like the vertical resizing either, that was just the easiest way to go about fixing this. I could technically also do dynamic font scaling, but that sounded messy
New version, now with ellipsis wrapping (based onto #710 to avoid making the same changes to Chip twice)
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 49.51%. Comparing base (
b516bb7) to head (a296d1a). Report is 58 commits behind head on main.
:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@ Coverage Diff @@
## main #708 +/- ##
==========================================
- Coverage 50.90% 49.51% -1.39%
==========================================
Files 25 26 +1
Lines 5838 6349 +511
==========================================
+ Hits 2972 3144 +172
- Misses 2866 3205 +339
| Flag | Coverage Ξ | |
|---|---|---|
| unittests | 49.51% <ΓΈ> (-1.39%) |
:arrow_down: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
is the original
SongInfo.jsxstill in use, or can it be removed?
The original Songinfo is still in use on the player page
I'm still hesitant to approve this, because I see no less than 5
useStates that can cause re-renders and auseEffectthat watches 3 of them. Is there any possible way to reduce the amount of state stored and reacted upon?
State could potentially be managed better, can't say I know how as everything here makes sense to me. I could maybe cut out the useEffect and put it on some sort of regular check timer but I don't think that's actually an improvement
This is leaking off the rightmost edge of playercards atm, needs more work but I have no opportunity to dig that deep atm
what other solutions have you examined for this problem?
I answered this in a slack DM, but to answer it publicly as well: I've tried doing it myself in CSS and that proved to have far too many levers that were all extremely sensitive to any little change and it made the animation look very chunky and didn't fully scroll like I wanted to (instead snapping back to the start position when the text was fully offscreen rather than wrapping back around to come to a stop), so this was the only alternative that I could find The library I'm using is depended on by 34k+ repos so I had the feeling it was a good route to go down even if it lacks a small amount of tooling that I've managed to create with that state handling I can pare down the state a little more yet, that windowSize ref is uneccessary and was only there because I blindly turned almost all states into refs instead when it can realistically be just the window.innerWidth at all times