Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added Hungama.js for support of hungama.com #667

Merged
merged 4 commits into from Jun 4, 2017

Conversation

tanmaysachan
Copy link
Contributor

No description provided.

Support for hungama.com
…ma.com

Adding new file to MediaStrategies
@Coder-256
Copy link
Contributor

Don't forget to update versions.plist, see the Developers' Guide

@tanmaysachan
Copy link
Contributor Author

Sorry! I forgot to see that.

return {
'track': getElementsByClassName("jw-songName")[0].innerText,
'album': getElementsByClassName("jw-songAlbum")[0].innerText,
'image': getElementsbyClassName("jw-albumThumb")[0].src
Copy link
Contributor

Choose a reason for hiding this comment

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

Change these to:

  • document.querySelector(".jw-songName").innerText
  • document.querySelector(".jw-songAlbum").innerText
  • document.querySelector(".jw-albumThumb").src

You have to include document. before each of those functions, querySelector(".class") is faster than document.getElementsByClassName("class")[0], and also these are capitalized correctly (the "b" in getElementsbyClassName was not capitalized for the image).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I can't believe how I missed that, this is my first time, so stuff is going all over the place..
Thanks for being helpful!

Copy link
Contributor

@Coder-256 Coder-256 left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@1ps0
Copy link
Member

1ps0 commented Jun 4, 2017

👍

@1ps0 1ps0 merged commit 6d9fbc6 into beardedspice:master Jun 4, 2017
1ps0 pushed a commit that referenced this pull request Jun 4, 2017
@Stillness-2
Copy link
Member

What about readme.md? Why nobody don't add new strategy in description? :)

@Coder-256
Copy link
Contributor

Coder-256 commented Jun 4, 2017

Yes, I forgot about that. Also beardedspice.github.io, but that must be done in a different pull request (maybe it should be retrieved automatically from this repo via javascript?)

Coder-256 added a commit to Coder-256/beardedspice that referenced this pull request Jun 5, 2017
Reminds strategy makers to add their strategy to the list in `README.md`. See the [discussion on beardedspice#667](beardedspice#667 (comment)). This does not include updating `beardedspice.github.io` in [this list](https://github.com/beardedspice/beardedspice.github.io/blob/77f870b0e30f6392c2c5467319c32050f82b4740/index.html#L72) because that would require a separate PR for each strategy. I cannot think of a good solution to this other than having a bot update that list (or by using Javascript on the website).
@Coder-256 Coder-256 mentioned this pull request Jun 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants