6
\$\begingroup\$

I am building a Chrome extension that injects a small overlay in to all websites using a content script. At this point all the injected elements do is sit in the bottom left corner. It has two buttons the top one produces another element and the button one doesn't do anything. The extra element has a button that closes itself, and a text box.

This is my first major JavaScript/Chrome extension project, so my main concerns are:

  • Don't violate chrome extension rules that will caused deprecated code or the like

  • Given that I am inserting new code into a website I want to make sure that this is as efficient as possible as to minimize strain on Chrome.

Basic Injection

Basic Injection

Injection after '+' pressed

Injection after '+' pressed

Manifest.json

{
    "name": "injection",
    "description": "samples at code injection",
    "version": "1",
    "manifest_version": 2,
    "content_scripts": [
        {
            "matches": [ "<all_urls>"],
            "css":["style.css"],
            "js":["jquery-2.1.0.min.js", "index.js"]
        }
    ],
    "permissions": [ "<all_urls>", "storage" ]
}

index.js

    $(function(){
    //$(".add_button").click(add_timeline_element);

    function add_timeline_element(){
        var text_input = $('<input />', {
            type: "text",
            class: "t_text_area"
        });
        var button = $('<button />', {
            text: '-',
            class: "t_intect_button",
            click: function() {$(this).parent().remove();}
        });
        var timeline_element = $('<td />', {
            class: "timeline_element"
        });
        timeline_element.append(button);
        timeline_element.append(text_input);
        $(".t_inject_row").append(timeline_element);
    }

    function minimize_t_inject_container(){
        $(".add_button").toggle();
    }

    function create_twitter_bar(){
        var table_container = $("<table />", {
            class: "t_inject_container"
        });
            var row = $("<tr />", {
                        class: "t_inject_row"
                        });
                var menu = $("<td />", {
                    class: "menu"
                });
                    var add_element_button = $("<button />", {
                        text: '+',
                        class: "add_button t_intect_button",
                        click: function() {add_timeline_element();}
                    });
                    var minimize_button = $("<button />", {
                        text: 'm',
                        click: function() {minimize_t_inject_container();},
                        class: "minimize_button t_intect_button"
                    });
                    menu.append(add_element_button);
                    menu.append(minimize_button);
            row.append(menu);
        table_container.append(row);
        $('body').append(table_container);
    }
    create_twitter_bar();



    var scroll_point = 0;
    var done_scrolling;
    var counting = "false";
    var time_counter = 0.00;
    var consecutive_scrolls = 0.00;

    var scroll_ending = function () {
        console.log("done scrolling");
        clearInterval(counting);
        counting = "false";
        time_counter = 0.00;
    }
    $(document).scroll(function() {
        var new_scroll_point = $(document).scrollTop();
        var scroll_delta = scroll_point - new_scroll_point;
        if(scroll_delta < 0){
            scroll_delta = scroll_delta * (-1);
        }
        scroll_point = new_scroll_point;
        if(counting=="false"){
            counting = setInterval(function(){
                time_counter += 0.1;
            }, 100);
        } else{
            var scroll_over_time = scroll_delta/time_counter;
            console.log("scrolling over time:"+ scroll_over_time);
            clearTimeout(done_scrolling);
            done_scrolling = setTimeout(scroll_ending, 150);
            if(scroll_over_time > 400 && scroll_over_time < 3000){
                $(".add_button").hide();
            }
        }        
    });
});

style.css

  /*----------------*/
 /*----Main Page---*/
/*----------------*/
.t_inject_container
{
    position:fixed;
    bottom:0px;
    left:0px;
    z-index: 999;
    background-color:grey;
    -webkit-box-sizing: border-box; /* Safari/Chrome, other WebKit */
    -moz-box-sizing: border-box;    /* Firefox, other Gecko */
    box-sizing: border-box; 
}
.menu {
    background-color:lightblue;
    border-radius:5px;
}
.t_intect_button {
    background-color:blue;
    display:block;
    height: 50px;
    width: 50px;
    margin: 5px;
    font-size: 20px;
    color: white;
    border: 0px;
    border-radius:7px;
    -webkit-box-sizing: border-box; /* Safari/Chrome, other WebKit */
    -moz-box-sizing: border-box;    /* Firefox, other Gecko */
    box-sizing: border-box; 
    border-radius:5px;
}

 /*-----------------*/
 /*Timeline Element*/
/*----------------*/
.timeline_element {
    background-color:red;
    border-radius:5px;

}
.t_text_area {
    height: 50px;
    width: 100px;
}

Here is a working JSFiddle.

The 'm' button on the main menu has been shrunk down, and now works. It minimizes and resizes the menu. I don't think this can be tested in the jsfiddle, but if you scroll quickly it will also minimize the menu.

The following is what the DOM looks like (copied from the JSFiddle using Chrome's Web Inspector):

<html><head>
  <meta http-equiv="content-type" content="text/html; charset=UTF-8">
  <title> - jsFiddle demo by easilyBaffled</title>

  <script type="text/javascript" src="//code.jquery.com/jquery-2.1.0.js"></script>



  <link rel="stylesheet" type="text/css" href="/css/result-light.css">

  <style type="text/css">
      /*----------------*/
 /*----Main Page---*/
/*----------------*/

// ... PLUS OTHER CSS AS SHOWN IN SOURCE CODE BELOW ...

  </style>



<script type="text/javascript">//<![CDATA[ 
$(window).load(function(){
$(function(){
    //$(".add_button").click(add_timeline_element);

// ... PLUS OTHER JAVASCRIPT AS SHOWN IN SOURCE CODE BELOW ...
    });
});
});//]]>  

</script>


</head>
<body>







<table class="t_inject_container"><tbody><tr class="t_inject_row"><td class="menu"><button class="add_button t_intect_button">+</button><button class="minimize_button t_intect_button">m</button></td></tr></tbody></table></body></html>
\$\endgroup\$
10
\$\begingroup\$

Some notes on your CSS:

  1. You can apply the box-sizing declaration to all the elements in your container:

    .t_inject_container,
    .t_inject_container * {
        -webkit-box-sizing: border-box;
           -moz-box-sizing: border-box;
                box-sizing: border-box;
    }
    
  2. Ommit the units if you have zero values like 0px: bottom: 0;, left: 0;, …

  3. In your rule declaration for .t_inject_button you use the border-radius property two times. I guess you were overlooking it and this is not on purpose?
  4. You repeat your border-radius declaration a few times. Let's make this a bit DRY'er and make an abstraction:

    .t_rounded {
        border-radius: 5px;
    }
    

    Now apply this class to every element you want to have rounded borders on.

Feel free to ask, if you have questions. That's it for now.

Update (based on the added HTML)

  1. There is no doctype in your code. If there really isn't one, you should really add this before the html tag:

    <!DOCTYPE html>
    
  2. If you use the HTML5 doctype (see above), you can switch to a newer, short charset declaration:

    <meta charset="utf-8">
    
  3. You're using a table to wrap your buttons. Tables are for tabular data. Since you this for layout purposes, use div's like so:

    <div class="t_inject_container">
        <ul class="menu">
            <li>
                <button class="add_button t_intect_button">+</button>
            </li>
            <li>
                <button class="minimize_button t_intect_button">m</button>
            </li>
        </ul>
    </div>
    

    I also used an unordered list to mark up your navigation. Use the following CSS to normalize the typical list styles:

    .menu {
        list-style: none;
        margin: 0;
        padding: 0;
    }
    .menu > li {
        display: inline-block;
    }
    
\$\endgroup\$
  • \$\begingroup\$ @EasilyBaffled And I copied the DOM from the fiddle into the question. \$\endgroup\$ – ChrisW Feb 10 '14 at 15:18
  • \$\begingroup\$ @EasilyBaffled Thank you for the demo and the added HTML. I've updated my answer based on this. Ask me, if you have questions. :) \$\endgroup\$ – kleinfreund Feb 10 '14 at 16:33
  • \$\begingroup\$ The div stuff makes sense, I was using a table because I initially had the menu and add as spans and they weren't lining up properly. As for the html, everything is being injected into existing webpages so I no real control of something like doctype. \$\endgroup\$ – EasilyBaffled Feb 11 '14 at 3:30
  • 1
    \$\begingroup\$ @EasilyBaffled I'm not an expert here. Is your stuff injected via an iframe? How does this work? \$\endgroup\$ – kleinfreund Feb 11 '14 at 5:46
  • \$\begingroup\$ nope no iframe necessary. Content scripts, are a specific bit of the Chrome Extension, that allows me to directly affect the DOM of any webpage. In this case 'body' in '$('body').append(table_container);' is the body of what ever is the current tab, and I can add stuff right to it. It's worth noting that I can only access the DOM and not the pages javascript \$\endgroup\$ – EasilyBaffled Feb 11 '14 at 11:11
4
\$\begingroup\$

From a once over:

  • The default for JavaScript is lowerCamelCase, so add_timeline_event -> addTimelineEvent etc. etc.
  • You only call minimize_t_inject_container once, so I would inline this one-liner:
    click: function() { $(".add_button").toggle();},
    or not in-line it, and point straight to the function:
    click:minimize_t_inject_container,
  • Same goes for click: function() {add_timeline_element();} you could just
    click: add_timeline_element
  • I would put your var block higher in the code, and use only 1 var :

    var scroll_point = 0,
        done_scrolling,
        counting = "false",
        time_counter = 0.00,
        consecutive_scrolls = 0.00;
    
  • I am not sure why you do not use false instead of "false" for counting

  • console.log does not belong in production ready code
  • You never use consecutive_scrolls
  • You could use scroll_delta = Math.abs(scroll_delta); instead of

    if(scroll_delta < 0){
        scroll_delta = scroll_delta * (-1);
    }
    

    it is faster and cleaner : http://jsperf.com/math-abs-vs-bitwise/6

As for your concerns:

Don't violate chrome extension rules that will caused deprecated code or the like
I think this looks fine.

Given that I am inserting new code into a website I want to make sure that this is as efficient as possible as to minimize strain on chrome.
There would be less strain if you wrote this without jQuery. Also, now you have to test your extension with sites that already use jQuery ( or use $ for other purposes).

\$\endgroup\$
  • \$\begingroup\$ Does the number of vars change efficiency or is it just for readability? As for abs, I was having trouble using it in the extension for some reason, I have yet to explore it further. \$\endgroup\$ – EasilyBaffled Feb 11 '14 at 3:33
  • 1
    \$\begingroup\$ That would be for readability, as for Math not being available.. that's odd. \$\endgroup\$ – konijn Feb 11 '14 at 17:30

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.