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

Add Hypemachine support #208

Merged
merged 4 commits into from
Feb 27, 2018
Merged

Conversation

bayang
Copy link
Contributor

@bayang bayang commented Feb 23, 2018

add Hypemachine plugin.

Hopefully this one will work seamlessly :)

@ColinDuquesnoy ColinDuquesnoy self-requested a review February 25, 2018 17:53
@ColinDuquesnoy ColinDuquesnoy self-assigned this Feb 25, 2018
@ColinDuquesnoy ColinDuquesnoy added this to the 3.4.0 milestone Feb 25, 2018
Copy link
Owner

@ColinDuquesnoy ColinDuquesnoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good. There are a few indentation issues and some improvements could be done.

Do you think you can tweak logo.svg to match the style of all the other services logo?

// not supported
}

function readTime(elementClassName) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MellowPlayer provide you with an utility function toSeconds that do exatcly what readTime does (see http://mellowplayer.readthedocs.io/en/latest/developers/plugins.html#utility-functions). Please use it instead.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about this comment?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that I've had a look at your commit it looks good.

return hasClass(document.getElementById("playerFav"), "fav-on");
}

function hasNext() {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function is useless, better set the property to true directly. Unless you planned to implement detection of next/previous songs?

return true;
}

function hasClass(elem, className) {
Copy link
Owner

@ColinDuquesnoy ColinDuquesnoy Feb 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation in this function looks broken

@bayang
Copy link
Contributor Author

bayang commented Feb 26, 2018

I made a new icon but i'm having issues with the svg. It renders perfectly fine in all the apps I opened it into except in Mellowplayer, it looks stretched... It should look like this :
capture-svg

@ColinDuquesnoy
Copy link
Owner

Thank you for looking in the logo. I think you have this issue because your canvas is not square. I'd suggest you start from the empty template

@bayang
Copy link
Contributor Author

bayang commented Feb 27, 2018

you were right, I think it's ok now

@ColinDuquesnoy
Copy link
Owner

Thank you. I'll have a look ASAP

@ColinDuquesnoy
Copy link
Owner

The icon looks good!

@ColinDuquesnoy ColinDuquesnoy merged commit a640bfc into ColinDuquesnoy:develop Feb 27, 2018
@ColinDuquesnoy ColinDuquesnoy changed the title add Hypemachine Add Hypemachine support May 8, 2018
@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