2
\$\begingroup\$

This is my first time writing a Vanilla JS plugin.

I have tried to make a simple HTML5 canvas drawing script. I have tried to handle both touch and mouse events on the canvas.

Kindly help me point out the mistakes or probable bugs that are present or may arise. I would also be thankful if you can guide me on how to fix them.

/*!
* Scribbly.js
* @author  Ayan_Dey
* @version 0.0.1
*/

(function (root, factory) {
    if ( typeof define === 'function' && define.amd ) {
        define([], factory(root));
    } else if ( typeof exports === 'object' ) {
        module.exports = factory(root);
    } else {
        root.Scribbly = factory(root);
    }
})(typeof global !== 'undefined' ? global : this.window || this.global, function (root) {

    'use strict';

    /**
     * Variables
     */
    var board,
        ctx,
        opts,
        isDrawing = false,
        isDragging = false,
        startX,
        startY;

    // Constructor
    function Scribbly(options) {

        // Default settings
        var defaults = {
            canvas: "",
            lineThickness: 2,
            lineColor: "#000000",
            toolbar: true // ToDo toolbar to be added with different tools
        };

        // extend config
        opts = extend(defaults, options || {} );

        // initialize plugin
        this.init();
    }


    /**
     *  Public Methods
     */

    Scribbly.prototype.init = function () {
        // Get the canvas ready to draw
        board = document.getElementById(opts.canvas);
        ctx = board.getContext("2d");

        // Add mouse event listeners to canvas element
        board.addEventListener("mousedown", press, false);
        board.addEventListener("mousemove", drag, false);
        board.addEventListener("mouseup", release);
        board.addEventListener("mouseout", cancel, false);

        // Add touch event listeners to canvas element
        board.addEventListener("touchstart", press, false);
        board.addEventListener("touchmove", drag, false);
        board.addEventListener("touchend", release, false);
        board.addEventListener("touchcancel", cancel, false);

    };

    // Clear the canvas
    Scribbly.prototype.clear = function() {
        ctx.clearRect(0, 0, board.width, board.height);
    };


    /**
     *  Private Methods
     */

    /**
     * Merge two or more objects. Returns a new object.
     */
    var extend = function () {

        // Variables
        var extended = {};
        var deep = false;
        var i = 0;
        var length = arguments.length;

        // Check if a deep merge
        if ( Object.prototype.toString.call( arguments[0] ) === '[object Boolean]' ) {
            deep = arguments[0];
            i++;
        }

        // Merge the object into the extended object
        var merge = function (obj) {
            for ( var prop in obj ) {
                if ( Object.prototype.hasOwnProperty.call( obj, prop ) ) {
                    // If deep merge and property is an object, merge properties
                    if ( deep && Object.prototype.toString.call(obj[prop]) === '[object Object]' ) {
                        extended[prop] = extend( true, extended[prop], obj[prop] );
                    } else {
                        extended[prop] = obj[prop];
                    }
                }
            }
        };

        // Loop through each object and conduct a merge
        for ( ; i < length; i++ ) {
            var obj = arguments[i];
            merge(obj);
        }

        return extended;

    };

    // Get the coordinates of the mouse click
    var getMousePos = function(evt) {
        var rect = board.getBoundingClientRect();
        return {
            x: evt.clientX - rect.left,
            y: evt.clientY - rect.top
        };
    };

    // Get the coordinates of the tap
    var getTouchPos = function(evt) {
        var rect = board.getBoundingClientRect();
        return {
            x: evt.touches[0].clientX - rect.left,
            y: evt.touches[0].clientY - rect.top
        };
    };

    // Mouse press/ touchstart event
    var press = function(e) {
        isDrawing = true;

        if(e.type === 'touchstart') {
            startX = getTouchPos(e).x;
            startY = getTouchPos(e).y;
        }
        else {
            startX = getMousePos(e).x;
            startY = getMousePos(e).y;
        }

        draw(startX, startY);
    };

    // Mouse/ touch drag event
    var drag = function(e) {
        if(isDrawing) {
            isDragging = true;
            if(e.type === 'touchmove') {
                draw(getTouchPos(e).x, getTouchPos(e).y);
            }
            else {
                draw(getMousePos(e).x, getMousePos(e).y);
            }
        }
    };

    // Mouse release/ touchend event
    var release = function(e) {
        isDrawing = false;
        isDragging = false;

        startX = null;
        startY = null;
    };

    // When mouse or touch goes out of the canvas
    var cancel = function(e) {
        isDrawing = false;
        isDragging = false;
    };

    var draw = function(x, y) {
        ctx.lineJoin = "round";
        ctx.lineWidth = opts.lineThickness;
        ctx.strokeStyle = opts.lineColor;

        ctx.beginPath();

        if(isDragging) {
            ctx.moveTo(startX, startY);
            startX = x;
            startY = y;
        }
        else {
            ctx.moveTo(x, y);
        }
        ctx.lineTo(x, y);
        ctx.closePath();
        ctx.stroke();
    };


    /**
     * Public APIs
     */

    return Scribbly;

});
\$\endgroup\$
  • \$\begingroup\$ Live demo: jsfiddle.net/khou0tp3 \$\endgroup\$ – ComFreek Dec 30 '17 at 10:02
  • 1
    \$\begingroup\$ Could you clarify — this is a plug-in to what system? \$\endgroup\$ – 200_success Dec 30 '17 at 14:41
  • \$\begingroup\$ Just a javascript plugin for drawing or scribbling on the web \$\endgroup\$ – Ayan Dec 30 '17 at 14:54
  • 1
    \$\begingroup\$ But "plugin" implies that it implements an API specified by some larger system (e.g. a Photoshop plugin). \$\endgroup\$ – 200_success Dec 30 '17 at 15:04
  • \$\begingroup\$ I guess it applies for web too since you can just plugin the code and get going with the functionality you want in your web project. For ex: plainjs.com/javascript/plugins \$\endgroup\$ – Ayan Dec 30 '17 at 15:13
1
\$\begingroup\$

Drawing comments

  • Currently, one can also draw with the right mouse button, however, upon releasing it, the context menu is still triggered. Contrary to generally disabling the right mouse button, which is abysmal UX, it is indeed useful for a gaming/drawing <canvas>.
    It works with the following code (live demo):

    board.addEventListener("contextmenu", function(e) {
      e.stopPropagation();
      e.preventDefault();
      return false;
    });
    

Code comments

  • It's good that you use === and !== for (in)equality checks!
  • You can use Object.assign instead of your custom extend() function: opts = Object.assign({}, defaults, options || {}). Be aware, though, that it cannot deep-copy/merge!

  • I got into the habit of declaring every variable either with let or const and never with var. In 99% of the cases, you really want block-scoped and not function-scoped variables.

    Especially, I would recommend against declaring your helper functions as var getMousePos = function(evt) {. This way, the variables could even be reassigned, which is almost always a strange behavior for functions.
    Either write const getMouePos = function(evt) { or function getMouePos(evt).

  • Use consistent string quotes: either '...' or "...", but not both if not required (e.g. const str = "Eve's dog is barking").

  • The code repeatedly makes unnecessary function calls: draw(getTouchPos(e).x, getTouchPos(e).y). Granted, they are probably super cheap in your case, but could be avoided even without doind any kind of premature optimization:

    • Make draw accept a {x, y} object. If you now also replace startX, startY by a single variable, this would make the internal API much more consistent.
    • Introduce a variable:

      // Previously
      draw(getTouchPos(e).x, getTouchPos(e).y);
      
      // Now
      const touchPos = getTouchPos(e);
      draw(touchPos.x, touchPos.y);
      
      // Previously
      startX = getTouchPos(e).x;
      startY = getTouchPos(e).y;
      
      // Now
      const touchPos = getTouchPos(e);
      [startX, startY] = [touchPos.x, touchPos.y]; // or arguably with two lines
      
\$\endgroup\$
  • \$\begingroup\$ So from the first part of your answer, I didn't quiet get you. Should I disable right mouse? \$\endgroup\$ – Ayan Dec 30 '17 at 12:38
  • \$\begingroup\$ one more thing and that is: Am I correctly handling touch movements? Because on desktop browsers (even while emulating touches), a single click or tap produces a dot or stroke on the canvas but on touch devices, a single tap doesn't produce anything unless I start dragging. \$\endgroup\$ – Ayan Dec 30 '17 at 12:46
  • \$\begingroup\$ @Ayan Re right mouse clicks: Well that depends on you and your users. This is more of a (subjective) UX question. Re event handling: I can't reproduce that, tapping in Chrome Canary does nothing. I have not done anything wrt. touch handling recently, so I can't comment on its correctness, unfortunately. \$\endgroup\$ – ComFreek Dec 30 '17 at 13:28
  • \$\begingroup\$ Ok. In case you want to reproduce, then just tap on the demo you created from your touch device (mobile) \$\endgroup\$ – Ayan Dec 30 '17 at 14:32

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.