1
\$\begingroup\$

I am allowing users to upload and drag images with this code. Give me a review of this.

Codepen

<script src="https://code.jquery.com/jquery-3.3.1.min.js"></script>

<input id="fileup" type="file" >

<div class="container">
</div>

<style>

.temp {}

.container {
    background: black;
}

.masked-img {
    overflow: hidden;
    margin-top: 30px;
    position: relative;
}
</style>


<script>

var mask;


$(document).ready(function() {

$.getJSON('9images.json', function(data) 

{       
        var width = 0;
        var height = 0;

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

        let layer2 = layer1[0].layers;

        for (i = 1; i < layer2.length; i++) {
            var x = layer2[i].x;
            var y = layer2[i].y;
            var src = layer2[i].layers[0].src;
            $(".container").css('width', width + "px").css('height', height + "px").addClass('temp');

            var mask = $(".container").mask({
                maskImageUrl: 'http://piccellsapp.com:1337/parse/files/PfAppId/' + src,
                onMaskImageCreate: function(img) {

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

            fileup.onchange = function() {
                mask.loadImage(URL.createObjectURL(fileup.files[0]));
            };
        }

    });


}); // end of document ready

// jq plugin for mask
(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) {
            container.selected(evt);
            prevX = evt.clientX;
            prevY = evt.clientY;
        };

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

        container.onDragOver = function(evt) {
            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;
                container.updateStyle();
            }
        };

        container.updateStyle = function() {
            clearTimeout(timeout);
            timeout = setTimeout(function() {
                context.clearRect(0, 0, canvas.width, canvas.height);
                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
                    });
                };

                img = new Image();
                img.src = settings.imageUrl;
                img.setAttribute('crossOrigin', 'anonymous');
                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;
                };
            }, 0);
        };

        // change the draggable image
        container.loadImage = function(imageUrl) {
            if (img)
                img.remove();
            // reset the code.
            settings.y = startY;
            settings.x = startX;
            prevX = prevY = 0;
            settings.imageUrl = imageUrl;
            initImage = true;
            container.updateStyle();
        };

        // change the masked Image
        container.loadMaskImage = function(imageUrl, from) {
            if (div)
                div.remove();
            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) {
                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));
</script>
\$\endgroup\$
4
\$\begingroup\$

Review

I understand that you are learning, and this review may be a little harsh, but there has to be a minimum standard and your code and design falls short. That is not to say you have done poorly, for a beginner you have done well. If not a beginner, well...

All good coders need access to a quality up to date reference. My preference is MDN, its not quite 5 star, but its keeps up with the tech, and missing content can always be found by following the standards links included with every reference.

A good entry point is MDN javascript reference


Interface

The interface design is very unfriendly.

What this needs

  • Your update and render functions are out of sync with the display which is causing very bad flicker as the image is being dragged.

  • The image is constrained to its edges. This means that if the image is larger than the mask it can not be moved.

  • There is no feedback for both the mouse and touch input. The mouse cursor should show appropriate cursors when mouse is over draggable image. While dragging, mouse over, or on touch, there should be a visible (top layer) outline of the dragged image so the user does not lose it under the mask.

  • The page is not responsive, and the scroll does not behave as expected. The page should scale to fit the visible page and be centered.

  • There is no way to adjust the relative scale of the draggable image.

Extras

Some extra features to make the app more usable.

  • Change the background or mask colour.

  • Mirror or rotate the draggable image. Use widget style handles (render in canvas) for mouse, and pinch scale and rotate for touch.

  • Add drag drop for image input.


Code

The main problem is that you are not rendering correctly. This means that the code needs to be rewritten from the ground up

JQuery

jQuery is unneeded bloat, (both for page load and resources) and is encouraging you into some very bad design. You DON'T need jQuery, modern browsers have very advanced APIs that are much faster than jQuery, using jQuery means you are not keeping up with the state of the art in front end web design.

Examples of jQuery native equivalents

/* You had...........................................................*/
$.getJSON('9images.json', function(data) {

// becomes
fetch("9images.json").then(result => result.json().then(function(data) {

/* You had...........................................................*/
$(".container")
// move to function scope and query once
const container = document.querySelector(".container");

/* You had...........................................................*/
$(".container").css('width', width + "px").css('height', height + "px").addClass('temp');

// becomes
container.style.width = width + "px";
container.style.height = height + "px";
container.classList.add("temp");

// or
Object.assign(container.style, {width: width + "px", height: height + "px");
container.classList.add("temp");

/* You had...........................................................*/
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);    

// becomes        
const settings = {
    maskImageUrl: undefined,
    imageUrl: undefined,
    scale: 1,
    id: new Date().getUTCMilliseconds().toString(),
    x: 0, 
    y: 0, 
    onMaskImageCreate: function(div) {},
    ...options,
};    

/* You had...........................................................*/
item.length > 0 ? $(item).attr("id") : "";

// becomes 
item.length ? item[0].id : "";


/* You had...........................................................*/
div.css({
    "z-index": 1
});

// becomes
div.style.zIndex = 1;

/* You had...........................................................*/
img.css({
    "position": "fixed",
    "left": x + "px",
    "top": y + "px"
});    

// becomes
Object.assign(img.style, {
    position : "fixed",
    left: x + "px",
    right: y + "px",
});

Rendering

To stop the flicker you need to use requestAnimationFrame.

Because mouse and touch events are not synced with the display device the render function needs to run in a polling loop. Mouse and touch events should signal via semaphore that there is need to render. The render function should check the semaphore and render as required.

Example

var renderUpdate = true; // semaphore to render content
const mouse = {x,y,button};
function mouseMoveListener(event) {
    mouse.x = event.pageX;
    mouse.y = event.pageY;
    renderUpdate = true;
}
function mouseOverOutListener(event) {  // add to main canvas.
    mouse.cursor = event.type === "mouseover" ? "move" : "default";
}


requestAnimationFrame(renderLoop); // starts the loop
function renderLoop() {
    if (renderUpdate) {
        // call render functions
        renderUpdate = false;
    }
    mainCanvas.cursor = mouse.cursor;
    requestAnimationFrame(renderLoop);
}

Compositing

The DOM is fairly good at compositing, but it is not good at guessing what you want to do.

Multi layer image editing is best done on a single canvas with the alpha turned off (Not transparent). In your render loop you can then have very fine control over how the compositing is done.

eg

// alpha : false. Prevents the DOM from compositing the canvas with the 
// page background, which may not even be visible.
const ctx = mainCanvas.getContext("2d", {alpha : false}); // do once

// load images and masks to a single array
const layers = [];
function addLayer(imgUrl) {
    const image = new Image;
    image.src = imgURL;
    image.onload = () => { // add image to layers when loaded
        layers.push({image, x : 0, y : 0, scale : 1});
    }
    // To do
    // Add handler for image error
}

// only call this from within the requestAnimationFrame callback function
function renderLayers(layers) {
    // As the canvas is not transparent you need to set the background colour
    ctx.fillStyle = backgroundColor;
    ctx.fillRect(0, 0, ctx.canvas.width, ctx.canvas.height);

    for (const layer of layers) {
        ctx.drawImage(
            layer.image, 
            layer.x, layer.y, 
            layer.image.width * layer.scale, 
            layer.image.height * layer.scale
        );
    }
}
\$\endgroup\$
3
  • \$\begingroup\$ Thanks a lot for your valuable time and suggestion , I will really follow it , you are right that sometimes larger images are not dragging , smaller images don't have any problem in dragging.... If you can give me updated code in codepen or in snippet , than it will be really very very helpful for me , when you get free time , please give me full code in codepen or snippet..... \$\endgroup\$ – vickey colors Mar 2 '19 at 1:35
  • 1
    \$\begingroup\$ @vickeycolors Sorry this is not a code writing service. \$\endgroup\$ – Blindman67 Mar 5 '19 at 16:38
  • 1
    \$\begingroup\$ You are absolutely right, i followed your suggestions & i tried to make snippet from my side , but it gave many errors, so i requested for you.... thats okay i will try from my side for some more time..... \$\endgroup\$ – vickey colors Mar 6 '19 at 9:52

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.