Skip to content
This repository has been archived by the owner on May 3, 2019. It is now read-only.

add bandcamp plugin #200

Merged
merged 2 commits into from
Feb 18, 2018
Merged

add bandcamp plugin #200

merged 2 commits into from
Feb 18, 2018

Conversation

bayang
Copy link
Contributor

@bayang bayang commented Feb 12, 2018

Hi,

here is a proposition for a bandcamp plugin.
Don't know if people are interested in it but I am already using it so...

@ColinDuquesnoy
Copy link
Owner

ColinDuquesnoy commented Feb 12, 2018

That looks good 👍 It might be useful for other users.

If I merge your PR you would have to maintain the plugin if it ever breaks, otherwise it will be removed (unless someone else steps in). Do you agree?

ColinDuquesnoy
ColinDuquesnoy previously approved these changes Feb 12, 2018
@bayang
Copy link
Contributor Author

bayang commented Feb 12, 2018

It's ok, I'll maintain it.
But just remind I only tested on Ubuntu though :)

@ColinDuquesnoy
Copy link
Owner

But just remind I only tested on Ubuntu though :)

It should work anywhere else then. I will try your plugin before I merge if the service offers free subscription and does not have localisation restrictions.

@ColinDuquesnoy ColinDuquesnoy mentioned this pull request Feb 13, 2018
@ColinDuquesnoy
Copy link
Owner

The plugin does not work for me or is incomplete. A plugin does not have to implement all features (some services are more restrictive than other) but information about current track is mandatory (at the very least the song title)

There is no track information here:

screenshot_20180213_181614

I see you have code to retrieve song title and so on. Maybe it works only in some specific areas of bandcamp?

Also it would be nice to customize theme.json to fit bandcamp branding colors.

@ColinDuquesnoy ColinDuquesnoy dismissed their stale review February 13, 2018 17:20

Current track information are missing

@bayang
Copy link
Contributor Author

bayang commented Feb 14, 2018

you are right, I never use the home page so I haven't noticed it. I'll have a look.

@bayang
Copy link
Contributor Author

bayang commented Feb 16, 2018

ok @ColinDuquesnoy I updated it, the home page and feed page are working.
The only unsupported page is the so called "daily" page because the player is inside an iframe on this page. So retrieving the dom inside it is not as easy.
If you get 5 minutes you can test it. :)

@ColinDuquesnoy
Copy link
Owner

@bayang Thanks. I'll have a look during the weekend.

@ColinDuquesnoy ColinDuquesnoy merged commit 27c1424 into ColinDuquesnoy:develop Feb 18, 2018
@ColinDuquesnoy
Copy link
Owner

Thank you! It works nice. I noticed a few small issues on the discover page but I quickly found a fix that I have applied in commit d2f8a68

@cpjeanpaul cpjeanpaul mentioned this pull request Jul 10, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants