New player and theming

[hucario]

wow, that title sounds underwhelming. This has been an ongoing discussion over in #53, so none of this is news, but... Wow! A new player, with

• snazzy new icons
• animations
• grids (begone, LISTS)
• theming support
• you can now dislike things from history
• a large amount of tomfoolery
• new JS
• volume slider is now horizontal??? :eyes:
• snazzy login screen

and that's not all! You also get: A theming page with

• ~1~ ~2~ ~8~ 10 theming options
• inport/export JSON capabilities
• all-new CSS

Plus, if you don't like it, the old player is still there! No changes to it ~that you can see~! Nada! If you don't like it, take it back to the store for a refund!

"but hucario, " - I hear you ask - "we still have no screenshots! How are we supposed to judge whether this is actually quality?" Say no more! (don't mind my terrible taste in music) (why was They Might Be Giants playing on "chillstep radio"??)

[pvrs12]

Awesome to see this in a PR. I'll probably move the conversation here since it should be easier to reference code directly. A few things:

1. Is it possible to theme the colors of the buttons? If someone (potentially insane) wants a light theme then the buttons become less usable.
2. I'm getting an issue saving the theme after an import on theming.js:222. I've added a comment to the code about this
3. I've not dug into the code completely yet, but I'll give it a full review to make sure you haven't dumped any coin miners in it or anything 😉 (and more to learn how to actually write JS, CSS, and HTML)
4. A bit of a nitpick, but I'd like to try to keep whitespace consistent through the project. I think all of the existing code is 4 spaces.
5. I noticed a bug with the album cover art in the station selector. If you change stations it'll update the previous station with the new station's art.
6. The station list should highlight the currently playing station.

I think that covers everything for now. I'm happy to pick some of these up, but I don't have any familiarity with the CSS stuff (yet), so you're probably better suited to tackle them.

Once again, fantastic work!

[pvrs12]

Oh, and IDK if you were affected by this, but something to note: https://haveibeenpwned.com/PwnedWebsites#000webhost

[hucario]

Awesome to see this in a PR. I'll probably move the conversation here since it should be easier to reference code directly. A few things:

Is it possible to theme the colors of the buttons? If someone (potentially insane) wants a light theme then the buttons become less usable.

Well... yes... but it makes the code a lot less clean. If you inline an SVG (paste its code into the HTML directly instead of just referencing it with an img tag), then you can add a fill: blue in the CSS. There may be a solution I could have for that, though. (Namely, on page load replace all svgs with inline versions)

I'm getting an issue saving the theme after an import on theming.js:222. I've added a comment to the code about this

Resolved (I think) :+1:

I've not dug into the code completely yet, but I'll give it a full review to make sure you haven't dumped any coin miners in it or anything wink (and more to learn how to actually write JS, CSS, and HTML)

:+1:, I would do the same. Not that I'm untrusting, but letting a random stranger on the internet put unreviewed code into my extension seems like a bad idea.

A bit of a nitpick, but I'd like to try to keep whitespace consistent through the project. I think all of the existing code is 4 spaces.

Sure, I'll do a global search+replace for tabs -> four spaces Edit: Resolved.

I noticed a bug with the album cover art in the station selector. If you change stations it'll update the previous station with the new station's art.

I think I know what's causing that?? I'll try to figure it out Edit: Resolved.

The station list should highlight the currently playing station.

Ah, good idea. I'll get to working on that. Edit: Resolved.

I think that covers everything for now. I'm happy to pick some of these up, but I don't have any familiarity with the CSS stuff (yet), so you're probably better suited to tackle them.

Once again, fantastic work!

Thanks :)

[pvrs12]

Well... yes... but it makes the code a lot less clean. If you inline an SVG (paste its code into the HTML directly instead of just referencing it with an img tag), then you can add a fill: blue in the CSS. There may be a solution I could have for that, though. (Namely, on page load replace all svgs with inline versions)

So the original solution I did for the hover colors on the buttons (which were PNGs) was to CSS filter with hue-rotate and saturate. It may be possible to do some fancy math to convert the RGB hex codes to HSV and apply them to the buttons in CSS.

Does that sound sane to you? I don't really know CSS too well

[hucario]

Well... yes... but it makes the code a lot less clean. If you inline an SVG (paste its code into the HTML directly instead of just referencing it with an img tag), then you can add a fill: blue in the CSS. There may be a solution I could have for that, though. (Namely, on page load replace all svgs with inline versions)

So the original solution I did for the hover colors on the buttons (which were PNGs) was to CSS filter with hue-rotate and saturate. It may be possible to do some fancy math to convert the RGB hex codes to HSV and apply them to the buttons in CSS.

Does that sound sane to you? I don't really know CSS too well

That's actually... a very, very good idea, or at least the start of one. Unfortunately, you can't hue-rotate white to anything but white, so what I'll do is this: I'll recolor all the buttons to.. let's say red. Then, yes, fancy math that I'll have to figure out (insert sad face here) but it'll be worth it because it'll still be so much simpler than inlining it all. Neat.

[hucario]

Alright, now everything is fixed except for recoloring the buttons. Edit: Hm... How am I going to handle recoloring the active (liked/disliked) buttons? I think I'll have to add another theme option for that. Rip. Edit 2: Unfortunately, parsing that all and doing all of the math would take a very long time for not much benefit, so I'll try the other method first.

[pvrs12]

it's a foreground bit, you could just make it the same as Text Color ¯\_(ツ)_/¯

I'll poke around with the RGB to HSV math a bit, see if I can't make something goofy work

[hucario]

Yeah, after about 2 hours of trying, I'm giving up and making the HTML messy. rip edit: actually, let me try something. I can't change the node type or do node.outerHTML or else I lose all properties, but if I just change... one sec, I'm going to work on this now edit 2: oh yeah, it's all coming together

function fillSvgs() {
    let x = document.querySelectorAll('svg');
    for (let i = 0; i < x.length; i++) {
        fetch(x[i].getAttribute('data-src')).then((d) => {
        d.text().then((e) => {
                x.setAttribute('xmlns', 'http://www.w3.org/2000/svg');
                x.setAttribute('viewBox', x.match(/viewBox="[0-9 ]*"/g)[0].match(/"[0-9 ]*/g)[0].substring(1));
                x[i].innerHTML = e.replace(/<svg[A-Za-z 0-9"=\n:/.\-]*>/g,'').replace(/<\/svg>/g, '');
            });
        });
    }
}

[hucario]

I've gotta get some food, this is giving me a headache.

[pvrs12]

Oh no... I hope this doesn't ruin everything, but using innerHTML is recommended against because it can be vulnerable and it makes it difficult for mozilla/chrome to vet the extension.

https://developer.chrome.com/extensions/security#avoid

https://developer.mozilla.org/en-US/docs/Web/API/Element/innerHTML#Security_considerations

[pvrs12]

https://codepen.io/sosuke/pen/Pjoqqp

This is an option

[hucario]

https://codepen.io/sosuke/pen/Pjoqqp

This is an option

Ooo, that looks good. I'll see what I can do with it, as what I've been doing before has been... unfruitful. So unfruitful, in fact, that I deleted what I've been doing for the past 3 hours and git clone'd again

[hucario]

Found this very clever solution: https://codepen.io/danielzen/pen/GRJzpxv

[pvrs12]

That looks absolutely perfect!

[hucario]

Alright, I'm getting off for the night, but before I go, here's a smidgen of hope:

[pvrs12]

Hell yeah! Appropriate song title too

On Sun, May 17, 2020, 1:28 AM hucario notifications@github.com wrote:

Alright, I'm getting off for the night, but before I go, here's a smidgen of hope: [image: image] https://user-images.githubusercontent.com/50851047/82136627-935dbe80-97c4-11ea-825b-73d7140cfe83.png

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/pvrs12/Anesidora/pull/54#issuecomment-629745794, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABVCK4P2VDRFUB6NGTE757TRR5YZFANCNFSM4NDBJKFQ .

[hucario]

@pvrs12 I feel simultaneously stupid and relieved that it was this simple to solve this issue. R E S O L V E D

[hucario]

yeah, I'm about done. you can do your code-checking now.

[hucario]

sorry - NOW I'm done. don't check the code that was there before the latest commit, it was trash anyways

[pvrs12]

Awesome, giving it an eye-ball now

[pvrs12]

Moving to new branch to merge prior to some code changes I'm going to make