4
\$\begingroup\$

I basically have a folder view structure, that each time I call grabs the value of the selected field and gets that specific folder data. I was wondering if there's a better way to do this by caching or something like that. So for example if I was to request that same folder it would be stored in memory, instead of having to make another get request.

var getFolder = function() {

    if ($('.list-group-item:hover').css('margin-left') === '0px') {
        $('.fold').remove();
    }

    var promises = [];

    $('.list-group-item').css("background-color", ""); // clear last selected

    $.get("/folder", {
        folder: $('.list-group-item:hover').attr('title') //.clone().children().text()
    }, function(data) {
        var folders = data.CommonPrefixes; // folders
        var jsonData = data.Contents; //json data

        var selected = $('.list-group-item:hover').css("background-color", "rgb(0, 196, 255)"); // new selected color



        var parFolder = parseInt($('.list-group-item:hover').css('margin-left'), 10);
        var add = parFolder + 15;
        var subFolder = add.toString() + 'px';

        if (folders.length > 0) {

            for (var k = 0; k < folders.length; k++) {

                var value = folders[k].Prefix;
                var displayVal = value.split('/').slice(-2).join("");


                var book = "<li class='fold list-group-item' onclick='getFolder()' title='" + value + "'>" + "<span class='glyphicon glyphicon-book'></span>" + displayVal + "<span class='glyphicon glyphicon-remove-sign' onclick='deleteFldr()'></span>" + "</li>"

                $(book).insertAfter(".list-group-item:hover").css('margin-left', subFolder);

            }
        }

        if (jsonData.length > 0) {
            for (var i = 0; i < jsonData.length; i++) {

                var req = $.get('/fldrContents', {
                    contents: jsonData[i].Key
                }, function(data) {
                    //var b = JSON.parse(data.body);
                });

                promises.push(req);

            }
        }

        $.when.apply(null, promises).done(function(d) {

            if ($('.dynamic')) {
                $('.dynamic').remove();
            }
            for (var i = 0; i < promises.length; i++) {

                var posTop;
                var posLeft;

                var z = JSON.parse(promises[i].responseText);
                console.log(z)

                var fileURL = z.path;

                var lastChar = fileURL.charAt(fileURL.length - 1);
                if (lastChar === '/') {
                    continue;
                }
                $('.description').css('display', 'none')

                var fileBody = JSON.parse(z.body);
                var website = fileBody.website;
                var title = fileBody.title;
                var description = fileBody.description;
                var created = fileBody.created;
                var canvasData = fileBody.canvas;

                // console.log(canvasData)

                if (fileBody.hasOwnProperty('description')) {

                    $('.description').css('display', 'block')
                }



                if (fileBody.hasOwnProperty('position')) {

                    var posTop = fileBody.position.top;
                    var posLeft = fileBody.position.left;

                    $('dynamic').css({
                        'top': posTop,
                        'left': posLeft
                    });



                    $(".static").append('<div class="dynamic col-md-6" style="top:' + posTop + 'px;left:' + posLeft + 'px">' +
                        '<div class="panel">' +
                        '<canvas id="canvas"></canvas>' +
                        '<div class="panel-body">' +
                        '<a href="' + website + '" target="_blank" class="title" title="Go to ' + website + '">' + title + '</a>' +
                        '<span class="website-edit"></span>' +
                        '<div class="description">' + description + '</div>' +
                        '</div>' +
                        '<div class="panel-footer">' +
                        '<span class="glyphicon glyphicon-bookmark" title="' + fileURL + '"></span>' +
                        '<span class="glyphicon glyphicon-time" title="' + created + '"></span>' +
                        '<span class="glyphicon glyphicon-edit" title="Edit this bookmark" onclick="edit()"></span>' +
                        '<span class="delete glyphicon glyphicon-remove-sign" title="Delete this bookmark" onclick="deleteBookmark()"></span>' +
                        '</div>' +
                        '</div>' +
                        '</div>');
                }

                if (!fileBody.hasOwnProperty('position')) {
                    $(".static").append('<div class="dynamic col-md-6">' +
                        '<div class="panel">' +
                        '<canvas id="canvas"></canvas>' +
                        '<div class="panel-body">' +
                        '<a href="' + website + '" target="_blank" class="title" title="Go to ' + website + '">' + title + '</a>' +
                        '<span class="website-edit"></span>' +
                        '<div class="description">' + description + '</div>' +
                        '</div>' +
                        '<div class="panel-footer">' +
                        '<span class="glyphicon glyphicon-bookmark" title="' + fileURL + '"></span>' +
                        '<span class="glyphicon glyphicon-time" title="' + created + '"></span>' +
                        '<span class="glyphicon glyphicon-edit" title="Edit this bookmark" onclick="edit()"></span>' +
                        '<span class="delete glyphicon glyphicon-remove-sign" title="Delete this bookmark" onclick="deleteBookmark()"></span>' +
                        '</div>' +
                        '</div>' +
                        '</div>')

                }

                if (fileBody.hasOwnProperty('canvas')) {

                    var canvas = document.getElementById('canvas');
                    var ctx = canvas.getContext('2d');
                    canvasData = new Image();
                    canvasData.src = promises[i];



                }



                $('.dynamic').draggable({
                    handle: "div",
                    stop: function(event, ui) {

                        var changedFile = $(this).children('.panel').children('.panel-footer').children('.glyphicon-bookmark').attr('title');

                        var pos = ui.position
                        var title = $(this).children('.panel').children('.panel-body').children('a.title').text();
                        var description = $(this).children('.panel').children('.panel-body').children('.description').text();
                        var website = $(this).children('.panel').children('.panel-body').children('a.title').attr('href');
                        var created = $(this).children('.panel').children('.panel-footer').children('.glyphicon-time').attr('title');
                        var canvas = $(this).children('.panel').children('#canvas') // fix this
                        var canvasData = canvas[0].toDataURL("image/png");

                        console.log(created + ' ' + title + ' ' + description + ' ' + canvasData)

                        $.post('/cssPosition', {
                            website: website,
                            title: title,
                            description: description,
                            position: pos,
                            folder: changedFile,
                            created: created,
                            canvas: canvasData
                        }, function() {

                        });
                        //location.reload();

                    }
                });



            }


        });


    });
}
\$\endgroup\$
3
  • \$\begingroup\$ jQuery automatically caches stuff if you set cache: true in the ajax options. No need to roll your own \$\endgroup\$ Commented Jun 28, 2015 at 15:17
  • \$\begingroup\$ i saw that before, but i was wondering if even before the get call you could store it, because jquery caches it but its still making another request \$\endgroup\$ Commented Jun 28, 2015 at 16:22
  • \$\begingroup\$ First, check that the request actually goes anywhere. jQuery's cache: true basically allows the browser to use its cache. A request is still created (and shows up in the dev tools), but it may never leave your computer if the browser has a cache - and your server is saying that it's ok to cache responses. Again, the console/network log in the dev tools will tell you what the request did. You can still make your own cache if you need to, but that's a question for, say, StackOverflow. This site is for reviewing existing code, so we can't review code you haven't written. \$\endgroup\$ Commented Jun 28, 2015 at 16:34

1 Answer 1

3
\$\begingroup\$
var selected = $('.list-group-item:hover').css("background-color", "rgb(0, 196, 255)");

rgb(0, 196, 255 tells me nothing, as I am not a computer and I can't visualize a color with just those numbers in my head.

Create a variable called with the name of the equivalent color in all caps, and set it to that rgb string.

For example, if you were changing the background to red, you would put:

var REDISH_ORANGE = "rgb(255, 68, 0)";

var selected = $('.list-group-item:hover').css("background-color", REDISH_ORANGE);

var parFolder = parseInt($('.list-group-item:hover').css('margin-left'), 10);
var add = parFolder + 15;
var subFolder = add.toString() + 'px';

This is a waste of memory; you are only using parFolder and add a single time, and that single time is to help create the variable right beneath it.

Just chain these together and stick them in subFolder:

var subFolder = (parseInt($('.list-group-item:hover').css('margin-left', 10) + 15).toString() + 'px';

Yes, you are only using subFolder once, but the name "subFolder" is describing what the value is. Names like "parFolder" (which I assume is short for "Parsed int folder", or something like that) and "add" sound like "filler words" and mean nothing to the variable or context.


You use

$('.list-group-item:hover')

quite a lot. Stick this into a variable so it is easier to access.


In your function, you should "use strict";.


This code was a little difficult to review without seeing the webpage, and without seeing the file structure/tree.

If you could add these to your post, I could edit my answer to add more improvements.

\$\endgroup\$
1
  • \$\begingroup\$ thanks for the review! I'm basically replicating a folder type structure for the UI and this particular function is getting all the contents of the folder and appending it to the view. \$\endgroup\$ Commented Jul 15, 2015 at 20:12

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.