Skip to content
This repository has been archived by the owner on Mar 30, 2021. It is now read-only.

New theme: “Rdiant” #444

Merged
merged 1 commit into from
Dec 14, 2015
Merged

New theme: “Rdiant” #444

merged 1 commit into from
Dec 14, 2015

Conversation

andrewnorell
Copy link
Contributor

After the loss of a certain streaming service, I tried out Google Play Music and came across Radiant Player. I really enjoyed this desktop application, and I wanted to create a new theme for it, called "Rdiant."

The theme is comprised mainly of CSS changes; mostly color, typeface, and font-weight. If the user has the font "Whitney" installed on their computer, it will use that, but otherwise it will fall back to Google's free-to-use "Open Sans" (which is being pulled in by the theme's CSS).

Another significant change the theme makes is that it replaces the application's icon file (only if the theme is enabled). Here’s what the theme's icon looks like:
rdiant-icon

Here's a screenshot of the theme:
rdiant

I've enjoyed this theme, and I hope others will too. Thank you for your consideration!

@chrismou
Copy link
Member

Hey @andrewnorell - screenshots look good. I'll pull down a copy and check it out at some point tomorrow.

One thing though - could you merge the 2 commits into one? Helps keep everything in one place. If you're not familiar with rebase, some good instructions are here: http://gitready.com/advanced/2009/02/10/squashing-commits-with-rebase.html and you should be able to force push the rebase back to github with git push origin +style/rdiant. The PR should update automatically. Thanks!

@chrismou
Copy link
Member

@radiant-player/radiant-player-mac Interested in you guys opinions on the icon. It does look cool, but the branding obsessive in me feels like it could get a bit confusing for users having the icon change with the theme.

Only other thing is the theme name. Might be worth having something that's a bit more descriptive of the colour scheme. I know names like "light blue" are boring, but with it so you have to restart the app to change theme, if you have a load of themes with non-descriptive names it could get tiring switching through them all trying to work out what each one is.

TBH, I don't think it would be an issue if we could get it to live change the theme (rather than requiring a restart) but until then it might be best to stick with something more obvious.

@andrewnorell
Copy link
Contributor Author

Thanks for the feedback @chrismou! Sorry about the two separate commits — I'll merge them into one.

Also, good point about the theme name and icon. I totally understand the concerns, and am open to the feedback. As for the theme name, it was an attempt at a nod to another, departing, streaming service that people may be familiar with; could we opt for a name that combines both? Such as "Rdiant (Light Blue)"?

@chrismou
Copy link
Member

@andrewnorell No problem! To be honest, I did see the description "An homage to the late, great streaming service" and wondered what it was referring to. I didn't realise Pandora has acquired Rdio till now. You learn a new thing every day 😉

@andrewnorell
Copy link
Contributor Author

@chrismou Yeah, Rdio is shutting down, and I imagine many other of the former patrons are currently seeking out a new streaming option — that’s what lead me to Google Play Music and Radiant Player.

Thanks for the primer on rebasing! I just squashed the two commits into one. :)

@ebramanti
Copy link
Contributor

@chrismou I think the name is fine. I think we should not change the icon for the app, however. Even apps like Atom that have icon changes are separate modifications that users can perform, and I think we should stick with that.

It is unfortunate that Rdio is shutting down, they had a good product and a great discount for students. Will definitely be missed.

Naming-wise, LGTM. 👍

Edit: Code-wise looks good too. @chrismou I think after removing the icon we should 🚢 ❗

@andrewnorell
Copy link
Contributor Author

Thanks for weighing in, @jadengore! I’ll go ahead and remove the custom app icon from the PR. (I’ll maybe just replace it manually on my machine’s install, to live in nostalgia for awhile. 😉)

Perhaps later on I can add an update to the theme and find a more subtle and appropriate placement of the image within the theme itself.

@ebramanti
Copy link
Contributor

@andrewnorell you did a great job on the theme!

@jacobwgillespie
Copy link
Member

As soon as this gets its commits squashed, it's got a LGTM :shipit: from me - nice work!

@andrewnorell
Copy link
Contributor Author

Thanks guys! Even though I've been a developer for quite a few years, this is my first contribution to an open-source project — I appreciate the feedback and the help. I removed the icon from the app, and the commits should all be squashed down to one now.

ebramanti added a commit that referenced this pull request Dec 14, 2015
@ebramanti ebramanti merged commit 1de90d4 into radiant-player:master Dec 14, 2015
@ebramanti
Copy link
Contributor

Thanks @andrewnorell! Your work is appreciated, and we all welcome you to the open-source community!

@andrewnorell andrewnorell deleted the style/rdiant branch December 15, 2015 20:56
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants