Browser extension for posting screenshots to Review Board

Review Request #7741 — Created Oct. 27, 2015 and discarded

jwu
rb-extension-pack
master
rb-extension-pack

Browser extension for Firefox and Chrome that allows the user to take screenshots of their browser's web content and post it to a Review Board server.

The extension adds the Review Board logo as an icon to the browser's toolbar. Clicking on the icon allows you to select different options of capturing web content. Currently you can captural all visible web content and crop a portion of the visible web content.

Chrome

  • Can take screenshot of all visible web content
  • Can crop a section of visible web content
  • Can send screenshot to a Review Board user's review request
  • Can add/delete/modify users from the user.html page

Firefox

  • Can take screenshot of all visible web content
  • Can crop a section of visible web content
  • Can send screenshot to a Review Board user's review request
  • Can add/delete/modify users from the user.html page
  • Multiprocessing friendly

TO DO LIST

Asana Link

  • Basic Validation on table cell editing
  • Fix infinite loading when requesting to some servers
  • Fill in all table cells alert message
  • Make Crop option overlay current page and open new tab after cropping (Chrome)
  • Organize javascript folder into import libraries and non
  • Alert user if user.html page has unsaved information
  • Sent a screenshot to a review request on a local server (Firefox)
  • Sent a screenshot to a review request on the Review Board server (Firefox)
  • Tested addon in nightly build with multiprocessing enabled (Firefox)
  • Sent a screenshot to a review request on a local server (Chrome)
  • Sent a screenshot to a review request on the Review Board server (Chrome)
Loading file attachments...

Description From Last Updated

It's possible that you'll be one of several people who eventually contribute to this project. Might be best to say: ...

mike_conleymike_conley

You can pull this out :P

brenniebrennie

Please put a space after : for each of these.

mike_conleymike_conley

Busted indentation.

mike_conleymike_conley

Yeah, pretty sure you can get away with just a 90% height and width on the #screenshot element instead of ...

mike_conleymike_conley

Can you explain this 0.9 thing?

mike_conleymike_conley

We should definitely do something more explicit to alert the user that the upload succeeded. Might also be worthwhile to ...

mike_conleymike_conley

I think this can be simplified to: return $("#rr-select").val();

mike_conleymike_conley

Alternatively, instead of generic <div>'s, you could use the new <section> element that is part of HTML5.

mike_conleymike_conley

For <button>'s, you don't need type="button".

mike_conleymike_conley

Nit - space before = Also, it's usually best to use <label>'s for these things. Example: <label for="account-select">Account:</label> <select id="account-select"></select>

mike_conleymike_conley

Again, no need for type="button".

mike_conleymike_conley

Nit - single quotes.

mike_conleymike_conley

Mixed tabs and spaces.

brenniebrennie

This can't be shared?

mike_conleymike_conley

Can you reindent your work with 4 spaces instead of tabs?

brenniebrennie

I believe it's gulp style to namespace tasks with colons, e.g. firefox:js.

brenniebrennie

Put this on previous line.

brenniebrennie

Probably not necessary.

mike_conleymike_conley

This is, again, using synchronous access to the content to do something. We'll need to update this at some point ...

mike_conleymike_conley

All of this, and anything that manipulates content directly like this, should probably go into an additional contentScript that sends ...

mike_conleymike_conley

Wonky indentation.

brenniebrennie

Pretty certain this can be shared, no?

mike_conleymike_conley

Can you have this file use 4 spaces instead of tabs.

brenniebrennie

Can we just include the minified source?

brenniebrennie

Single space indentation for HTML.

brenniebrennie

Missing <meta charset="utf-8"> Also, indentation.

brenniebrennie

Likewise.

brenniebrennie

This should be the first element in <head/>

brenniebrennie

Sorry to be pedantic, but I thought we were going to go with 4 spaces?

mike_conleymike_conley

4 space indentation in here please

mike_conleymike_conley

Leftover tab?

mike_conleymike_conley

What if there's no tabindex? Do we even really need a tabindex in this case?

mike_conleymike_conley

We probably shouldn't alert. Logging to the console is fine, but alerting is usually a bad idea.

mike_conleymike_conley

4 space indentation please

mike_conleymike_conley

Is this file still being used?

mike_conleymike_conley

Why? Is this leftovers? Why are we using both?

mike_conleymike_conley

I think I'd prefer: if (users && users.length && users.length != table.rows.length && !modified)

mike_conleymike_conley

Is this still being used?

mike_conleymike_conley

Each of these functions should have a quick bit of documentation above to describe what exactly they do, what parameters ...

mike_conleymike_conley

I'm curious why we're sticking with a numerical ID... doesn't that make it (cognitively) more difficult to map it to ...

mike_conleymike_conley

space after if

mike_conleymike_conley

Best to use textContent here instead of innerHTML.

mike_conleymike_conley

This can be simplified to: option.value = serverDropdown.options.length; Even better though is to set the value to the URL itself? ...

mike_conleymike_conley

Space before (

mike_conleymike_conley

Space before (

mike_conleymike_conley

Might as well simplify this: var allFields = $([apiKey, username, server]);

mike_conleymike_conley

This doesn't appear to be used.

mike_conleymike_conley

This should be indented 1 space. Also, replace "Reviewboard" with "Review Board"

mike_conleymike_conley

Instead of adding an event listener to every anchor element, you should add an event listener on a parent element ...

mike_conleymike_conley

Bit strange to have a switch for just one option. Should probably just be an if condition.

mike_conleymike_conley

"review board" -> "Review Board"

mike_conleymike_conley

What calls this?

mike_conleymike_conley

This is not going to work with multi-process Firefox, as you'll have no access to the contentWindow.

mike_conleymike_conley

Same as above, re: adding an event listener to a parent and inspecting the event target id.

mike_conleymike_conley

"review board" -> "Review Board". Maybe take a TODO down to have all of this project metadata come from one ...

mike_conleymike_conley
reviewbot
  1. Tool: Pyflakes
    Ignored Files:
        browser_screenshot/content/images/icons/icon64.png
        browser_screenshot/content/css/jquery-ui.css
        browser_screenshot/xpi/package.json
        browser_screenshot/xpi/js/save_user.js
        browser_screenshot/content/js/jquery-ui.min.js
        browser_screenshot/xpi/popup.html
        browser_screenshot/crx/js/save_user.js
        browser_screenshot/content/js/jquery.Jcrop.min.js
        browser_screenshot/content/js/jquery-2.1.4.min.js
        browser_screenshot/content/images/icons/icon16.png
        browser_screenshot/crx/popup.html
        browser_screenshot/content/images/ui-icons_222222_256x240.png
        browser_screenshot/content/images/icons/icon48.png
        browser_screenshot/content/screenshot.html
        browser_screenshot/AUTHORS.md
        browser_screenshot/content/js/screenshot.js
        browser_screenshot/crx/js/popup.js
        browser_screenshot/crx/js/background.js
        browser_screenshot/gulpfile.js
        browser_screenshot/content/images/icons/icon32.png
        browser_screenshot/content/images/icons/icon128.png
        browser_screenshot/xpi/chrome.manifest
        browser_screenshot/.eslintrc
        browser_screenshot/xpi/js/popup.js
        browser_screenshot/.gitignore
        browser_screenshot/crx/manifest.json
        browser_screenshot/README.md
        browser_screenshot/xpi/README.md
        browser_screenshot/LICENSE
        browser_screenshot/xpi/index.js
        browser_screenshot/CONTRIBUTING.md
        browser_screenshot/content/css/jquery.Jcrop.min.css
        browser_screenshot/content/images/filler.png
        browser_screenshot/content/css/popup.less
        browser_screenshot/content/js/user_form.js
        browser_screenshot/content/css/screenshot.less
    
    
    
    Tool: PEP8 Style Checker
    Ignored Files:
        browser_screenshot/content/images/icons/icon64.png
        browser_screenshot/content/css/jquery-ui.css
        browser_screenshot/xpi/package.json
        browser_screenshot/xpi/js/save_user.js
        browser_screenshot/content/js/jquery-ui.min.js
        browser_screenshot/xpi/popup.html
        browser_screenshot/crx/js/save_user.js
        browser_screenshot/content/js/jquery.Jcrop.min.js
        browser_screenshot/content/js/jquery-2.1.4.min.js
        browser_screenshot/content/images/icons/icon16.png
        browser_screenshot/crx/popup.html
        browser_screenshot/content/images/ui-icons_222222_256x240.png
        browser_screenshot/content/images/icons/icon48.png
        browser_screenshot/content/screenshot.html
        browser_screenshot/AUTHORS.md
        browser_screenshot/content/js/screenshot.js
        browser_screenshot/crx/js/popup.js
        browser_screenshot/crx/js/background.js
        browser_screenshot/gulpfile.js
        browser_screenshot/content/images/icons/icon32.png
        browser_screenshot/content/images/icons/icon128.png
        browser_screenshot/xpi/chrome.manifest
        browser_screenshot/.eslintrc
        browser_screenshot/xpi/js/popup.js
        browser_screenshot/.gitignore
        browser_screenshot/crx/manifest.json
        browser_screenshot/README.md
        browser_screenshot/xpi/README.md
        browser_screenshot/LICENSE
        browser_screenshot/xpi/index.js
        browser_screenshot/CONTRIBUTING.md
        browser_screenshot/content/css/jquery.Jcrop.min.css
        browser_screenshot/content/images/filler.png
        browser_screenshot/content/css/popup.less
        browser_screenshot/content/js/user_form.js
        browser_screenshot/content/css/screenshot.less
    
    
  2. 
      
brennie
  1. Can you attach the images you're including? They don't show up in the diff.

  2. browser_screenshot/gulpfile.js (Diff revision 1)
     
     

    Can you reindent your work with 4 spaces instead of tabs?

  3. 
      
JW
mike_conley
  1. This is a really great start, Justin!

    I'll have more comments soon, once I've had another few nights to look at this.

  2. browser_screenshot/LICENSE (Diff revision 1)
     
     

    It's possible that you'll be one of several people who eventually contribute to this project. Might be best to say:

    Copyright (c) 2015 The Authors

    and then put your name in the AUTHORS file.

    Also, we should start this with "The MIT License (MIT)"

  3. Please put a space after : for each of these.

  4. Busted indentation.

    1. Sorry, what do you mean by busted indentation?

    2. I hopefully made this clear in Slack the other day.

  5. browser_screenshot/content/js/screenshot.js (Diff revision 1)
     
     
     

    Can you explain this 0.9 thing?

    1. Since the screenshot will be the size of the browser's web content, it would be too tall to fit on the screenshot page. This resizes the image down to 90% of the original size so it can fit on the screen.

    2. Ah, I see. Might be better than to set the width and height of the #screenshot element to 90% in the CSS file.

    3. It also helped for the cropped image as well since if we resized the element to 90% in the CSS file then the image would be resized in the browser but the actual image size would still be the same. This caused the coordinates obtained from the crop of the (smaller) image to be different than the actual image.

      I would like to make it so the crop function doesn't open a new tab and just puts a crop overlay over the current webcontent, I believe this is how you envisioned it in the hackpad. This would solve the * 0.9 problem and then we could set the element to 90%.

  6. Alternatively, instead of generic <div>'s, you could use the new <section> element that is part of HTML5.

  7. For <button>'s, you don't need type="button".

  8. Nit - space before =

    Also, it's usually best to use <label>'s for these things. Example:

    <label for="account-select">Account:</label>
    <select id="account-select"></select>
    
  9. browser_screenshot/content/screenshot.html (Diff revision 1)
     
     
     

    Again, no need for type="button".

  10. Nit - single quotes.

  11. 
      
brennie
  1. 
      
  2. browser_screenshot/README.md (Diff revision 1)
     
     
     
     
     

    You can pull this out :P

  3. browser_screenshot/crx/js/popup.js (Diff revision 1)
     
     

    Mixed tabs and spaces.

  4. browser_screenshot/gulpfile.js (Diff revision 1)
     
     

    I believe it's gulp style to namespace tasks with colons, e.g. firefox:js.

  5. browser_screenshot/gulpfile.js (Diff revision 1)
     
     

    Put this on previous line.

  6. browser_screenshot/xpi/js/save_user.js (Diff revision 1)
     
     
     

    Wonky indentation.

  7. 
      
mike_conley
  1. 
      
  2. browser_screenshot/content/js/screenshot.js (Diff revision 1)
     
     
     
     
     

    Yeah, pretty sure you can get away with just a 90% height and width on the #screenshot element instead of actually resizing the image.

  3. We should definitely do something more explicit to alert the user that the upload succeeded.

    Might also be worthwhile to emit progress data somewhere with a progress bar while the upload occurs.

    1. There is currently a GIF loading image when AJAX requests are made and a "toast" is shown for alerting the user (as seen in the demo video).

  4. browser_screenshot/content/js/screenshot.js (Diff revision 1)
     
     
     
     
     

    I think this can be simplified to:

    return $("#rr-select").val();

  5. browser_screenshot/crx/popup.html (Diff revision 1)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     

    This can't be shared?

  6. browser_screenshot/xpi/index.js (Diff revision 1)
     
     

    So right away, I can tell this add-on is not going to be multi-process friendly out of the box, because this script (which runs in the parent process) is synchronously accessing the contentWindow for the selected browser.

    That's probably okay for now, but in the future, we're probably going to need to send a message down to the page content, and wait for the dataURL to come back from it.

  7. browser_screenshot/xpi/index.js (Diff revision 1)
     
     

    Probably not necessary.

  8. browser_screenshot/xpi/index.js (Diff revision 1)
     
     
     
     

    This is, again, using synchronous access to the content to do something. We'll need to update this at some point to be multiprocess friendly.

  9. browser_screenshot/xpi/index.js (Diff revision 1)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     

    All of this, and anything that manipulates content directly like this, should probably go into an additional contentScript that sends messages up to the parent to do things like persistence.

  10. browser_screenshot/xpi/popup.html (Diff revision 1)
     
     
     
     
     
     
     
     
     
     
     
     
     

    Pretty certain this can be shared, no?

    1. Previously, there was an error with having the same file as Firefox needs to set the popup.html script with panels.Panel() while Chrome needs to set the script in the actual html file. This caused a bug where some user information was being added twice in Firefox since the script was linked both in panels.Panel() and in popup.html. However, I just tried reproducing the bugs and I can't seem to reproduce them so I've merged the files for now.

  11. 
      
JW
reviewbot
  1. Tool: Pyflakes
    Ignored Files:
        browser_screenshot/content/images/icons/icon64.png
        browser_screenshot/content/font-awesome/scss/_rotated-flipped.scss
        browser_screenshot/content/css/jquery-ui.css
        browser_screenshot/content/font-awesome/less/font-awesome.less
        browser_screenshot/content/js/user_form.js
        browser_screenshot/content/font-awesome/scss/_core.scss
        browser_screenshot/content/font-awesome/less/fixed-width.less
        browser_screenshot/content/font-awesome/fonts/fontawesome-webfont.woff2
        browser_screenshot/xpi/package.json
        browser_screenshot/content/font-awesome/scss/_animated.scss
        browser_screenshot/content/images/filler.png
        browser_screenshot/content/js/users.js
        browser_screenshot/xpi/popup.html
        browser_screenshot/content/images/ajax-loader.gif
        browser_screenshot/crx/js/save_user.js
        browser_screenshot/content/font-awesome/fonts/fontawesome-webfont.svg
        browser_screenshot/content/font-awesome/less/variables.less
        browser_screenshot/crx/js/background.js
        browser_screenshot/content/font-awesome/scss/_path.scss
        browser_screenshot/content/font-awesome/less/bordered-pulled.less
        browser_screenshot/content/css/toastr.less
        browser_screenshot/content/js/jquery-2.1.4.min.js
        browser_screenshot/content/images/icons/icon16.png
        browser_screenshot/content/js/jquery.Jcrop.min.js
        browser_screenshot/content/font-awesome/fonts/FontAwesome.otf
        browser_screenshot/xpi/js/popup.js
        browser_screenshot/crx/popup.html
        browser_screenshot/crx/js/popup.js
        browser_screenshot/content/images/ui-icons_222222_256x240.png
        browser_screenshot/content/js/toastr.min.js
        browser_screenshot/content/images/icons/icon48.png
        browser_screenshot/content/popup.html
        browser_screenshot/AUTHORS.md
        browser_screenshot/content/font-awesome/scss/_mixins.scss
        browser_screenshot/content/font-awesome/scss/_variables.scss
        browser_screenshot/content/js/screenshot.js
        browser_screenshot/gulpfile.js
        browser_screenshot/content/font-awesome/less/animated.less
        browser_screenshot/content/font-awesome/scss/_fixed-width.scss
        browser_screenshot/xpi/js/save_user.js
        browser_screenshot/content/font-awesome/scss/_stacked.scss
        browser_screenshot/crx/js/all_content.js
        browser_screenshot/content/font-awesome/scss/font-awesome.scss
        browser_screenshot/content/screenshot.html
        browser_screenshot/content/font-awesome/less/mixins.less
        browser_screenshot/content/font-awesome/less/rotated-flipped.less
        browser_screenshot/content/images/icons/icon32.png
        browser_screenshot/xpi/index.js
        browser_screenshot/content/font-awesome/scss/_icons.scss
        browser_screenshot/crx/manifest.json
        browser_screenshot/content/images/icons/icon128.png
        browser_screenshot/content/font-awesome/less/larger.less
        browser_screenshot/content/font-awesome/scss/_bordered-pulled.scss
        browser_screenshot/.eslintrc
        browser_screenshot/xpi/chrome.manifest
        browser_screenshot/content/font-awesome/less/path.less
        browser_screenshot/content/users.html
        browser_screenshot/.gitignore
        browser_screenshot/content/font-awesome/less/list.less
        browser_screenshot/content/font-awesome/less/stacked.less
        browser_screenshot/content/font-awesome/fonts/fontawesome-webfont.ttf
        browser_screenshot/README.md
        browser_screenshot/content/font-awesome/less/core.less
        browser_screenshot/content/font-awesome/scss/_larger.scss
        browser_screenshot/xpi/README.md
        browser_screenshot/content/font-awesome/fonts/fontawesome-webfont.eot
        browser_screenshot/LICENSE
        browser_screenshot/content/font-awesome/less/icons.less
        browser_screenshot/content/font-awesome/scss/_list.scss
        browser_screenshot/content/font-awesome/fonts/fontawesome-webfont.woff
        browser_screenshot/content/font-awesome/css/font-awesome.css
        browser_screenshot/CONTRIBUTING.md
        browser_screenshot/crx/js/modify_users.js
        browser_screenshot/content/css/jquery.Jcrop.min.css
        browser_screenshot/content/font-awesome/css/font-awesome.min.css
        browser_screenshot/content/js/jquery-ui.min.js
        browser_screenshot/content/css/popup.less
        browser_screenshot/content/css/users.css
        browser_screenshot/content/css/screenshot.less
    
    
    
    Tool: PEP8 Style Checker
    Ignored Files:
        browser_screenshot/content/images/icons/icon64.png
        browser_screenshot/content/font-awesome/scss/_rotated-flipped.scss
        browser_screenshot/content/css/jquery-ui.css
        browser_screenshot/content/font-awesome/less/font-awesome.less
        browser_screenshot/content/js/user_form.js
        browser_screenshot/content/font-awesome/scss/_core.scss
        browser_screenshot/content/font-awesome/less/fixed-width.less
        browser_screenshot/content/font-awesome/fonts/fontawesome-webfont.woff2
        browser_screenshot/xpi/package.json
        browser_screenshot/content/font-awesome/scss/_animated.scss
        browser_screenshot/content/images/filler.png
        browser_screenshot/content/js/users.js
        browser_screenshot/xpi/popup.html
        browser_screenshot/content/images/ajax-loader.gif
        browser_screenshot/crx/js/save_user.js
        browser_screenshot/content/font-awesome/fonts/fontawesome-webfont.svg
        browser_screenshot/content/font-awesome/less/variables.less
        browser_screenshot/crx/js/background.js
        browser_screenshot/content/font-awesome/scss/_path.scss
        browser_screenshot/content/font-awesome/less/bordered-pulled.less
        browser_screenshot/content/css/toastr.less
        browser_screenshot/content/js/jquery-2.1.4.min.js
        browser_screenshot/content/images/icons/icon16.png
        browser_screenshot/content/js/jquery.Jcrop.min.js
        browser_screenshot/content/font-awesome/fonts/FontAwesome.otf
        browser_screenshot/xpi/js/popup.js
        browser_screenshot/crx/popup.html
        browser_screenshot/crx/js/popup.js
        browser_screenshot/content/images/ui-icons_222222_256x240.png
        browser_screenshot/content/js/toastr.min.js
        browser_screenshot/content/images/icons/icon48.png
        browser_screenshot/content/popup.html
        browser_screenshot/AUTHORS.md
        browser_screenshot/content/font-awesome/scss/_mixins.scss
        browser_screenshot/content/font-awesome/scss/_variables.scss
        browser_screenshot/content/js/screenshot.js
        browser_screenshot/gulpfile.js
        browser_screenshot/content/font-awesome/less/animated.less
        browser_screenshot/content/font-awesome/scss/_fixed-width.scss
        browser_screenshot/xpi/js/save_user.js
        browser_screenshot/content/font-awesome/scss/_stacked.scss
        browser_screenshot/crx/js/all_content.js
        browser_screenshot/content/font-awesome/scss/font-awesome.scss
        browser_screenshot/content/screenshot.html
        browser_screenshot/content/font-awesome/less/mixins.less
        browser_screenshot/content/font-awesome/less/rotated-flipped.less
        browser_screenshot/content/images/icons/icon32.png
        browser_screenshot/xpi/index.js
        browser_screenshot/content/font-awesome/scss/_icons.scss
        browser_screenshot/crx/manifest.json
        browser_screenshot/content/images/icons/icon128.png
        browser_screenshot/content/font-awesome/less/larger.less
        browser_screenshot/content/font-awesome/scss/_bordered-pulled.scss
        browser_screenshot/.eslintrc
        browser_screenshot/xpi/chrome.manifest
        browser_screenshot/content/font-awesome/less/path.less
        browser_screenshot/content/users.html
        browser_screenshot/.gitignore
        browser_screenshot/content/font-awesome/less/list.less
        browser_screenshot/content/font-awesome/less/stacked.less
        browser_screenshot/content/font-awesome/fonts/fontawesome-webfont.ttf
        browser_screenshot/README.md
        browser_screenshot/content/font-awesome/less/core.less
        browser_screenshot/content/font-awesome/scss/_larger.scss
        browser_screenshot/xpi/README.md
        browser_screenshot/content/font-awesome/fonts/fontawesome-webfont.eot
        browser_screenshot/LICENSE
        browser_screenshot/content/font-awesome/less/icons.less
        browser_screenshot/content/font-awesome/scss/_list.scss
        browser_screenshot/content/font-awesome/fonts/fontawesome-webfont.woff
        browser_screenshot/content/font-awesome/css/font-awesome.css
        browser_screenshot/CONTRIBUTING.md
        browser_screenshot/crx/js/modify_users.js
        browser_screenshot/content/css/jquery.Jcrop.min.css
        browser_screenshot/content/font-awesome/css/font-awesome.min.css
        browser_screenshot/content/js/jquery-ui.min.js
        browser_screenshot/content/css/popup.less
        browser_screenshot/content/css/users.css
        browser_screenshot/content/css/screenshot.less
    
    
  2. 
      
mike_conley
  1. Hey Justin,

    I think if we can, we should try to land what you've currently got into rb-extension-pack, and then continue building on top of it.

    So can you take this out of WIP and try polishing it up for landing? We can do your TODOs in follow-up review requests. Sound good?

    1. Sounds good. I'll work on polishing it up this week then :)

  2. 
      
brennie
  1. 
      
  2. browser_screenshot/.eslintrc (Diff revision 2)
     
     

    Can you have this file use 4 spaces instead of tabs.

  3. Can we just include the minified source?

  4. Single space indentation for HTML.

  5. Missing <meta charset="utf-8">
    Also, indentation.

  6. browser_screenshot/content/users.html (Diff revision 2)
     
     

    Likewise.

  7. browser_screenshot/content/users.html (Diff revision 2)
     
     

    This should be the first element in <head/>

  8. 
      
JW
JW
reviewbot
  1. Tool: PEP8 Style Checker
    Ignored Files:
        browser_screenshot/content/css/jquery-ui.css
        browser_screenshot/content/css/images/ui-icons_ef8c08_256x240.png
        browser_screenshot/content/font-awesome/less/font-awesome.less
        browser_screenshot/content/font-awesome/scss/_core.scss
        browser_screenshot/content/font-awesome/less/fixed-width.less
        browser_screenshot/crx/js/save_user.js
        browser_screenshot/content/js/jquery.Jcrop.min.js
        browser_screenshot/content/css/images/ui-bg_glass_65_ffffff_1x400.png
        browser_screenshot/content/fonts/fontawesome-webfont.woff
        browser_screenshot/content/images/icons/icon16.png
        browser_screenshot/crx/js/background.js
        browser_screenshot/crx/js/all_content.js
        browser_screenshot/content/css/users.less
        browser_screenshot/content/images/icons/icon32.png
        browser_screenshot/content/font-awesome/less/rotated-flipped.less
        browser_screenshot/.eslintrc
        browser_screenshot/.gitignore
        browser_screenshot/crx/manifest.json
        browser_screenshot/content/css/images/ui-bg_gloss-wave_35_f6a828_500x100.png
        browser_screenshot/content/css/images/ui-bg_glass_100_f6f6f6_1x400.png
        browser_screenshot/content/fonts/fontawesome-webfont.woff2
        browser_screenshot/content/css/jquery.Jcrop.min.css
        browser_screenshot/content/fonts/fontawesome-webfont.eot
        browser_screenshot/content/css/font-awesome.min.css
        browser_screenshot/content/fonts/FontAwesome.otf
        browser_screenshot/content/css/images/ui-icons_228ef1_256x240.png
        browser_screenshot/xpi/js/save_user.js
        browser_screenshot/content/js/users.js
        browser_screenshot/content/font-awesome/less/bordered-pulled.less
        browser_screenshot/content/font-awesome/fonts/fontawesome-webfont.svg
        browser_screenshot/content/font-awesome/fonts/FontAwesome.otf
        browser_screenshot/content/css/images/ui-icons_222222_256x240.png
        browser_screenshot/crx/popup.html
        browser_screenshot/content/js/toastr.min.js
        browser_screenshot/content/screenshot.html
        browser_screenshot/AUTHORS.md
        browser_screenshot/content/font-awesome/scss/_variables.scss
        browser_screenshot/content/font-awesome/less/animated.less
        browser_screenshot/content/css/images/ui-bg_highlight-soft_75_ffe45c_1x100.png
        browser_screenshot/content/images/icons/icon128.png
        browser_screenshot/content/fonts/fontawesome-webfont.ttf
        browser_screenshot/content/font-awesome/scss/_icons.scss
        browser_screenshot/content/users.html
        browser_screenshot/content/font-awesome/less/list.less
        browser_screenshot/content/font-awesome/fonts/fontawesome-webfont.ttf
        browser_screenshot/CONTRIBUTING.md
        browser_screenshot/content/font-awesome/less/core.less
        browser_screenshot/content/font-awesome/scss/_larger.scss
        browser_screenshot/content/font-awesome/scss/_list.scss
        browser_screenshot/xpi/index.js
        browser_screenshot/content/css/popup.less
        browser_screenshot/content/js/user_form.js
        browser_screenshot/content/images/icons/icon64.png
        browser_screenshot/content/font-awesome/fonts/fontawesome-webfont.woff2
        browser_screenshot/content/font-awesome/scss/_animated.scss
        browser_screenshot/content/images/filler.png
        browser_screenshot/content/css/images/ui-icons_ffffff_256x240.png
        browser_screenshot/xpi/popup.html
        browser_screenshot/content/css/images/ui-bg_highlight-soft_100_eeeeee_1x100.png
        browser_screenshot/content/images/ui-icons_222222_256x240.png
        browser_screenshot/content/font-awesome/less/path.less
        browser_screenshot/content/js/screenshot.js
        browser_screenshot/content/css/images/ui-bg_diagonals-thick_20_666666_40x40.png
        browser_screenshot/content/images/ajax-loader.gif
        browser_screenshot/content/font-awesome/scss/_stacked.scss
        browser_screenshot/gulpfile.js
        browser_screenshot/content/font-awesome/scss/font-awesome.scss
        browser_screenshot/content/font-awesome/less/mixins.less
        browser_screenshot/content/font-awesome/less/larger.less
        browser_screenshot/content/fonts/fontawesome-webfont.svg
        browser_screenshot/content/css/images/ui-bg_flat_10_000000_40x100.png
        browser_screenshot/README.md
        browser_screenshot/content/css/jquery-ui.min.css
        browser_screenshot/content/css/jquery-ui.min.js
        browser_screenshot/content/font-awesome/less/icons.less
        browser_screenshot/content/font-awesome/fonts/fontawesome-webfont.woff
        browser_screenshot/crx/js/modify_users.js
        browser_screenshot/crx/js/popup.js
        browser_screenshot/content/font-awesome/less/stacked.less
        browser_screenshot/content/css/screenshot.less
        browser_screenshot/content/font-awesome/scss/_rotated-flipped.scss
        browser_screenshot/content/js/jquery-2.1.4.min.js
        browser_screenshot/content/css/images/ui-icons_ffd27a_256x240.png
        browser_screenshot/LICENSE
        browser_screenshot/xpi/package.json
        browser_screenshot/content/js/jquery-ui.min.js
        browser_screenshot/content/font-awesome/less/variables.less
        browser_screenshot/content/font-awesome/scss/_path.scss
        browser_screenshot/content/css/toastr.less
        browser_screenshot/content/images/logo.png
        browser_screenshot/content/css/images/ui-bg_glass_100_fdf5ce_1x400.png
        browser_screenshot/content/images/icons/icon48.png
        browser_screenshot/content/font-awesome/scss/_fixed-width.scss
        browser_screenshot/content/popup.html
        browser_screenshot/xpi/chrome.manifest
        browser_screenshot/xpi/js/popup.js
        browser_screenshot/content/font-awesome/scss/_mixins.scss
        browser_screenshot/content/font-awesome/scss/_bordered-pulled.scss
        browser_screenshot/xpi/README.md
        browser_screenshot/content/font-awesome/fonts/fontawesome-webfont.eot
        browser_screenshot/xpi/js/modify_users.js
        browser_screenshot/content/font-awesome/css/font-awesome.css
        browser_screenshot/content/font-awesome/css/font-awesome.min.css
        browser_screenshot/content/css/users.css
        browser_screenshot/content/css/images/ui-bg_diagonals-thick_18_b81900_40x40.png
    
    
    
    Tool: Pyflakes
    Ignored Files:
        browser_screenshot/content/css/jquery-ui.css
        browser_screenshot/content/css/images/ui-icons_ef8c08_256x240.png
        browser_screenshot/content/font-awesome/less/font-awesome.less
        browser_screenshot/content/font-awesome/scss/_core.scss
        browser_screenshot/content/font-awesome/less/fixed-width.less
        browser_screenshot/crx/js/save_user.js
        browser_screenshot/content/js/jquery.Jcrop.min.js
        browser_screenshot/content/css/images/ui-bg_glass_65_ffffff_1x400.png
        browser_screenshot/content/fonts/fontawesome-webfont.woff
        browser_screenshot/content/images/icons/icon16.png
        browser_screenshot/crx/js/background.js
        browser_screenshot/crx/js/all_content.js
        browser_screenshot/content/css/users.less
        browser_screenshot/content/images/icons/icon32.png
        browser_screenshot/content/font-awesome/less/rotated-flipped.less
        browser_screenshot/.eslintrc
        browser_screenshot/.gitignore
        browser_screenshot/crx/manifest.json
        browser_screenshot/content/css/images/ui-bg_gloss-wave_35_f6a828_500x100.png
        browser_screenshot/content/css/images/ui-bg_glass_100_f6f6f6_1x400.png
        browser_screenshot/content/fonts/fontawesome-webfont.woff2
        browser_screenshot/content/css/jquery.Jcrop.min.css
        browser_screenshot/content/fonts/fontawesome-webfont.eot
        browser_screenshot/content/css/font-awesome.min.css
        browser_screenshot/content/fonts/FontAwesome.otf
        browser_screenshot/content/css/images/ui-icons_228ef1_256x240.png
        browser_screenshot/xpi/js/save_user.js
        browser_screenshot/content/js/users.js
        browser_screenshot/content/font-awesome/less/bordered-pulled.less
        browser_screenshot/content/font-awesome/fonts/fontawesome-webfont.svg
        browser_screenshot/content/font-awesome/fonts/FontAwesome.otf
        browser_screenshot/content/css/images/ui-icons_222222_256x240.png
        browser_screenshot/crx/popup.html
        browser_screenshot/content/js/toastr.min.js
        browser_screenshot/content/screenshot.html
        browser_screenshot/AUTHORS.md
        browser_screenshot/content/font-awesome/scss/_variables.scss
        browser_screenshot/content/font-awesome/less/animated.less
        browser_screenshot/content/css/images/ui-bg_highlight-soft_75_ffe45c_1x100.png
        browser_screenshot/content/images/icons/icon128.png
        browser_screenshot/content/fonts/fontawesome-webfont.ttf
        browser_screenshot/content/font-awesome/scss/_icons.scss
        browser_screenshot/content/users.html
        browser_screenshot/content/font-awesome/less/list.less
        browser_screenshot/content/font-awesome/fonts/fontawesome-webfont.ttf
        browser_screenshot/CONTRIBUTING.md
        browser_screenshot/content/font-awesome/less/core.less
        browser_screenshot/content/font-awesome/scss/_larger.scss
        browser_screenshot/content/font-awesome/scss/_list.scss
        browser_screenshot/xpi/index.js
        browser_screenshot/content/css/popup.less
        browser_screenshot/content/js/user_form.js
        browser_screenshot/content/images/icons/icon64.png
        browser_screenshot/content/font-awesome/fonts/fontawesome-webfont.woff2
        browser_screenshot/content/font-awesome/scss/_animated.scss
        browser_screenshot/content/images/filler.png
        browser_screenshot/content/css/images/ui-icons_ffffff_256x240.png
        browser_screenshot/xpi/popup.html
        browser_screenshot/content/css/images/ui-bg_highlight-soft_100_eeeeee_1x100.png
        browser_screenshot/content/images/ui-icons_222222_256x240.png
        browser_screenshot/content/font-awesome/less/path.less
        browser_screenshot/content/js/screenshot.js
        browser_screenshot/content/css/images/ui-bg_diagonals-thick_20_666666_40x40.png
        browser_screenshot/content/images/ajax-loader.gif
        browser_screenshot/content/font-awesome/scss/_stacked.scss
        browser_screenshot/gulpfile.js
        browser_screenshot/content/font-awesome/scss/font-awesome.scss
        browser_screenshot/content/font-awesome/less/mixins.less
        browser_screenshot/content/font-awesome/less/larger.less
        browser_screenshot/content/fonts/fontawesome-webfont.svg
        browser_screenshot/content/css/images/ui-bg_flat_10_000000_40x100.png
        browser_screenshot/README.md
        browser_screenshot/content/css/jquery-ui.min.css
        browser_screenshot/content/css/jquery-ui.min.js
        browser_screenshot/content/font-awesome/less/icons.less
        browser_screenshot/content/font-awesome/fonts/fontawesome-webfont.woff
        browser_screenshot/crx/js/modify_users.js
        browser_screenshot/crx/js/popup.js
        browser_screenshot/content/font-awesome/less/stacked.less
        browser_screenshot/content/css/screenshot.less
        browser_screenshot/content/font-awesome/scss/_rotated-flipped.scss
        browser_screenshot/content/js/jquery-2.1.4.min.js
        browser_screenshot/content/css/images/ui-icons_ffd27a_256x240.png
        browser_screenshot/LICENSE
        browser_screenshot/xpi/package.json
        browser_screenshot/content/js/jquery-ui.min.js
        browser_screenshot/content/font-awesome/less/variables.less
        browser_screenshot/content/font-awesome/scss/_path.scss
        browser_screenshot/content/css/toastr.less
        browser_screenshot/content/images/logo.png
        browser_screenshot/content/css/images/ui-bg_glass_100_fdf5ce_1x400.png
        browser_screenshot/content/images/icons/icon48.png
        browser_screenshot/content/font-awesome/scss/_fixed-width.scss
        browser_screenshot/content/popup.html
        browser_screenshot/xpi/chrome.manifest
        browser_screenshot/xpi/js/popup.js
        browser_screenshot/content/font-awesome/scss/_mixins.scss
        browser_screenshot/content/font-awesome/scss/_bordered-pulled.scss
        browser_screenshot/xpi/README.md
        browser_screenshot/content/font-awesome/fonts/fontawesome-webfont.eot
        browser_screenshot/xpi/js/modify_users.js
        browser_screenshot/content/font-awesome/css/font-awesome.css
        browser_screenshot/content/font-awesome/css/font-awesome.min.css
        browser_screenshot/content/css/users.css
        browser_screenshot/content/css/images/ui-bg_diagonals-thick_18_b81900_40x40.png
    
    
  2. 
      
mike_conley
  1. I'll look into this more deeply tomorrow, but for now, I'm wondering about this FontAwesome library that I see in here. It looks like you've brought in the entire FontAwesome source code... I was under the impression that you'd just need the font itself and the CSS? If so, can we get rid of the rest of the files?

    1. It seems I forgot to delete some extraneous files in my rb-extension-pack directory and copy over the hidden files. Sorry about that and please check the updated request.

      (Updated Request does not have users.css, and the extra popup.html files.

  2. browser_screenshot/.eslintrc (Diff revision 3)
     
     

    Sorry to be pedantic, but I thought we were going to go with 4 spaces?

  3. 4 space indentation in here please

  4. Leftover tab?

  5. browser_screenshot/content/popup.html (Diff revision 3)
     
     

    What if there's no tabindex? Do we even really need a tabindex in this case?

    1. If there is no tabindex, focus is automatically given to the first element without a tabindex.

  6. browser_screenshot/crx/js/background.js (Diff revision 3)
     
     

    We probably shouldn't alert. Logging to the console is fine, but alerting is usually a bad idea.

  7. browser_screenshot/crx/manifest.json (Diff revision 3)
     
     

    4 space indentation please

  8. browser_screenshot/crx/popup.html (Diff revision 3)
     
     

    Is this file still being used?

  9. browser_screenshot/gulpfile.js (Diff revision 3)
     
     

    Why? Is this leftovers? Why are we using both?

    1. The standalone imports in the first browserify task allows js/screenshot.js to have the standalone package screenshot, every time I've tried to combine the two tasks into one, screenshot.js loses its standalone pacakge.

      I haven't been able to figure a way to combine them so that's why both are being used.

  10. I think I'd prefer:

    if (users && users.length && users.length != table.rows.length && !modified)
    
  11. browser_screenshot/xpi/popup.html (Diff revision 3)
     
     

    Is this still being used?

  12. 
      
JW
reviewbot
  1. Tool: Pyflakes
    Ignored Files:
        browser_screenshot/content/images/icons/icon64.png
        browser_screenshot/content/css/font-awesome.min.css
        browser_screenshot/content/fonts/fontawesome-webfont.woff
        browser_screenshot/content/fonts/FontAwesome.otf
        browser_screenshot/content/css/images/ui-icons_ef8c08_256x240.png
        browser_screenshot/content/js/user_form.js
        browser_screenshot/content/css/jquery-ui.min.js
        browser_screenshot/xpi/package.json
        browser_screenshot/xpi/js/save_user.js
        browser_screenshot/content/images/filler.png
        browser_screenshot/content/css/images/ui-icons_ffffff_256x240.png
        browser_screenshot/content/js/users.js
        browser_screenshot/crx/js/save_user.js
        browser_screenshot/content/css/images/ui-bg_highlight-soft_100_eeeeee_1x100.png
        browser_screenshot/content/js/jquery.Jcrop.min.js
        browser_screenshot/content/css/images/ui-bg_glass_65_ffffff_1x400.png
        browser_screenshot/content/popup.html
        browser_screenshot/content/css/toastr.less
        browser_screenshot/content/js/jquery-2.1.4.min.js
        browser_screenshot/content/images/icons/icon16.png
        browser_screenshot/content/images/logo.png
        browser_screenshot/xpi/js/modify_users.js
        browser_screenshot/xpi/index.js
        browser_screenshot/content/css/images/ui-icons_222222_256x240.png
        browser_screenshot/content/css/images/ui-bg_glass_100_fdf5ce_1x400.png
        browser_screenshot/content/js/toastr.min.js
        browser_screenshot/content/images/icons/icon48.png
        browser_screenshot/content/css/images/ui-icons_ffd27a_256x240.png
        browser_screenshot/content/css/images/ui-icons_228ef1_256x240.png
        browser_screenshot/AUTHORS.md
        browser_screenshot/crx/js/modify_users.js
        browser_screenshot/content/js/screenshot.js
        browser_screenshot/content/css/images/ui-bg_diagonals-thick_20_666666_40x40.png
        browser_screenshot/crx/js/popup.js
        browser_screenshot/crx/js/background.js
        browser_screenshot/content/css/images/ui-bg_highlight-soft_75_ffe45c_1x100.png
        browser_screenshot/gulpfile.js
        browser_screenshot/content/screenshot.html
        browser_screenshot/content/css/users.less
        browser_screenshot/content/images/icons/icon32.png
        browser_screenshot/content/images/icons/icon128.png
        browser_screenshot/xpi/chrome.manifest
        browser_screenshot/content/fonts/fontawesome-webfont.ttf
        browser_screenshot/content/fonts/fontawesome-webfont.svg
        browser_screenshot/.eslintrc
        browser_screenshot/xpi/js/popup.js
        browser_screenshot/content/users.html
        browser_screenshot/.gitignore
        browser_screenshot/content/css/images/ui-bg_flat_10_000000_40x100.png
        browser_screenshot/crx/manifest.json
        browser_screenshot/README.md
        browser_screenshot/content/css/images/ui-bg_gloss-wave_35_f6a828_500x100.png
        browser_screenshot/content/css/jquery-ui.min.css
        browser_screenshot/LICENSE
        browser_screenshot/content/css/images/ui-bg_glass_100_f6f6f6_1x400.png
        browser_screenshot/CONTRIBUTING.md
        browser_screenshot/content/fonts/fontawesome-webfont.woff2
        browser_screenshot/content/css/jquery.Jcrop.min.css
        browser_screenshot/xpi/README.md
        browser_screenshot/content/js/jquery-ui.min.js
        browser_screenshot/content/css/popup.less
        browser_screenshot/content/fonts/fontawesome-webfont.eot
        browser_screenshot/content/css/images/ui-bg_diagonals-thick_18_b81900_40x40.png
        browser_screenshot/content/css/screenshot.less
    
    
    
    Tool: PEP8 Style Checker
    Ignored Files:
        browser_screenshot/content/images/icons/icon64.png
        browser_screenshot/content/css/font-awesome.min.css
        browser_screenshot/content/fonts/fontawesome-webfont.woff
        browser_screenshot/content/fonts/FontAwesome.otf
        browser_screenshot/content/css/images/ui-icons_ef8c08_256x240.png
        browser_screenshot/content/js/user_form.js
        browser_screenshot/content/css/jquery-ui.min.js
        browser_screenshot/xpi/package.json
        browser_screenshot/xpi/js/save_user.js
        browser_screenshot/content/images/filler.png
        browser_screenshot/content/css/images/ui-icons_ffffff_256x240.png
        browser_screenshot/content/js/users.js
        browser_screenshot/crx/js/save_user.js
        browser_screenshot/content/css/images/ui-bg_highlight-soft_100_eeeeee_1x100.png
        browser_screenshot/content/js/jquery.Jcrop.min.js
        browser_screenshot/content/css/images/ui-bg_glass_65_ffffff_1x400.png
        browser_screenshot/content/popup.html
        browser_screenshot/content/css/toastr.less
        browser_screenshot/content/js/jquery-2.1.4.min.js
        browser_screenshot/content/images/icons/icon16.png
        browser_screenshot/content/images/logo.png
        browser_screenshot/xpi/js/modify_users.js
        browser_screenshot/xpi/index.js
        browser_screenshot/content/css/images/ui-icons_222222_256x240.png
        browser_screenshot/content/css/images/ui-bg_glass_100_fdf5ce_1x400.png
        browser_screenshot/content/js/toastr.min.js
        browser_screenshot/content/images/icons/icon48.png
        browser_screenshot/content/css/images/ui-icons_ffd27a_256x240.png
        browser_screenshot/content/css/images/ui-icons_228ef1_256x240.png
        browser_screenshot/AUTHORS.md
        browser_screenshot/crx/js/modify_users.js
        browser_screenshot/content/js/screenshot.js
        browser_screenshot/content/css/images/ui-bg_diagonals-thick_20_666666_40x40.png
        browser_screenshot/crx/js/popup.js
        browser_screenshot/crx/js/background.js
        browser_screenshot/content/css/images/ui-bg_highlight-soft_75_ffe45c_1x100.png
        browser_screenshot/gulpfile.js
        browser_screenshot/content/screenshot.html
        browser_screenshot/content/css/users.less
        browser_screenshot/content/images/icons/icon32.png
        browser_screenshot/content/images/icons/icon128.png
        browser_screenshot/xpi/chrome.manifest
        browser_screenshot/content/fonts/fontawesome-webfont.ttf
        browser_screenshot/content/fonts/fontawesome-webfont.svg
        browser_screenshot/.eslintrc
        browser_screenshot/xpi/js/popup.js
        browser_screenshot/content/users.html
        browser_screenshot/.gitignore
        browser_screenshot/content/css/images/ui-bg_flat_10_000000_40x100.png
        browser_screenshot/crx/manifest.json
        browser_screenshot/README.md
        browser_screenshot/content/css/images/ui-bg_gloss-wave_35_f6a828_500x100.png
        browser_screenshot/content/css/jquery-ui.min.css
        browser_screenshot/LICENSE
        browser_screenshot/content/css/images/ui-bg_glass_100_f6f6f6_1x400.png
        browser_screenshot/CONTRIBUTING.md
        browser_screenshot/content/fonts/fontawesome-webfont.woff2
        browser_screenshot/content/css/jquery.Jcrop.min.css
        browser_screenshot/xpi/README.md
        browser_screenshot/content/js/jquery-ui.min.js
        browser_screenshot/content/css/popup.less
        browser_screenshot/content/fonts/fontawesome-webfont.eot
        browser_screenshot/content/css/images/ui-bg_diagonals-thick_18_b81900_40x40.png
        browser_screenshot/content/css/screenshot.less
    
    
  2. 
      
mike_conley
  1. Still reviewing, but some more feedback here.

    Do we also not need a package.json so that npm knows what other packages are required?

  2. Each of these functions should have a quick bit of documentation above to describe what exactly they do, what parameters they take, what they return, etc.

    Example:

    /**
     * Adds a server to the list of servers that the user
     * can attach the image to.
     *
     * @param server (String)
     *        The URL of the server to add.
     */
    
  3. browser_screenshot/content/popup.html (Diff revision 4)
     
     
     
     

    I'm curious why we're sticking with a numerical ID... doesn't that make it (cognitively) more difficult to map it to the right action?

    Perhaps change "1" to be "capture_visible_content", "2" to be "capture_area", "3" to be "accounts", etc.

  4. space after if

  5. Best to use textContent here instead of innerHTML.

  6. 
      
JW
reviewbot
  1. Tool: Pyflakes
    Ignored Files:
        browser_screenshot/content/images/icons/icon64.png
        browser_screenshot/content/css/font-awesome.min.css
        browser_screenshot/xpi/js/cs_screenshot.js
        browser_screenshot/content/fonts/fontawesome-webfont.woff
        browser_screenshot/content/fonts/FontAwesome.otf
        browser_screenshot/content/css/images/ui-icons_ef8c08_256x240.png
        browser_screenshot/content/js/user_form.js
        browser_screenshot/content/css/jquery-ui.min.js
        browser_screenshot/xpi/package.json
        browser_screenshot/xpi/js/save_user.js
        browser_screenshot/content/images/filler.png
        browser_screenshot/content/css/images/ui-icons_ffffff_256x240.png
        browser_screenshot/content/js/users.js
        browser_screenshot/crx/js/save_user.js
        browser_screenshot/content/css/images/ui-bg_highlight-soft_100_eeeeee_1x100.png
        browser_screenshot/content/js/jquery.Jcrop.min.js
        browser_screenshot/content/css/images/ui-bg_glass_65_ffffff_1x400.png
        browser_screenshot/content/popup.html
        browser_screenshot/content/css/toastr.less
        browser_screenshot/content/js/jquery-2.1.4.min.js
        browser_screenshot/content/images/icons/icon16.png
        browser_screenshot/content/images/logo.png
        browser_screenshot/xpi/js/modify_users.js
        browser_screenshot/xpi/index.js
        browser_screenshot/content/css/images/ui-icons_222222_256x240.png
        browser_screenshot/content/css/images/ui-bg_glass_100_fdf5ce_1x400.png
        browser_screenshot/content/js/toastr.min.js
        browser_screenshot/content/images/icons/icon48.png
        browser_screenshot/content/css/images/ui-icons_ffd27a_256x240.png
        browser_screenshot/content/css/images/ui-icons_228ef1_256x240.png
        browser_screenshot/AUTHORS.md
        browser_screenshot/package.json
        browser_screenshot/crx/js/modify_users.js
        browser_screenshot/content/js/screenshot.js
        browser_screenshot/content/css/images/ui-bg_diagonals-thick_20_666666_40x40.png
        browser_screenshot/crx/js/popup.js
        browser_screenshot/crx/js/background.js
        browser_screenshot/content/css/images/ui-bg_highlight-soft_75_ffe45c_1x100.png
        browser_screenshot/gulpfile.js
        browser_screenshot/content/screenshot.html
        browser_screenshot/content/css/users.less
        browser_screenshot/content/images/icons/icon32.png
        browser_screenshot/content/images/icons/icon128.png
        browser_screenshot/xpi/chrome.manifest
        browser_screenshot/content/fonts/fontawesome-webfont.ttf
        browser_screenshot/xpi/js/capture.js
        browser_screenshot/content/fonts/fontawesome-webfont.svg
        browser_screenshot/.eslintrc
        browser_screenshot/xpi/js/popup.js
        browser_screenshot/content/users.html
        browser_screenshot/.gitignore
        browser_screenshot/content/css/images/ui-bg_flat_10_000000_40x100.png
        browser_screenshot/crx/manifest.json
        browser_screenshot/README.md
        browser_screenshot/content/css/images/ui-bg_gloss-wave_35_f6a828_500x100.png
        browser_screenshot/content/css/jquery-ui.min.css
        browser_screenshot/LICENSE
        browser_screenshot/content/css/images/ui-bg_glass_100_f6f6f6_1x400.png
        browser_screenshot/CONTRIBUTING.md
        browser_screenshot/content/fonts/fontawesome-webfont.woff2
        browser_screenshot/content/css/jquery.Jcrop.min.css
        browser_screenshot/xpi/README.md
        browser_screenshot/content/js/jquery-ui.min.js
        browser_screenshot/content/css/popup.less
        browser_screenshot/content/fonts/fontawesome-webfont.eot
        browser_screenshot/content/css/images/ui-bg_diagonals-thick_18_b81900_40x40.png
        browser_screenshot/content/css/screenshot.less
    
    
    
    Tool: PEP8 Style Checker
    Ignored Files:
        browser_screenshot/content/images/icons/icon64.png
        browser_screenshot/content/css/font-awesome.min.css
        browser_screenshot/xpi/js/cs_screenshot.js
        browser_screenshot/content/fonts/fontawesome-webfont.woff
        browser_screenshot/content/fonts/FontAwesome.otf
        browser_screenshot/content/css/images/ui-icons_ef8c08_256x240.png
        browser_screenshot/content/js/user_form.js
        browser_screenshot/content/css/jquery-ui.min.js
        browser_screenshot/xpi/package.json
        browser_screenshot/xpi/js/save_user.js
        browser_screenshot/content/images/filler.png
        browser_screenshot/content/css/images/ui-icons_ffffff_256x240.png
        browser_screenshot/content/js/users.js
        browser_screenshot/crx/js/save_user.js
        browser_screenshot/content/css/images/ui-bg_highlight-soft_100_eeeeee_1x100.png
        browser_screenshot/content/js/jquery.Jcrop.min.js
        browser_screenshot/content/css/images/ui-bg_glass_65_ffffff_1x400.png
        browser_screenshot/content/popup.html
        browser_screenshot/content/css/toastr.less
        browser_screenshot/content/js/jquery-2.1.4.min.js
        browser_screenshot/content/images/icons/icon16.png
        browser_screenshot/content/images/logo.png
        browser_screenshot/xpi/js/modify_users.js
        browser_screenshot/xpi/index.js
        browser_screenshot/content/css/images/ui-icons_222222_256x240.png
        browser_screenshot/content/css/images/ui-bg_glass_100_fdf5ce_1x400.png
        browser_screenshot/content/js/toastr.min.js
        browser_screenshot/content/images/icons/icon48.png
        browser_screenshot/content/css/images/ui-icons_ffd27a_256x240.png
        browser_screenshot/content/css/images/ui-icons_228ef1_256x240.png
        browser_screenshot/AUTHORS.md
        browser_screenshot/package.json
        browser_screenshot/crx/js/modify_users.js
        browser_screenshot/content/js/screenshot.js
        browser_screenshot/content/css/images/ui-bg_diagonals-thick_20_666666_40x40.png
        browser_screenshot/crx/js/popup.js
        browser_screenshot/crx/js/background.js
        browser_screenshot/content/css/images/ui-bg_highlight-soft_75_ffe45c_1x100.png
        browser_screenshot/gulpfile.js
        browser_screenshot/content/screenshot.html
        browser_screenshot/content/css/users.less
        browser_screenshot/content/images/icons/icon32.png
        browser_screenshot/content/images/icons/icon128.png
        browser_screenshot/xpi/chrome.manifest
        browser_screenshot/content/fonts/fontawesome-webfont.ttf
        browser_screenshot/xpi/js/capture.js
        browser_screenshot/content/fonts/fontawesome-webfont.svg
        browser_screenshot/.eslintrc
        browser_screenshot/xpi/js/popup.js
        browser_screenshot/content/users.html
        browser_screenshot/.gitignore
        browser_screenshot/content/css/images/ui-bg_flat_10_000000_40x100.png
        browser_screenshot/crx/manifest.json
        browser_screenshot/README.md
        browser_screenshot/content/css/images/ui-bg_gloss-wave_35_f6a828_500x100.png
        browser_screenshot/content/css/jquery-ui.min.css
        browser_screenshot/LICENSE
        browser_screenshot/content/css/images/ui-bg_glass_100_f6f6f6_1x400.png
        browser_screenshot/CONTRIBUTING.md
        browser_screenshot/content/fonts/fontawesome-webfont.woff2
        browser_screenshot/content/css/jquery.Jcrop.min.css
        browser_screenshot/xpi/README.md
        browser_screenshot/content/js/jquery-ui.min.js
        browser_screenshot/content/css/popup.less
        browser_screenshot/content/fonts/fontawesome-webfont.eot
        browser_screenshot/content/css/images/ui-bg_diagonals-thick_18_b81900_40x40.png
        browser_screenshot/content/css/screenshot.less
    
    
  2. 
      
JW
reviewbot
  1. Tool: PEP8 Style Checker
    Ignored Files:
        browser_screenshot/content/images/icons/icon64.png
        browser_screenshot/content/css/font-awesome.min.css
        browser_screenshot/xpi/js/cs_screenshot.js
        browser_screenshot/content/fonts/fontawesome-webfont.woff
        browser_screenshot/content/fonts/FontAwesome.otf
        browser_screenshot/content/css/images/ui-icons_ef8c08_256x240.png
        browser_screenshot/content/js/user_form.js
        browser_screenshot/content/css/jquery-ui.min.js
        browser_screenshot/xpi/package.json
        browser_screenshot/xpi/js/save_user.js
        browser_screenshot/content/images/filler.png
        browser_screenshot/content/css/images/ui-icons_ffffff_256x240.png
        browser_screenshot/content/js/users.js
        browser_screenshot/crx/js/save_user.js
        browser_screenshot/content/css/images/ui-bg_highlight-soft_100_eeeeee_1x100.png
        browser_screenshot/content/js/jquery.Jcrop.min.js
        browser_screenshot/content/css/images/ui-bg_glass_65_ffffff_1x400.png
        browser_screenshot/content/popup.html
        browser_screenshot/content/css/toastr.less
        browser_screenshot/content/js/jquery-2.1.4.min.js
        browser_screenshot/content/images/icons/icon16.png
        browser_screenshot/content/images/logo.png
        browser_screenshot/xpi/js/modify_users.js
        browser_screenshot/xpi/index.js
        browser_screenshot/content/css/images/ui-icons_222222_256x240.png
        browser_screenshot/content/css/images/ui-bg_glass_100_fdf5ce_1x400.png
        browser_screenshot/content/js/toastr.min.js
        browser_screenshot/content/images/icons/icon48.png
        browser_screenshot/content/css/images/ui-icons_ffd27a_256x240.png
        browser_screenshot/content/css/images/ui-icons_228ef1_256x240.png
        browser_screenshot/AUTHORS.md
        browser_screenshot/package.json
        browser_screenshot/crx/js/modify_users.js
        browser_screenshot/content/js/screenshot.js
        browser_screenshot/content/css/images/ui-bg_diagonals-thick_20_666666_40x40.png
        browser_screenshot/crx/js/popup.js
        browser_screenshot/crx/js/background.js
        browser_screenshot/content/css/images/ui-bg_highlight-soft_75_ffe45c_1x100.png
        browser_screenshot/gulpfile.js
        browser_screenshot/content/screenshot.html
        browser_screenshot/content/css/users.less
        browser_screenshot/content/images/icons/icon32.png
        browser_screenshot/content/images/icons/icon128.png
        browser_screenshot/xpi/chrome.manifest
        browser_screenshot/content/fonts/fontawesome-webfont.ttf
        browser_screenshot/xpi/js/capture.js
        browser_screenshot/content/fonts/fontawesome-webfont.svg
        browser_screenshot/.eslintrc
        browser_screenshot/xpi/js/popup.js
        browser_screenshot/content/users.html
        browser_screenshot/.gitignore
        browser_screenshot/content/css/images/ui-bg_flat_10_000000_40x100.png
        browser_screenshot/crx/manifest.json
        browser_screenshot/README.md
        browser_screenshot/content/css/images/ui-bg_gloss-wave_35_f6a828_500x100.png
        browser_screenshot/content/css/jquery-ui.min.css
        browser_screenshot/LICENSE
        browser_screenshot/content/css/images/ui-bg_glass_100_f6f6f6_1x400.png
        browser_screenshot/CONTRIBUTING.md
        browser_screenshot/content/fonts/fontawesome-webfont.woff2
        browser_screenshot/content/css/jquery.Jcrop.min.css
        browser_screenshot/xpi/README.md
        browser_screenshot/content/js/jquery-ui.min.js
        browser_screenshot/content/css/popup.less
        browser_screenshot/content/fonts/fontawesome-webfont.eot
        browser_screenshot/content/css/images/ui-bg_diagonals-thick_18_b81900_40x40.png
        browser_screenshot/content/css/screenshot.less
    
    
    
    Tool: Pyflakes
    Ignored Files:
        browser_screenshot/content/images/icons/icon64.png
        browser_screenshot/content/css/font-awesome.min.css
        browser_screenshot/xpi/js/cs_screenshot.js
        browser_screenshot/content/fonts/fontawesome-webfont.woff
        browser_screenshot/content/fonts/FontAwesome.otf
        browser_screenshot/content/css/images/ui-icons_ef8c08_256x240.png
        browser_screenshot/content/js/user_form.js
        browser_screenshot/content/css/jquery-ui.min.js
        browser_screenshot/xpi/package.json
        browser_screenshot/xpi/js/save_user.js
        browser_screenshot/content/images/filler.png
        browser_screenshot/content/css/images/ui-icons_ffffff_256x240.png
        browser_screenshot/content/js/users.js
        browser_screenshot/crx/js/save_user.js
        browser_screenshot/content/css/images/ui-bg_highlight-soft_100_eeeeee_1x100.png
        browser_screenshot/content/js/jquery.Jcrop.min.js
        browser_screenshot/content/css/images/ui-bg_glass_65_ffffff_1x400.png
        browser_screenshot/content/popup.html
        browser_screenshot/content/css/toastr.less
        browser_screenshot/content/js/jquery-2.1.4.min.js
        browser_screenshot/content/images/icons/icon16.png
        browser_screenshot/content/images/logo.png
        browser_screenshot/xpi/js/modify_users.js
        browser_screenshot/xpi/index.js
        browser_screenshot/content/css/images/ui-icons_222222_256x240.png
        browser_screenshot/content/css/images/ui-bg_glass_100_fdf5ce_1x400.png
        browser_screenshot/content/js/toastr.min.js
        browser_screenshot/content/images/icons/icon48.png
        browser_screenshot/content/css/images/ui-icons_ffd27a_256x240.png
        browser_screenshot/content/css/images/ui-icons_228ef1_256x240.png
        browser_screenshot/AUTHORS.md
        browser_screenshot/package.json
        browser_screenshot/crx/js/modify_users.js
        browser_screenshot/content/js/screenshot.js
        browser_screenshot/content/css/images/ui-bg_diagonals-thick_20_666666_40x40.png
        browser_screenshot/crx/js/popup.js
        browser_screenshot/crx/js/background.js
        browser_screenshot/content/css/images/ui-bg_highlight-soft_75_ffe45c_1x100.png
        browser_screenshot/gulpfile.js
        browser_screenshot/content/screenshot.html
        browser_screenshot/content/css/users.less
        browser_screenshot/content/images/icons/icon32.png
        browser_screenshot/content/images/icons/icon128.png
        browser_screenshot/xpi/chrome.manifest
        browser_screenshot/content/fonts/fontawesome-webfont.ttf
        browser_screenshot/xpi/js/capture.js
        browser_screenshot/content/fonts/fontawesome-webfont.svg
        browser_screenshot/.eslintrc
        browser_screenshot/xpi/js/popup.js
        browser_screenshot/content/users.html
        browser_screenshot/.gitignore
        browser_screenshot/content/css/images/ui-bg_flat_10_000000_40x100.png
        browser_screenshot/crx/manifest.json
        browser_screenshot/README.md
        browser_screenshot/content/css/images/ui-bg_gloss-wave_35_f6a828_500x100.png
        browser_screenshot/content/css/jquery-ui.min.css
        browser_screenshot/LICENSE
        browser_screenshot/content/css/images/ui-bg_glass_100_f6f6f6_1x400.png
        browser_screenshot/CONTRIBUTING.md
        browser_screenshot/content/fonts/fontawesome-webfont.woff2
        browser_screenshot/content/css/jquery.Jcrop.min.css
        browser_screenshot/xpi/README.md
        browser_screenshot/content/js/jquery-ui.min.js
        browser_screenshot/content/css/popup.less
        browser_screenshot/content/fonts/fontawesome-webfont.eot
        browser_screenshot/content/css/images/ui-bg_diagonals-thick_18_b81900_40x40.png
        browser_screenshot/content/css/screenshot.less
    
    
  2. 
      
mike_conley
  1. As a first landing, I think this is pretty good! I think there are some UX and UI improvements to make, perhaps some simplifications in architecture[1], and perhaps there some more cleverness we can do to reduce the duplication between the crx/xpi folders, but as an initial landing I'm digging this.

    [1]: At some point, we might want to try to combine some of these modules, unless there's a good reason to keep them separate. We might be able to reduce some of the functions that seem to just add another level of abstraction. We might also want to opt for top-level objects with function properties instead of top level functions. But I don't think we should worry about that for now - let's just stand this thing up and see how it feels, and then start filing bugs against it so we can start making improvements.

  2. I guess this kind of magic is needed for jQuery selectmenus? Because traditional <select>'s normally size themselves to their largest child.

  3. TIL this is a quick way of clearing out the list!

  4. browser_screenshot/content/js/screenshot.js (Diff revision 6)
     
     
     
     
     
     

    This can be simplified to:

    option.value = serverDropdown.options.length;
    

    Even better though is to set the value to the URL itself? Or is there an advantage to the value being the index?

  5. Space before (

  6. Space before (

  7. Might as well simplify this:

    var allFields = $([apiKey, username, server]);
    
  8. This doesn't appear to be used.

  9. browser_screenshot/content/screenshot.html (Diff revision 6)
     
     
     
     
     
     
     
     
     
     
     
     

    This should be indented 1 space.

    Also, replace "Reviewboard" with "Review Board"

  10. browser_screenshot/crx/js/popup.js (Diff revision 6)
     
     
     
     

    Instead of adding an event listener to every anchor element, you should add an event listener on a parent element (like the buttons div), and then have your handler use the event.target.id to decide what to do.

  11. browser_screenshot/crx/js/save_user.js (Diff revision 6)
     
     
     
     
     
     

    Bit strange to have a switch for just one option. Should probably just be an if condition.

  12. browser_screenshot/xpi/README.md (Diff revision 6)
     
     

    "review board" -> "Review Board"

  13. browser_screenshot/xpi/index.js (Diff revision 6)
     
     

    What calls this?

  14. browser_screenshot/xpi/index.js (Diff revision 6)
     
     

    This is not going to work with multi-process Firefox, as you'll have no access to the contentWindow.

  15. browser_screenshot/xpi/js/popup.js (Diff revision 6)
     
     
     
     
     
     
     

    Same as above, re: adding an event listener to a parent and inspecting the event target id.

  16. browser_screenshot/xpi/package.json (Diff revision 6)
     
     

    "review board" -> "Review Board".

    Maybe take a TODO down to have all of this project metadata come from one central place and get injected at build-time for the addons.

  17. 
      
JW
reviewbot
  1. Tool: PEP8 Style Checker
    Ignored Files:
        browser_screenshot/content/images/icons/icon64.png
        browser_screenshot/content/css/font-awesome.min.css
        browser_screenshot/xpi/js/cs_screenshot.js
        browser_screenshot/content/fonts/fontawesome-webfont.woff
        browser_screenshot/content/fonts/FontAwesome.otf
        browser_screenshot/content/css/images/ui-icons_ef8c08_256x240.png
        browser_screenshot/content/js/user_form.js
        browser_screenshot/content/css/jquery-ui.min.js
        browser_screenshot/xpi/package.json
        browser_screenshot/xpi/js/save_user.js
        browser_screenshot/content/images/exit.png
        browser_screenshot/xpi/js/crop.js
        browser_screenshot/content/css/images/ui-icons_ffffff_256x240.png
        browser_screenshot/content/js/users.js
        browser_screenshot/crx/js/save_user.js
        browser_screenshot/content/css/images/ui-bg_highlight-soft_100_eeeeee_1x100.png
        browser_screenshot/content/js/jquery.Jcrop.min.js
        browser_screenshot/content/css/images/ui-bg_glass_65_ffffff_1x400.png
        browser_screenshot/content/popup.html
        browser_screenshot/content/css/toastr.less
        browser_screenshot/content/js/jquery-2.1.4.min.js
        browser_screenshot/content/images/icons/icon16.png
        browser_screenshot/content/images/logo.png
        browser_screenshot/xpi/js/modify_users.js
        browser_screenshot/xpi/index.js
        browser_screenshot/content/css/images/ui-icons_222222_256x240.png
        browser_screenshot/xpi/js/capture-area.js
        browser_screenshot/content/css/images/ui-bg_glass_100_fdf5ce_1x400.png
        browser_screenshot/content/js/toastr.min.js
        browser_screenshot/content/images/icons/icon48.png
        browser_screenshot/content/css/images/ui-icons_ffd27a_256x240.png
        browser_screenshot/content/css/images/ui-icons_228ef1_256x240.png
        browser_screenshot/AUTHORS.md
        browser_screenshot/package.json
        browser_screenshot/content/images/crop.png
        browser_screenshot/crx/js/modify_users.js
        browser_screenshot/content/js/screenshot.js
        browser_screenshot/content/css/images/ui-bg_diagonals-thick_20_666666_40x40.png
        browser_screenshot/content/css/jcrop-rb-style.css
        browser_screenshot/crx/js/popup.js
        browser_screenshot/crx/js/background.js
        browser_screenshot/content/css/images/ui-bg_highlight-soft_75_ffe45c_1x100.png
        browser_screenshot/gulpfile.js
        browser_screenshot/content/js/jquery-ui.min.js
        browser_screenshot/content/screenshot.html
        browser_screenshot/content/css/users.less
        browser_screenshot/content/images/icons/icon32.png
        browser_screenshot/content/images/icons/icon128.png
        browser_screenshot/xpi/chrome.manifest
        browser_screenshot/content/fonts/fontawesome-webfont.ttf
        browser_screenshot/xpi/js/capture.js
        browser_screenshot/content/fonts/fontawesome-webfont.svg
        browser_screenshot/.eslintrc
        browser_screenshot/xpi/js/popup.js
        browser_screenshot/content/users.html
        browser_screenshot/.gitignore
        browser_screenshot/content/css/images/ui-bg_flat_10_000000_40x100.png
        browser_screenshot/crx/manifest.json
        browser_screenshot/README.md
        browser_screenshot/content/css/images/ui-bg_gloss-wave_35_f6a828_500x100.png
        browser_screenshot/content/css/jquery-ui.min.css
        browser_screenshot/LICENSE
        browser_screenshot/content/css/images/ui-bg_glass_100_f6f6f6_1x400.png
        browser_screenshot/CONTRIBUTING.md
        browser_screenshot/content/fonts/fontawesome-webfont.woff2
        browser_screenshot/content/css/jquery.Jcrop.min.css
        browser_screenshot/xpi/README.md
        browser_screenshot/content/images/filler.png
        browser_screenshot/content/css/popup.less
        browser_screenshot/content/fonts/fontawesome-webfont.eot
        browser_screenshot/content/css/images/ui-bg_diagonals-thick_18_b81900_40x40.png
        browser_screenshot/content/css/screenshot.less
    
    
    
    Tool: Pyflakes
    Ignored Files:
        browser_screenshot/content/images/icons/icon64.png
        browser_screenshot/content/css/font-awesome.min.css
        browser_screenshot/xpi/js/cs_screenshot.js
        browser_screenshot/content/fonts/fontawesome-webfont.woff
        browser_screenshot/content/fonts/FontAwesome.otf
        browser_screenshot/content/css/images/ui-icons_ef8c08_256x240.png
        browser_screenshot/content/js/user_form.js
        browser_screenshot/content/css/jquery-ui.min.js
        browser_screenshot/xpi/package.json
        browser_screenshot/xpi/js/save_user.js
        browser_screenshot/content/images/exit.png
        browser_screenshot/xpi/js/crop.js
        browser_screenshot/content/css/images/ui-icons_ffffff_256x240.png
        browser_screenshot/content/js/users.js
        browser_screenshot/crx/js/save_user.js
        browser_screenshot/content/css/images/ui-bg_highlight-soft_100_eeeeee_1x100.png
        browser_screenshot/content/js/jquery.Jcrop.min.js
        browser_screenshot/content/css/images/ui-bg_glass_65_ffffff_1x400.png
        browser_screenshot/content/popup.html
        browser_screenshot/content/css/toastr.less
        browser_screenshot/content/js/jquery-2.1.4.min.js
        browser_screenshot/content/images/icons/icon16.png
        browser_screenshot/content/images/logo.png
        browser_screenshot/xpi/js/modify_users.js
        browser_screenshot/xpi/index.js
        browser_screenshot/content/css/images/ui-icons_222222_256x240.png
        browser_screenshot/xpi/js/capture-area.js
        browser_screenshot/content/css/images/ui-bg_glass_100_fdf5ce_1x400.png
        browser_screenshot/content/js/toastr.min.js
        browser_screenshot/content/images/icons/icon48.png
        browser_screenshot/content/css/images/ui-icons_ffd27a_256x240.png
        browser_screenshot/content/css/images/ui-icons_228ef1_256x240.png
        browser_screenshot/AUTHORS.md
        browser_screenshot/package.json
        browser_screenshot/content/images/crop.png
        browser_screenshot/crx/js/modify_users.js
        browser_screenshot/content/js/screenshot.js
        browser_screenshot/content/css/images/ui-bg_diagonals-thick_20_666666_40x40.png
        browser_screenshot/content/css/jcrop-rb-style.css
        browser_screenshot/crx/js/popup.js
        browser_screenshot/crx/js/background.js
        browser_screenshot/content/css/images/ui-bg_highlight-soft_75_ffe45c_1x100.png
        browser_screenshot/gulpfile.js
        browser_screenshot/content/js/jquery-ui.min.js
        browser_screenshot/content/screenshot.html
        browser_screenshot/content/css/users.less
        browser_screenshot/content/images/icons/icon32.png
        browser_screenshot/content/images/icons/icon128.png
        browser_screenshot/xpi/chrome.manifest
        browser_screenshot/content/fonts/fontawesome-webfont.ttf
        browser_screenshot/xpi/js/capture.js
        browser_screenshot/content/fonts/fontawesome-webfont.svg
        browser_screenshot/.eslintrc
        browser_screenshot/xpi/js/popup.js
        browser_screenshot/content/users.html
        browser_screenshot/.gitignore
        browser_screenshot/content/css/images/ui-bg_flat_10_000000_40x100.png
        browser_screenshot/crx/manifest.json
        browser_screenshot/README.md
        browser_screenshot/content/css/images/ui-bg_gloss-wave_35_f6a828_500x100.png
        browser_screenshot/content/css/jquery-ui.min.css
        browser_screenshot/LICENSE
        browser_screenshot/content/css/images/ui-bg_glass_100_f6f6f6_1x400.png
        browser_screenshot/CONTRIBUTING.md
        browser_screenshot/content/fonts/fontawesome-webfont.woff2
        browser_screenshot/content/css/jquery.Jcrop.min.css
        browser_screenshot/xpi/README.md
        browser_screenshot/content/images/filler.png
        browser_screenshot/content/css/popup.less
        browser_screenshot/content/fonts/fontawesome-webfont.eot
        browser_screenshot/content/css/images/ui-bg_diagonals-thick_18_b81900_40x40.png
        browser_screenshot/content/css/screenshot.less
    
    
  2. 
      
mike_conley
  1. As a foundation, I think this is good stuff. Thanks Justin!

    I think we can build off of this over time - although it'd be good to get a polished architecture diagram somewhere for where you ended up, like on hackpad.

    For now, just keep working off this in your GitHub, and don't change this underlying commit. Mark any future work as depending on this change. We're eventually going to get a repository for this thing set up, and then we'll get this landed in it.

    Great work!

  2. 
      
david
Review request changed

Status: Discarded

Loading...