2
\$\begingroup\$

I am allowing user to upload images & drag uploaded image once user click on mask images, also gave an option to Edit the text....

I am fetching mask images and text from json file....

I am uploading each mask images in single canvas & using absolute positing....

But now its getting very difficult to do any code changes because the code structure looks very bad, i have no idea how to improve the look and feel of code structure , need help.....

enter image description here

JSfiddle : https://jsfiddle.net/kidsdial1/rhowf7dn/

pen : https://codepen.io/kidsdial/pen/rbeRZK

function closePopUp(el) {
    el.style.display = 'none';
    openID = null
}

function openPopUp(el) {
    ///console.log(" open is called ",id)
    if (openID != null) {
        closePopUp(openID)
    }
    el.style.display = 'block';
    openID = el;
}


let openID = null;


var target;
var imageUrl = "https://i.imgur.com/RzEm1WK.png";

let jsonData = {  
  "layers" : [
    {
      "x" : 0,
      "height" : 600,
      "layers" : [        
        {
          "x" : 20,
          "height" : 788,
          "src" : "ax0HVTs.png",
          "y" : 70,
          "width" : 650,
          "type" : "image",
          "name" : "shape_1"
        },
        {
          "x" : 40,
          "height" : 140,
          "src" : "iEA642D.jpg",
          "y" : 360,
          "width" : 430,
          "type" : "image",
          "name" : "shape_2"
        },
        
        {
          "font" : "ArchivoNarrow-Bold",
          "x" : 19,
          "y" : 17,
          "width" : 211,
          "src" : "611aa93612da8fde1b17d87368355d1f_Font83.otf",
          "type" : "text",
          "text" : "VALENTINES DAY ",
		  "size" : 27,
          "height" : 22,
          "name" : "edit_sale"
        }
      ],
      "y" : 0,
      "width" : 600,
      "type" : "group",
      "name" : "fb_post_5"
    }
  ]
};

$(document).ready(function() {

    // below code will upload image onclick mask image

    $('.container').click(function(e) {
        // filtering out non-canvas clicks
        if (e.target.tagName !== 'CANVAS') return;

        // getting absolute points relative to container
        const absX = e.offsetX + e.target.parentNode.offsetLeft + e.currentTarget.offsetLeft;
        const absY = e.offsetY + e.target.parentNode.offsetTop + e.currentTarget.offsetTop;

        const $canvasList = $(this).find('canvas');
        // moving all canvas parents on the same z-index
        $canvasList.parent().css({
            zIndex: 0
        });

        $canvasList.filter(function() { // filtering only applicable canvases
            const bbox = this.getBoundingClientRect();
            const canvasTop = bbox.top + window.scrollY;
            const canvasLeft = bbox.left + window.scrollX;
            return (
                absX >= canvasLeft && absX <= canvasLeft + bbox.width &&
                absY >= canvasTop && absY <= canvasTop + bbox.height)
        }).each(function() { // checking white in a click position
            const x = absX - this.parentNode.offsetLeft - e.currentTarget.offsetLeft;
            const y = absY - this.parentNode.offsetTop - e.currentTarget.offsetTop;
            const pixel = this.getContext('2d').getImageData(x, y, 1, 1).data;
            if (pixel[3] === 255) {
                $(this).parent().css({
                    zIndex: 2
                })
                target = this.id;
                console.log(target);
                setTimeout(() => {
                    $('#fileup').click();
                }, 20);
            }
        })
    });

    // Below code will fetch mask images from json file

    function getAllSrc(layers) {
        let arr = [];
        layers.forEach(layer => {
            if (layer.src) {
                arr.push({
                    src: layer.src,
                    x: layer.x,
                    y: layer.y,
                    name: layer.name
                });
            } else if (layer.layers) {
                let newArr = getAllSrc(layer.layers);
                if (newArr.length > 0) {
                    newArr.forEach(({
                        src,
                        x,
                        y,
                        name
                    }) => {
                        arr.push({
                            src,
                            x: (layer.x + x),
                            y: (layer.y + y),
                            name: (name)
                        });
                    });
                }
            }
        });
        return arr;
    }

    function json(data)

    {
        var width = 0;
        var height = 0;

        let arr = getAllSrc(data.layers);

        let layer1 = data.layers;
        width = layer1[0].width;
        height = layer1[0].height;
        let counter = 0;
        let table = [];

        for (let {
                src,
                x,
                y,
                name
            } of arr) {
            $(".container").css('width', width + "px").css('height', height + "px").addClass('temp');
            if (name.indexOf('mask_') !== -1) {
                var imageUrl1 = imageUrl;
            } else {
                var imageUrl1 = '';
            }
            var mask = $(".container").mask({
                imageUrl: imageUrl1,
                maskImageUrl: 'https://i.imgur.com/' + src,
                onMaskImageCreate: function(img) {

                    img.css({
                        "position": "absolute",
                        "left": x + "px",
                        "top": y + "px"
                    });

                },
                id: counter
            });
            table.push(mask);
            fileup.onchange = function() {

                let mask2 = table[target];
                const newImageLoadedId = mask2.loadImage(URL.createObjectURL(fileup.files[0]));
                document.getElementById('fileup').value = "";

                //  Edit image
                $("<span class=\"pip\">" +
                    "<br/><a onclick='document.getElementById(\"light\").style.display=\"block\";'><span class=\"edit\" >Edit </span>" +
                    '<div id=\'light\' class=\'white_content\'>content <a href=\"javascript:void(0)\" onclick=\"document.getElementById(\'light\').style.display=\'none\';">Close</a></div> ' +
                    "</span>").insertAfter("#fileup");

                // Edit image code end here....

                //  Remove image

                $("<br/><span id=\"" + newImageLoadedId + "\" class=\"remove\">Remove image</span>").insertAfter("#fileup");

                $(".remove").click(function(event) {
                    const canvasId = "canvas#" + event.currentTarget.id;
                    // Delete the image
                    const ctx = $("canvas")[event.currentTarget.id].getContext("2d");
                    ctx.fillStyle = "white"
                    ctx.fillRect(0, 0, 500, 500)
                    // Delete the button
                    $(this).remove();
                });

                // Remove image code end here....	
            };
            counter++;
        }
        drawText(data);
    }
    json(jsonData);
}); // end of document ready

const fonts = []; // caching duplicate fonts

function drawText(layer) {

    if (layer.type === 'image') return;

    if (!layer.type || layer.type === 'group') {
        return layer.layers.forEach(drawText)
    }

    if (layer.type === 'text') {
        const url = 'http://piccellsapp.com:1337/parse/files/PfAppId/' + layer.src;

        if (!fonts.includes(url)) {
            fonts.push(url);
            $("style").prepend("@font-face {\n" +
                "\tfont-family: \"" + layer.font + "\";\n" +
                "\tsrc: url(" + url + ") format('truetype');\n" +
                "}");
        }

        // Below is POP UP Code
        const lightId = 'light' + layer.name
        const lightIdString = '#' + lightId
        $('.container').append(
            '<a id ="' + layer.name + '" onclick="openPopUp(' + lightId + ')"' +
            '<div class="txtContainer" contenteditable="true" ' +
            'style="' +
            'left: ' + layer.x + 'px; ' +
            'top: ' + layer.y + 'px; ' +
            'font-size: ' + layer.size + 'px; ' +
            '">' + layer.text + '</div></a>' +
            '<div id="light' + layer.name + '" class="white_content" style="' +
            'left: ' + layer.x + 'px; ' +
            'top: ' + layer.y + 'px; ' + '"> content <a href="javascript:void(0)" ' +
            'onclick="closePopUp(' + lightId + ')">Close</a></div> <div>'
        );
        document.getElementById(lightId).style.left = layer.x + document.getElementById(layer.name).offsetWidth + 'px'
        // Above is POP UP Code
    }

}
// extempl code end	



// jq plugin 

(function($) {
    var JQmasks = [];
    $.fn.mask = function(options) {
        // This is the easiest way to have default options.
        var settings = $.extend({
            // These are the defaults.
            maskImageUrl: undefined,
            imageUrl: undefined,
            scale: 1,
            id: new Date().getUTCMilliseconds().toString(),
            x: 0, // image start position
            y: 0, // image start position
            onMaskImageCreate: function(div) {},
        }, options);


        var container = $(this);

        let prevX = 0,
            prevY = 0,
            draggable = false,
            img,
            canvas,
            context,
            image,
            timeout,
            initImage = false,
            startX = settings.x,
            startY = settings.y,
            div;

        container.mousePosition = function(event) {
            return {
                x: event.pageX || event.offsetX,
                y: event.pageY || event.offsetY
            };
        }

        container.selected = function(ev) {
            var pos = container.mousePosition(ev);
            var item = $(".masked-img canvas").filter(function() {
                var offset = $(this).offset()
                var x = pos.x - offset.left;
                var y = pos.y - offset.top;
                var d = this.getContext('2d').getImageData(x, y, 1, 1).data;
                return d[0] > 0
            });

            JQmasks.forEach(function(el) {
                var id = item.length > 0 ? $(item).attr("id") : "";
                if (el.id == id)
                    el.item.enable();
                else el.item.disable();
            });
        };

        container.enable = function() {
            draggable = true;
            $(canvas).attr("active", "true");
            div.css({
                "z-index": 2
            });
        }

        container.disable = function() {
            draggable = false;
            $(canvas).attr("active", "false");
            div.css({
                "z-index": 1
            });
        }


        container.onDragStart = function(evt) {
            if (evt.target.getContext) {
                var pixel = evt.target.getContext('2d').getImageData(evt.offsetX, evt.offsetY, 1, 1).data;

                $(canvas).attr("active", "true");
                container.selected(evt);
                prevX = evt.clientX;
                prevY = evt.clientY;
                var img = new Image();
                evt.originalEvent.dataTransfer.setDragImage(img, 10, 10);
                evt.originalEvent.dataTransfer.setData('text/plain', 'anything');

            }
        };

        container.getImagePosition = function() {
            return {
                x: settings.x,
                y: settings.y,
                scale: settings.scale
            };
        };

        container.onDragOver = function(evt) {

            if (evt.target.getContext) {
                var pixel = evt.target.getContext('2d').getImageData(evt.offsetX, evt.offsetY, 1, 1).data;
                if (pixel[3] === 255) {
                    if (draggable && $(canvas).attr("active") === "true") {
                        var x = settings.x + evt.clientX - prevX;
                        var y = settings.y + evt.clientY - prevY;
                        if (x == settings.x && y == settings.y)
                            return; // position has not changed
                        settings.x += evt.clientX - prevX;
                        settings.y += evt.clientY - prevY;
                        prevX = evt.clientX;
                        prevY = evt.clientY;
                        clearTimeout(timeout);
                        timeout = setTimeout(function() {
                            container.updateStyle();
                            renderInnerImage();
                        }, 20);
                    }
                } else {
                    evt.stopPropagation();
                    return false;
                }
            }
        };

        container.updateStyle = function() {
            return new Promise((resolve, reject) => {
                context.beginPath();
                context.globalCompositeOperation = "source-over";
                image = new Image();
                image.setAttribute('crossOrigin', 'anonymous');
                image.src = settings.maskImageUrl;
                image.onload = function() {
                    canvas.width = image.width;
                    canvas.height = image.height;
                    context.drawImage(image, 0, 0, image.width, image.height);
                    div.css({
                        "width": image.width,
                        "height": image.height
                    });
                    resolve();
                };
            });
        };

        function renderInnerImage() {
            img = new Image();
            img.setAttribute('crossOrigin', 'anonymous');
            img.src = settings.imageUrl;
            img.onload = function() {
                settings.x = settings.x == 0 && initImage ? (canvas.width - (img.width * settings.scale)) / 2 : settings.x;
                settings.y = settings.y == 0 && initImage ? (canvas.height - (img.height * settings.scale)) / 2 : settings.y;
                context.globalCompositeOperation = 'source-atop';
                context.drawImage(img, settings.x, settings.y, img.width * settings.scale, img.height * settings.scale);
                initImage = false;
            };
        }

        // change the draggable image

        container.loadImage = function(imageUrl) {
            console.log("load");
            //if (img)
            // img.remove();
            // reset the code.
            settings.y = startY;
            settings.x = startX;
            prevX = prevY = 0;
            settings.imageUrl = imageUrl;
            initImage = true;
            container.updateStyle().then(renderInnerImage);
            // sirpepole  Add this
            return settings.id;
        };


        // change the masked Image
        container.loadMaskImage = function(imageUrl, from) {
            canvas = document.createElement("canvas");
            context = canvas.getContext('2d');
            canvas.setAttribute("draggable", "true");
            canvas.setAttribute("id", settings.id);
            settings.maskImageUrl = imageUrl;
            div = $("<div/>", {
                "class": "masked-img"
            }).append(canvas);

            // div.find("canvas").on('touchstart mousedown', function(event)
            div.find("canvas").on('dragstart', function(event) {
                if (event.handled === false) return;
                event.handled = true;
                container.onDragStart(event);
            });

            div.find("canvas").on('touchend mouseup', function(event) {
                if (event.handled === false) return;
                event.handled = true;
                container.selected(event);
            });

            div.find("canvas").bind("dragover", container.onDragOver);
            container.append(div);
            if (settings.onMaskImageCreate)
                settings.onMaskImageCreate(div);
            container.loadImage(settings.imageUrl);
        };
        container.loadMaskImage(settings.maskImageUrl);
        JQmasks.push({
            item: container,
            id: settings.id
        })
        return container;
    };
}(jQuery));
.temp {}

.container {
	background: silver;
  position: relative;
 
}

.container img {
   position:absolute;
   top:0;
   bottom:250px;
   left:0;
   right:0;
   margin:auto;
   z-index:999;
}

.masked-img {
	overflow: hidden;	
	position: relative;
}


.txtContainer{ position:absolute;  text-align:center; color:#FFF}

.txtContainer:hover {
     background: red;
     padding: 1px;
	border-style: dotted;
}

.pip {
  display: inline-block;
  margin: 10px 10px 0 0;
}
.remove {
  display: block;
  background: #444;
  border: 1px solid black;
  color: white;
  text-align: center;
  cursor: pointer;
}
.remove:hover {
  background: white;
  color: black;
}

.edit {
  display: block;
  background: #444;
  border: 1px solid black;
  color: white;
  text-align: center;
  cursor: pointer;
}
.edit:hover {
  background: white;
  color: black;
}

.white_content {
  display: none;
  position: absolute;
  top: 25%;
  left: 25%;
  width: 50%;
  height: 50%;
  padding: 16px;
  border: 16px solid orange;
  background-color: white;
  z-index: 1002;
  overflow: auto;
}
<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>

<input id="fileup" name="fileup" type="file" style="display:none" >

<div class="container">

</div>

\$\endgroup\$
  • \$\begingroup\$ it will be really helpfull if someone give codepen.... \$\endgroup\$ – vickey colors Apr 11 at 7:04
1
+50
\$\begingroup\$

It looks like this code is very similar to the code you posted last month, and in that review blindman67 recommended you consider eliminating the jQuery code. While it isn't imperative that you do so it would perhaps simplify some code and decrease page load times - not that it would likely be a dramatic change but could be less.

You could consider moving anonymous/lambda functions/closures out to named functions - this would allow decreasing the nesting levels.

If you keep the jQuery code, perhaps the suggestions below will be helpful.

While that syntax of $(document).ready() still works with the latest jQuery version (i.e. 3.3.1 at the time of typing), but is deprecated and the recommended syntax is simply $(function() {})1.


I see this variable declaration:

var imageUrl = "https://i.imgur.com/RzEm1WK.png";

That value is never re-assigned, so it could be declared with the const keyword and a common convention among developers is to name constants in all capital letters and separate words with underscores. Additionally that name is somewhat vague. Maybe a better name would be DEFAULT_IMAGE_URL or FALLBACK_IMAGE_URL.


It is wise to cache DOM elements in variables instead of repeatedly querying the DOM2 - e.g.

const containerElement = $('.container');

If that element is the only container on the page, perhaps it would be appropriate to use an id attribute

<div id="container"></div> <!-- could still also have class="container"  -->

and then select it in JavaScript with the id selector:

const containerElement = $('#container');

Then that can be referenced later in the code - e.g.

// below code will upload image onclick mask image

containerElement.click(function(e) {
    // filtering out non-canvas clicks

and

containerElement.css('width', width + "px").css('height', height + "px").addClass('temp');
if (name.indexOf('mask_') !== -1) {
    var imageUrl1 = imageUrl;
 } else {
     var imageUrl1 = '';
 }
 var mask = containerElement.mask({...});

In order to use that in drawText() the scope would need to be widened (e.g. outside of the DOM-ready callback, or have that function moved).

Other elements used frequently that can be cached are $('#fileup'), $(".container"), etc.


In function json() I see a for loop that starts like this:

for (let {
          src,
          x,
          y,
          name
      } of arr) {
      $(".container").css('width', width + "px").css('height', height + "px").addClass('temp');

That first line inside the block will be updating the css of the container element once for each iteration of arr. It would be better to move that call outside the for loop in order to avoid excess function calls.


Lower in function json() I see this line:

fileup.onchange = function() {

But I don't see fileup declared as a variable, local to this code or function, which means this is utilizing a global variable. As joews explains in this answer to Why don't we just use element IDs as identifiers in JavaScript? the HTML5 spec advises against using Named access.

window[name]

Returns the indicated element or collection of elements.
As a general rule, relying on this will lead to brittle code. Which IDs end up mapping to this API can vary over time, as new features are added to the Web platform, for example. Instead of this, use document.getElementById() or document.querySelector().3

Additionally, setting the onchange property of an element would overwrite any previous onchange handler. If there was a need to have multiple events then you can use Event.addEventListener()


At the end of the first click handler on $('.container') I see:

setTimeout(() => {
  $('#fileup').click();
}, 20);

This could be simplified using a Bound function:

const fileUp = $('#fileup'); //can be moved to top of DOM ready callback
setTimeout(fileUp.click.bind(fileUp), 20);

The CSS has a few peculiar aspects. First, I see an empty ruleset for .temp. Also, some of the lines have unique indentation levels- some have tabs while others have two spaces, yet the majority have four spaces

.container {
    background: silver;
  position: relative;

}
.container img {
   position:absolute;
   top:0;
   bottom:250px;
   left:0;
   right:0;
   margin:auto;
   z-index:999;
}

It would improve readability to make all indentation spacing uniform.


In your review Edit and delete uploaded images code inside script tag , which also has very similar code to the code here, x539 recommended using template tags and you asked in comments how to make the styles change. That can be done with JavaScript after the template is cloned - similar to the MDN example that modifies contents after cloning.

Instead of inserting the cloned element immediately:

$('#demoTemplate').clone().insertAfter('#fileup');

modify the sub-elements in-between. Use a traversal function like .find() to locate the elements to alter.

const clone = $('#demoTemplate').clone();
clone.find('.edit').css({left: '3px'});
clone.id = layer.name; //make unique
clone.show();          //remove style="display: none"
clone.insertAfter('#fileup');

1https://api.jquery.com/ready/

2https://www.sitepoint.com/jquery-performance-dom-caching/

3http://w3c.github.io/html/browsers.html#named-access-on-the-window-object

\$\endgroup\$
  • \$\begingroup\$ Thanks for your valuable time, you spent lot of time on this.... you guys deserved lot, ..... not just thanks or upvotes.... \$\endgroup\$ – vickey colors Apr 10 at 9:42
  • \$\begingroup\$ i used const containerElement = $('#container'); in code : codepen.io/kidsdial/pen/gyWwZJ , images loaded , but not displaying in page, please check when you get free time.... \$\endgroup\$ – vickey colors Apr 11 at 7:03
  • \$\begingroup\$ Darn My original statements were too vague and I didn’t explicitly state that the container <div> would need to have an id attribute (e.g. <div id="container"></div>) in order for that to work. I have updated this answer \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ Apr 15 at 5:38

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.