Refactor now playing #1241
Refactor now playing #1241
Conversation
There was a problem hiding this comment.
Not sure what caused the options panel lag when this was first merged but since this PR only updates the CSS that bug should still be here.
Using a Set would be simpler and less error prone than a bit field. The banner board list uses this approach.
I'm not a fan of the stacked banners. Instead of changing the default look, why not design it to be easy to style the way you like with custom CSS?
| switch (oldNowPlaying) { | ||
| case "false": | ||
| case "none": | ||
| localStorage.setItem("nowPlaying", "" + (0 ^ 0)); |
There was a problem hiding this comment.
| localStorage.setItem("nowPlaying", "" + (0 ^ 0)); | |
| localStorage.setItem("nowPlaying", "0"); |
This is just confusing. Add a comment if you think the reader won't understand bit flags. Same with the others.
There was a problem hiding this comment.
Maybe it's unnecessary for that part, but I think particularly in specs.ts it makes it clearer how to add new options.
| break | ||
| let matched = false; | ||
| let s: string[] = []; | ||
| for (const i of enabled) { |
There was a problem hiding this comment.
Post name replacement should only happen when r/a/dio matches.
There was a problem hiding this comment.
I disagree. It currently happens if eden is the only enabled station, so I see no reason to exclude it now.
I don't think that actually was lag, it was odd behaviour that just seemed like it. Should be fixed.
Maybe. I'll admit I didn't look at how the board list worked before writing this, but logic mistakes aside I still quite like this way.
Things already get quite tight with both stations enabled. If there were 3 or more enabled, it would be a cramped mess unless you had a massive monitor. The default should be usable. The new html layout does make it easy to put it back inline with custom css if you prefer it that way. |
|
Seems to be fixed. Merging. |
Had the wrong part of the overlay stickied last time. This should work properly.