search

msfeldstein / chrome-media-keys

Chrome extension that adds media keys to most web music players
GNU Affero General Public License v3.0
89 stars 33 forks source link

Simplified init routine in content.coffee #105

Closed joepvl closed 7 years ago

joepvl commented 7 years ago

Fixes #107.

What

  1. Merged the two conditionals
  2. Renamed init to initIfReady
  3. Changed call to run immediately

Why

  1. In addition to trying again if controller.init returns false, we also want to try again if controller is undefined, because it should eventually always be defined. I ran into this while adding support for beta.mixcloud.com on a relatively slow machine - it wouldn't consistently have the controller code injected and executed within that first 500ms window.
  2. initIfReady describes the functionality more accurately
  3. No unnecessary repetition of the setTimeout call

Notes

I am not a regular coffeescript user, but I'm pretty sure much of the coffeescript in this file is not exactly idiomatic (I tried to keep the bits that I added in this PR as much in line with the rest of the file as possible while maintaining readability). That's fine, but since the controllers are all written in normal JS anyway, and some work has been done to convert things to ES2015, perhaps it would be a good idea to start converting all of the code, getting rid of the coffeescript dependency in the process (and we may as well dump Haml as well at that point).

Comments & suggestions welcome, of course! 😄

msfeldstein commented 7 years ago

Good catch, and yes i'm keen on getting rid of coffeescript and haml too, i haven't used those in years and the surface on this project is small enough that it should be fairly easy