search

arnoappenzeller / PiPifier

PiPifier is a native macOS 10.12 Safari extension that lets you use every HTML5 video in Picture in Picture mode
MIT License
761 stars 60 forks source link

Cleaned up the code and made the toolbar button intelligently enable/disable #11

Closed kabiroberai closed 8 years ago

kabiroberai commented 8 years ago

The title (and commits) say it all. I've removed a lot of things that I didn't think were needed, but if they are supposed to be for future features, feel free to select what to merge.

kabiroberai commented 8 years ago

There also seem to be a few bugs with the toolbar enabling/disabling thing, and I think that activate isn't always being called on tab change. This may be worth looking into.

kabiroberai commented 8 years ago

Also, this doesn't seem to be working with Netflix, since the video tag there seems to be loaded dynamically (it may require a Mutation Observer).

arnoappenzeller commented 8 years ago

I'm going to look into it in the next few days but already thanks in advance - sounds fantastic.

Netflix worked for me a few weeks ago but maybe they did some changes in their code

arnoappenzeller commented 8 years ago

The cleanup is very welcome and you finally made the activation feature work.

Didn't think about the Mutation Observer - this was where I was stuck in the first place so I'm merging this!

Will also add the additions for the AppStore version soon

Thanks so much!

kabiroberai commented 8 years ago

@arnoappenzeller no problem 👍 Also, I've made another commit in my fork that removes both the local reference to stateManager (now there's no self capturing in validateToolbarItem's getActivePage), and the firstCheck variable in script.js (instead, previousResult is now null during the first loop). You could manually do those here or I can make another PR if you want.

arnoappenzeller commented 8 years ago

Feel free to add another PR :-)

Otherwise I'm currently working on a new feature (adds buttons directly to popular players) which I will push later or next weekish then I can include those changes

arnoappenzeller commented 8 years ago

I added some of your changes in 2d783e04d24d9e6e28d7de48bd65b650c0bbfb54

I didn't add the changes to the js because after a while the button didn't activate/deactivate properly in my tests.

kabiroberai commented 8 years ago

@arnoappenzeller I figured out the issue (it happens even with the old logic), so expect a PR soon :)