-
Notifications
You must be signed in to change notification settings - Fork 13
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
Separate into its own module. #3
Comments
Which testing suite do you want to use? |
That's a really good question. I don't have too much experience with any testing tool specifically, so I'm all ears. 👂 Open to any of these. I'm feeling QUnit because it handles our use-case pretty well, what about you? |
Okay, I'll take a look into this testing framework. At the moment I'm separating the player in its own file. And I have just following question: Should we remove jQuery completely? Everything we do with jQuery is easy with VanillaJS. Or would you like to keep it? |
Yes that's actually a great idea. As far as I can tell you are correct. Everything will be possible via vanilla JS. 😄 |
@shahzeb1 I just want to make sure I'm clear on the ultimate goal of this project - are you aiming to have a set of javascript functions that would allow someone to easily create their own SoundCloud players? I.e. is your UI here just to showcase an example of what your javascript functionality allows? I'm wondering what assumptions could be made about how this would be used, for example will there have to be an iframe with id="so"? |
@PaulTFreedman yes so the goal is to separate all the Sound Cloud related logic into it's own library (file). This will make testing these specific functions easier. Anything related to the iFrame / Soundcloud should be placed in the new |
@PaulTFreedman Feel free to looking at this 👍🏼 |
I've made an attempt at the separation on my fork but haven't written any tests or updated the README yet. I'm not sure when I'll next have time to look at them so I'd be OK with someone taking over that part. |
@PaulTFreedman hey playa, feel free to make a fork request with the work you currently have. We'll forsure look it over and see what's going on / how we can help you with the tests! 🙌 |
I see @PaulTFreedman has separated the SoundCloud logic and UI logic. But this dumps everything in the global namespace. I would like to add some code that detects the current operating environment and exports the functions appropriately (Node.JS, RequireJS etc). |
@Vortex83 sounds good to me. I have started looking at writing unit tests using QUnit and Sinon but I can wait until you finish with that refactoring. |
So right now mySC.js includes both the code for (a) interacting with SoundCloud (getting info / playing actual song) and (b) interacting with our UI (managing the play / pause button getting clicked.
(a) and (b) should be split up. (a) should be placed into its own
soundcloud-player.js
module. It should include the key functions which should allow for anyone to include the<script>
and grab info / play music.mySC.js
andsoundcloud-player.js
into two distinct filessoundcloud-player.js
includes all the soundcloud related functionssoundcloud-player.js
soundcloud-player.js
module.The text was updated successfully, but these errors were encountered: