New player and theming #54

hucario · 2020-05-16T19:13:01Z

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??? 👀
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"??)

Yeesh that took a long time. New player includes: + New icons (need to change one) + Snazzy login screen + Begone, LISTS. We have _grids_ here, which means that the station list & history list are now grids + just general tomfoolery + settings screen now shows a full* preview, not just a placeholder square *ok full admission you can't interact with it and it doesn't show the other panels, but it's still PRETTY GOOD

common/js/theming.js

pvrs12 · 2020-05-16T20:32:08Z

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.
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
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)
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.
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.
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 · 2020-05-16T20:33:37Z

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

hucario · 2020-05-16T20:45:08Z

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) 👍

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)

👍, 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 · 2020-05-16T21:08:57Z

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 · 2020-05-16T21:15:46Z

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 · 2020-05-16T21:18:06Z

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 · 2020-05-16T21:28:04Z

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 · 2020-05-16T23:01:25Z

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 · 2020-05-16T23:45:29Z

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

pvrs12 · 2020-05-16T23:59:23Z

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 · 2020-05-17T00:02:21Z

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

This is an option

hucario · 2020-05-17T04:38:40Z

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 · 2020-05-17T05:02:35Z

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

pvrs12 · 2020-05-17T05:05:42Z

That looks absolutely perfect!

hucario · 2020-05-17T05:28:39Z

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

pvrs12 · 2020-05-17T06:01:49Z

Hell yeah! Appropriate song title too

…

On Sun, May 17, 2020, 1:28 AM hucario ***@***.***> 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 <#54 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABVCK4P2VDRFUB6NGTE757TRR5YZFANCNFSM4NDBJKFQ> .

hucario · 2020-05-17T21:47:49Z

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

that took _way_ too long, but at least it's done now ¯\_(ツ)_/¯

real programmers use nano

hucario · 2020-05-17T22:30:12Z

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

hucario · 2020-05-17T22:31:29Z

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

pvrs12 · 2020-05-18T00:25:51Z

Awesome, giving it an eye-ball now

pvrs12 · 2020-05-19T21:41:14Z

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

hucario added 4 commits May 15, 2020 11:01

Merge branch 'master' of https://github.com/hucario/Anesidora

1f3c6df

Player defaults to controls without needing JS

9eebbff

theming support 👀 👀

ac54e21

pvrs12 reviewed May 16, 2020

View reviewed changes

common/js/theming.js Outdated Show resolved Hide resolved

pvrs12 mentioned this pull request May 16, 2020

Redesign Thread #53

Closed

https://github.com/pvrs12/Anesidora/pull/54#discussion_r426187898

bace446

hucario added 2 commits May 16, 2020 14:00

tabs -> 4 spaces

0e91302

Merge branch 'master' of https://github.com/hucario/Anesidora

4c8bd1e

bugfix + active station is highlighted

caebe4e

hucario added 2 commits May 17, 2020 15:08

you can theme svgs now

6b60763

that took _way_ too long, but at least it's done now ¯\_(ツ)_/¯

bugfix???

bac0d67

real programmers use nano

quick, hide the crypto-miners

adeb3eb

pvrs12 changed the base branch from master to v2-new-player May 19, 2020 21:40

pvrs12 merged commit 9602f64 into pvrs12:v2-new-player May 19, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New player and theming #54

New player and theming #54

hucario commented May 16, 2020 •

edited

Loading

pvrs12 commented May 16, 2020

pvrs12 commented May 16, 2020

hucario commented May 16, 2020 •

edited

Loading

pvrs12 commented May 16, 2020

hucario commented May 16, 2020

hucario commented May 16, 2020 •

edited

Loading

pvrs12 commented May 16, 2020 •

edited

Loading

hucario commented May 16, 2020 •

edited

Loading

hucario commented May 16, 2020

pvrs12 commented May 16, 2020 •

edited

Loading

pvrs12 commented May 17, 2020

hucario commented May 17, 2020 •

edited

Loading

hucario commented May 17, 2020

pvrs12 commented May 17, 2020

hucario commented May 17, 2020

pvrs12 commented May 17, 2020 via email

hucario commented May 17, 2020 •

edited

Loading

hucario commented May 17, 2020 •

edited

Loading

hucario commented May 17, 2020 •

edited

Loading

pvrs12 commented May 18, 2020

pvrs12 commented May 19, 2020

New player and theming #54

New player and theming #54

Conversation

hucario commented May 16, 2020 • edited Loading

pvrs12 commented May 16, 2020

pvrs12 commented May 16, 2020

hucario commented May 16, 2020 • edited Loading

pvrs12 commented May 16, 2020

hucario commented May 16, 2020

hucario commented May 16, 2020 • edited Loading

pvrs12 commented May 16, 2020 • edited Loading

hucario commented May 16, 2020 • edited Loading

hucario commented May 16, 2020

pvrs12 commented May 16, 2020 • edited Loading

pvrs12 commented May 17, 2020

hucario commented May 17, 2020 • edited Loading

hucario commented May 17, 2020

pvrs12 commented May 17, 2020

hucario commented May 17, 2020

pvrs12 commented May 17, 2020 via email

hucario commented May 17, 2020 • edited Loading

hucario commented May 17, 2020 • edited Loading

hucario commented May 17, 2020 • edited Loading

pvrs12 commented May 18, 2020

pvrs12 commented May 19, 2020

hucario commented May 16, 2020 •

edited

Loading

hucario commented May 16, 2020 •

edited

Loading

hucario commented May 16, 2020 •

edited

Loading

pvrs12 commented May 16, 2020 •

edited

Loading

hucario commented May 16, 2020 •

edited

Loading

pvrs12 commented May 16, 2020 •

edited

Loading

hucario commented May 17, 2020 •

edited

Loading

hucario commented May 17, 2020 •

edited

Loading

hucario commented May 17, 2020 •

edited

Loading

hucario commented May 17, 2020 •

edited

Loading