thieman / github-selfies

Everything is better with selfies. Yes, even pull requests. Trust me.
MIT License
1.14k stars 76 forks source link

Revamp the github selfies extension #69

Closed bhollis closed 8 years ago

bhollis commented 8 years ago

I got super carried away with working on the extension, and this is the result.

First, I want to apologize - as part of messing around with everything, I rewrote a lot of the UI code to build objects to handle the state - as a result, the code is full of this junk. Sorry :crying_cat_face:. I tried to reduce the impact by switching to arrow functions where possible.

On to the good stuff. First, a demo: http://imgur.com/RBB6ZcZ

There are a bunch of things going on in these changes:

  1. There's now a build script at build.sh that'll build both the Chrome and Firefox extension.
  2. The Firefox extension works again. It now takes all of its code as symlinks into the Chrome extension, so fixes to one will apply to the other. No more copy/paste. I also fixed the CSP problems with the Firefox version.
  3. The extension uses one JS and one CSS file for all pages, so there's no duplication. There's also no special-case logic per-page - the same selectors work everywhere.
  4. GitHub's pushState based navigation is properly observed, and the extension correctly attaches the selfie button as you navigate around. No more having to refresh the page. I also prevent the extension from accidentally multiplying by making sure there's only ever one selfie button on the page.
  5. The selfie button is now a toggle to show and hide the selfie controls and preview. The preview uses flexbox to display as large as possible without messing with the existing buttons (it takes the full width of the blank space to the left of the buttons. Photo/Video are now toggle buttons. This makes it easy to choose when you do and don't want to have your camera running.
  6. The preview is still properly mirrored, but the output is now properly reversed again, so words on t-shirts and signs will be readable.
  7. I used the page visibility API to stop and start the camera stream on visibility events, so the camera shuts off when you switch tabs, hide the browser, start the screensaver, switch to a different space, etc. This both saves battery and reduces the creepy factor of having your camera going all the time.
  8. I couldn't help but add a "camera flash" effect for when you take a photo.
  9. The image is now linked to this repository, for self-promotion.
  10. Rather presumptuously, I bumped the version to 2.0.0 and added myself to the "Awesome People" table in the README.

If you like this and decide to merge it, please don't ship the extension - I have more changes coming, but I figured I'd get this out there first.

This should fix most of the open issues in this repo.

selfie-2

bhollis commented 8 years ago

OK, here's the rest of the changes:

  1. Fixed a bug in my previous changes where the video was recording too fast.
  2. Optimize GIF recording quite a bit by moving most work out of the frame capture callback, removing some unnecessary work, switching to requestAnimationFrame for video sync, and splitting work up into a Promise chain so the UI doesn't freeze as much while encoding the GIF.
  3. As a result of #2, I changed the GIF default to 15fps (up from 10) and locked 320x240 resolution video (vs "1/3 original size", which was 213x160 on my machine, but could be 640x360 for folks with a 1080p camera).
  4. Added a dropdown to choose video length from 1 to 4 seconds (default 2).
  5. Save preferences - video vs. stills, and video length, so you don't have to choose them each time. This should resolve #40.
  6. Change the text placeholder to [[selfie-0 uploading...]] so it's more like a progress indicator.

selfie-0

thieman commented 8 years ago

Rather presumptuously, I bumped the version to 2.0.0 and added myself to the "Awesome People" table in the README.

PRs like this are exactly why we have that table. :smile:

This should fix most of the open issues in this repo.

Literally laughed out loud at this, this is amazing.

I'm doing the whole Christmas vacation thing so I might not be able to review this immediately. I'll probably just do a usability review since your JS abilities seem to be pretty head and shoulders above mine. Will get this out by the new year for sure.

bhollis commented 8 years ago

Thanks! :blush:

No hurry merging this - I'm excited to get my co-workers using it in the new year, though!

thieman commented 8 years ago

If I go to the "Files Changed" sub-tab in the PR view and come back, the Selfie button is gone. However, if I go to "Commits" and come back, it's still here. That's odd.

thieman commented 8 years ago

Looks good other than those two things :+1:

bhollis commented 8 years ago

I'll fix it!! šŸ˜¤

thieman commented 8 years ago

I'll QA it!

bhollis commented 8 years ago

OK, those bugs should be fixed. Turns out they were using replaceState for the tabs instead of pushState. I also added two more enhancements:

  1. The duration dropdown is only shown in video mode.
  2. Wait until the upload starts to add the text placeholder.

selfie-1

thieman commented 8 years ago

thieman commented 8 years ago

oh man this new build script is so luxurious

thieman commented 8 years ago

Chrome version should be live within 60 minutes, Firefox version is apparently in some sort of review queue

bhollis commented 8 years ago

Awesome, thank you for your help!