HiveBrain v1.2.0
Get Started
← Back to all entries
snippetjavascriptModerate

Image Filter client

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
clientimagefilter

Problem

I've made an image filtering client for JavaScript that runs on jQuery, Vex and Fabric.JS.

You simply include the required files in your html page, along with something like:


    ImageFilter.defaultOptions.imageCallback = server.upload();


and then have a ` structure where you add an empty data attribute (data-imagefilter) and a filled data attribute (data-imagefilter-image, which is filled with the src).

Once you click on the image, it opens up a popup and lets you edit and save, or edit and cancel a bunch of filters. This has issues running on the local machines are that cross-origin domains don't work, however, this is intended for use on a live environment.




/* ImageFilter.JS
*
* Dependencies:
* - Browser Environment
* - Fabric.js - fabricjs.com
* - jQuery
* - Vex - github.hubspot.com/vex
*/

(function(global) {
"use strict";

global.ImageFilter = {
defaultOptions: {
imageCallback: function(){}
}
};
/ polyfills /
var extend = jQuery.extend;

function $(selector, el) {
if (typeof selector !== "string") {
return selector;
};
if (!el) { el = document; }
return el.querySelector(selector);
}

function selectMultiple(selector, el) {
if (!el) { el = document; }
return Array.prototype.slice.call(el.querySelectorAll(selector));
}
/ end of polyfills /
var ImageFilter = function() {
if (!global) {
throw new Error("window not found.");
}
if (!fabric) {
throw new Error("Fabric.JS not found.");
}
if (!jQuery) {
throw new Error("jQuery not found.");
}
if (!vex) {
throw new Error("Vex not found");
}
this.defaultOptions = {};
this.CANVAS_ID = "#_IFCanvas";
this.filters = {
'sepia': new fabric.Image.filters.Sepia(),
'brightness100': new fabric.Imag

Solution

defaultOptions:

Providing a defaultOptions array is useless if you don't use any other property than imageCallback

The following line is redundant if you wish to create imageCallback and not an empty object:

this.defaultOptions = {};


Getting and assigning click listeners:

There's duplicate logic as far as assigning click listeners go, consider moving them to a function:

$("[data-imagefilter]", document).forEach(function($el) {
        $el.addEventListener("click", function() {
            global.ImageFilter.loadImage(this.dataset["imagefilterImage"]);
        });
    });


into:

ImageFilter.prototype.batchAssignClicks = function(selector, callback, datasetSelector){
            $(selector, document).forEach(function($el) {
            $el.addEventListener("click", function() {
                    callback(this.dataset[datasetSelector]);
            });
            });
    }
    ...
    this.batchAssignClicks("[data-imagefilter]", global.ImageFilter.loadImage, "imagefilterImage");


self:

Where do the references to self come from?

self.canvas = new fabric.Canvas(this.CANVAS_ID.replace("#", ""));


Assuming a old self = this trick was tried, the leftover self calls are still present. Remove them.

vex.dialog.open:

The first parameter of vex.dialog.open is extraneous, renove it.

currentObject.filters:

In the following two lines, an array is reset and then a value added. Why can't we do both?

currentObject.filters = [];
    currentObject.filters.push(this.filters[filter]);


into:

currentObject.filters = [this.filters[filter]];


Miscellanous

  • Name the library something more creative than ImageFilter



  • If it's a real library, a minified version of the code should be made too.

Code Snippets

this.defaultOptions = {};
$$("[data-imagefilter]", document).forEach(function($el) {
        $el.addEventListener("click", function() {
            global.ImageFilter.loadImage(this.dataset["imagefilterImage"]);
        });
    });
ImageFilter.prototype.batchAssignClicks = function(selector, callback, datasetSelector){
            $$(selector, document).forEach(function($el) {
            $el.addEventListener("click", function() {
                    callback(this.dataset[datasetSelector]);
            });
            });
    }
    ...
    this.batchAssignClicks("[data-imagefilter]", global.ImageFilter.loadImage, "imagefilterImage");
self.canvas = new fabric.Canvas(this.CANVAS_ID.replace("#", ""));
currentObject.filters = [];
    currentObject.filters.push(this.filters[filter]);

Context

StackExchange Code Review Q#113931, answer score: 10

Revisions (0)

No revisions yet.