UpvoteJS, a simple voting widget in vanilla JavaScript
$begingroup$
I implemented a simple voting widget like the one used on Stack Exchange sites, to use for other purposes (see live on bashoneliners.com), as a reusable package, dubbed UpvoteJS.
Here's how it works:
Upvote.create('topic-123');
<link rel="stylesheet" href="https://janosgyerik.github.io/upvotejs/dist/upvotejs/upvotejs.css">
<script src="https://janosgyerik.github.io/upvotejs/dist/upvotejs/upvotejs.vanilla.js"></script>
<div id="topic-123" class="upvotejs upvotejs-serverfault">
<a class="upvote upvote-on"></a>
<span class="count">101</span>
<a class="downvote"></a>
<a class="star star-on"></a>
</div>
How to use
As you can see in the above snippet, the typical use has the following elements:
Include the stylesheet
upvotejs.css
with a<link>
tag. The companionupvotejs.svg
is expected in the same directory on the web server.Add HTML markup with the fields you need. With the stylesheet included, this is enough to get a nicely rendered widget, read-only, since without JavaScript, the clicks will have no effect.
To make the widget interactive, responding to user clicks, include the JavaScript package
upvotejs.vanilla.js
with a<script>
tag, and activate the widget using the providedUpvote.create(topicId)
function in JavaScript (to happen after the DOM is loaded). ThetopicId
parameter is mandatory, and it must refer to a unique id in the DOM.To make the widget save state to a backend (not featured in the snippet), pass to
Upvote.create
as second parameter a JSON object, with a field namedcallback
, which must be a JavaScript function, for example:Upvote.create(topicId, {callback: your_callback_function})
. It is up to the developer to implementyour_callback_function
to persist its payload. The function will get called on any state change.
The above should be enough for typical use cases.
Additional notes:
All sub-elements in the HTML markup are optional, for example it's possible to have a widget without a star button, or any other element.
The initial state of a widget can be set either with HTML markup (recommended), or using basic markup (without
upvote-on
,downvote-on
,star-on
classes) and setting values in the second parameter of theUpvote.create
call, for example:Upvote.create(topicId, {count: 123, upvoted: true})
Upvote.create
does some sanity checks, and throws an exception when it detects illegal state, for example (not an exhaustive list):
- The specified ID doesn't exist in the DOM
- The parameter types in the JSON object don't match what's expected (
count
must be integer, and so on)
upvoted
anddownvoted
are bothtrue
Upvote.create
returns an object that represents the widget, and can be used to inspect and control its state. In the typical use case, this is probably not necessary.
Implementation
I'm most interested in a review of the JavaScript part (upvotejs.vanilla.js
(2.1.0)),
the complete project is available on GitHub.
const Upvote = function() {
const upvoteClass = 'upvote';
const enabledClass = 'upvotejs-enabled';
const upvoteOnClass = 'upvote-on';
const downvoteClass = 'downvote';
const downvoteOnClass = 'downvote-on';
const starClass = 'star';
const starOnClass = 'star-on';
const countClass = 'count';
const Utils = {
combine: function() {
const combined = {};
for (let i = 0; i < arguments.length; i++) {
Object.entries(arguments[i])
.filter(e => e[1] !== undefined)
.forEach(e => combined[e[0]] = e[1]);
}
return combined;
},
isBoolean: v => typeof v === "boolean",
isFunction: v => typeof v === "function",
classes: dom => dom.className.split(/ +/).filter(x => x),
removeClass: (dom, className) => {
dom.className = dom.className.split(/ +/)
.filter(x => x)
.filter(c => c !== className)
.join(' ');
},
noop: () => {}
};
const Model = function() {
const validate = params => {
if (!Number.isInteger(params.count)) {
throw 'error: parameter "count" must be a valid integer';
}
if (!Utils.isBoolean(params.upvoted)) {
throw 'error: parameter "upvoted" must be a boolean';
}
if (!Utils.isBoolean(params.downvoted)) {
throw 'error: parameter "downvoted" must be a boolean';
}
if (!Utils.isBoolean(params.starred)) {
throw 'error: parameter "starred" must be a boolean';
}
if (params.callback && !Utils.isFunction(params.callback)) {
throw 'error: parameter "callback" must be a function';
}
if (params.upvoted && params.downvoted) {
throw 'error: parameters "upvoted" and "downvoted" must not be true at the same time';
}
};
const create = params => {
validate(params);
const data = Utils.combine(params);
const upvote = () => {
if (data.upvoted) {
data.count--;
} else {
data.count++;
if (data.downvoted) {
data.downvoted = false;
data.count++;
}
}
data.upvoted = !data.upvoted;
};
const downvote = () => {
if (data.downvoted) {
data.count++;
} else {
data.count--;
if (data.upvoted) {
data.upvoted = false;
data.count--;
}
}
data.downvoted = !data.downvoted;
};
return {
count: () => data.count,
upvote: upvote,
upvoted: () => data.upvoted,
downvote: downvote,
downvoted: () => data.downvoted,
star: () => data.starred = !data.starred,
starred: () => data.starred,
data: () => Utils.combine(data)
};
};
return {
create: create
};
}();
const View = function() {
const create = id => {
const dom = document.getElementById(id);
if (dom === null) {
throw 'error: could not find element with ID ' + id + ' in the DOM';
}
if (Utils.classes(dom).includes(enabledClass)) {
throw 'error: element with ID ' + id + ' is already in use by another upvote controller';
}
dom.className += ' ' + enabledClass;
const firstElementByClass = className => {
const list = dom.getElementsByClassName(className);
if (list === null) {
throw 'error: could not find element with class ' + className + ' within element with ID ' + id + ' in the DOM';
}
return list[0];
};
const createCounter = className => {
const dom = firstElementByClass(className);
if (dom === undefined) {
return {
count: () => undefined,
set: Utils.noop
};
}
return {
count: () => parseInt(dom.innerHTML || 0, 10),
set: value => dom.innerHTML = value
};
};
const createToggle = (className, activeClassName) => {
const createClasses = () => {
const classes = {
[className]: true,
[activeClassName]: false,
};
item.className.split(/ +/)
.filter(x => x)
.forEach(className => classes[className] = true);
return classes;
};
const formatClassName = () => {
return Object.entries(classes)
.filter(e => e[1])
.map(e => e[0])
.join(' ');
};
const item = firstElementByClass(className);
if (item === undefined) {
return {
get: () => false,
set: Utils.noop,
onClick: Utils.noop
};
}
const classes = createClasses();
return {
get: () => classes[activeClassName],
set: value => {
classes[activeClassName] = value;
item.className = formatClassName();
},
onClick: fun => item.onclick = fun
};
};
const render = model => {
counter.set(model.count());
upvote.set(model.upvoted());
downvote.set(model.downvoted());
star.set(model.starred());
};
const parseParamsFromDom = () => {
return {
count: counter.count(),
upvoted: upvote.get(),
downvoted: downvote.get(),
starred: star.get()
};
};
const destroy = () => {
Utils.removeClass(dom, enabledClass);
upvote.onClick(null);
downvote.onClick(null);
star.onClick(null);
};
const counter = createCounter(countClass);
const upvote = createToggle(upvoteClass, upvoteOnClass);
const downvote = createToggle(downvoteClass, downvoteOnClass);
const star = createToggle(starClass, starOnClass);
return {
render: render,
parseParamsFromDom: parseParamsFromDom,
onClickUpvote: fun => upvote.onClick(fun),
onClickDownvote: fun => downvote.onClick(fun),
onClickStar: fun => star.onClick(fun),
destroy: destroy
};
};
return {
create: create
};
}();
const create = (id, params = {}) => {
var destroyed = false;
const view = View.create(id);
const domParams = view.parseParamsFromDom();
const defaults = {
id: id,
count: 0,
upvoted: false,
downvoted: false,
starred: false,
callback: () => {}
};
const combinedParams = Utils.combine(defaults, domParams, params);
const model = Model.create(combinedParams);
const callback = combinedParams.callback;
const throwIfDestroyed = () => {
if (destroyed) {
throw "fatal: unexpected call to destroyed controller";
}
};
const upvote = () => {
throwIfDestroyed();
model.upvote();
view.render(model);
callback(model.data());
};
const downvote = () => {
throwIfDestroyed();
model.downvote();
view.render(model);
callback(model.data());
};
const star = () => {
throwIfDestroyed();
model.star();
view.render(model);
callback(model.data());
};
const destroy = () => {
throwIfDestroyed();
destroyed = true;
view.destroy();
};
view.render(model);
view.onClickUpvote(upvote);
view.onClickDownvote(downvote);
view.onClickStar(star);
return {
id: id,
count: () => {
throwIfDestroyed();
return model.count();
},
upvote: upvote,
upvoted: () => {
throwIfDestroyed();
return model.upvoted();
},
downvote: downvote,
downvoted: () => {
throwIfDestroyed();
return model.downvoted();
},
star: star,
starred: () => {
throwIfDestroyed();
return model.starred();
},
destroy: destroy
};
};
return {
create: create
};
}();
Unit tests
I'm also interested in a review of the unit tests (live tests of current version):
const create = (id, params) => {
return Upvote.create(id, params);
};
QUnit.test('throw exception if destroy is called twice', assert => {
const obj = gen();
obj.destroy();
assert.throws(() => obj.destroy());
});
const gen = function() {
var idcount = 0;
return (params = {}) => {
++idcount;
const id = params.id || ('u' + idcount);
const jqdom = $('#templates div.upvotejs').clone();
jqdom.attr('id', id);
$('#tests').append(jqdom);
params.callback = params.callback || (data => {});
return create(id, params, jqdom);
};
}();
const uiTester = obj => {
const widget = $('#' + obj.id);
const count = widget.find('.count');
const upvote = widget.find('.upvote');
const downvote = widget.find('.downvote');
const star = widget.find('.star');
return {
count: () => parseInt(count.text(), 10),
upvoted: () => upvote.hasClass('upvote-on'),
downvoted: () => downvote.hasClass('downvote-on'),
starred: () => star.hasClass('star-on'),
upvote: () => upvote.click(),
downvote: () => downvote.click(),
star: () => star.click()
};
};
QUnit.test('initialize from params', assert => {
const obj = gen();
assert.equal(obj.count(), 0);
assert.equal(obj.upvoted(), false);
assert.equal(obj.downvoted(), false);
assert.equal(obj.starred(), false);
assert.equal(gen({count: 17}).count(), 17);
assert.equal(gen({upvoted: true}).upvoted(), true);
assert.equal(gen({downvoted: true}).downvoted(), true);
assert.equal(gen({starred: true}).starred(), true);
assert.throws(() => gen({count: 'foo'}), 'throw if count param is not an integer');
assert.throws(() => gen({upvoted: 'foo'}), 'throw if upvoted param is not a boolean');
assert.throws(() => gen({downvoted: 'foo'}), 'throw if downvoted param is not a boolean');
assert.throws(() => gen({starred: 'foo'}), 'throw if starred param is not a boolean');
assert.throws(() => gen({callback: 'foo'}), 'throw if callback param is not a function');
assert.throws(() => gen({upvoted: true, downvoted: true}), 'throw if upvoted=true and downvoted=true');
});
QUnit.test('initialize from dom', assert => {
const v1 = Upvote.create('count-1');
assert.equal(v1.count(), 1);
assert.equal(v1.upvoted(), false);
assert.equal(v1.downvoted(), false);
assert.equal(v1.starred(), false);
const v2 = Upvote.create('count-2-upvoted');
assert.equal(v2.count(), 2);
assert.equal(v2.upvoted(), true);
assert.equal(v2.downvoted(), false);
assert.equal(v2.starred(), false);
const v3 = Upvote.create('count-3-upvoted-starred');
assert.equal(v3.count(), 3);
assert.equal(v3.upvoted(), true);
assert.equal(v3.downvoted(), false);
assert.equal(v3.starred(), true);
const v4 = Upvote.create('count-4-downvoted');
assert.equal(v4.count(), 4);
assert.equal(v4.upvoted(), false);
assert.equal(v4.downvoted(), true);
assert.equal(v4.starred(), false);
const v5 = Upvote.create('count-5-downvoted-starred');
assert.equal(v5.count(), 5);
assert.equal(v5.upvoted(), false);
assert.equal(v5.downvoted(), true);
assert.equal(v5.starred(), true);
const vLarge = Upvote.create('count-456789');
assert.equal(vLarge.count(), 456789);
assert.equal(vLarge.upvoted(), false);
assert.equal(vLarge.downvoted(), false);
assert.equal(vLarge.starred(), false);
const vNegativeLarge = Upvote.create('count-minus-456789');
assert.equal(vNegativeLarge.count(), -456789);
assert.equal(vNegativeLarge.upvoted(), false);
assert.equal(vNegativeLarge.downvoted(), false);
assert.equal(vNegativeLarge.starred(), false);
assert.throws(() => Upvote.create('upvoted-downvoted'));
});
QUnit.test('UI updated from params', assert => {
const obj = uiTester(gen());
assert.equal(obj.count(), 0);
assert.equal(obj.upvoted(), false);
assert.equal(obj.downvoted(), false);
assert.equal(obj.starred(), false);
assert.equal(uiTester(gen({count: 17})).count(), 17);
assert.equal(uiTester(gen({upvoted: true})).upvoted(), true);
assert.equal(uiTester(gen({downvoted: true})).downvoted(), true);
assert.equal(uiTester(gen({starred: true})).starred(), true);
});
QUnit.test('upvote non-downvoted non-upvoted', assert => {
const count = 5;
const obj = gen({count: count});
assert.equal(obj.count(), count);
obj.upvote();
assert.equal(obj.count(), count + 1);
});
QUnit.test('upvote downvoted', assert => {
const count = 6;
const obj = gen({count: count, downvoted: true});
assert.equal(obj.count(), count);
obj.upvote();
assert.equal(obj.count(), count + 2);
});
QUnit.test('upvote upvoted', assert => {
const count = 7;
const obj = gen({count: count, upvoted: true});
assert.equal(obj.count(), count);
obj.upvote();
assert.equal(obj.count(), count - 1);
});
QUnit.test('downvote non-downvoted non-upvoted', assert => {
const count = 5;
const obj = gen({count: count});
assert.equal(obj.count(), count);
obj.downvote();
assert.equal(obj.count(), count - 1);
});
QUnit.test('downvote upvoted', assert => {
const count = 6;
const obj = gen({count: count, upvoted: true});
assert.equal(obj.count(), count);
obj.downvote();
assert.equal(obj.count(), count - 2);
});
QUnit.test('downvote downvoted', assert => {
const count = 7;
const obj = gen({count: count, downvoted: true});
assert.equal(obj.count(), count);
obj.downvote();
assert.equal(obj.count(), count + 1);
});
QUnit.test('star non-starred', assert => {
const obj = gen();
obj.star();
assert.ok(obj.starred(), 'should be starred');
});
QUnit.test('star starred', assert => {
const obj = gen({starred: true});
obj.star();
assert.ok(!obj.starred(), 'should not be starred');
});
QUnit.test('upvote indepently', assert => {
const count1 = 5;
const v1 = gen({count: count1});
const count2 = 5;
const v2 = gen({count: count2});
v1.upvote();
assert.equal(v1.count(), count1 + 1);
assert.equal(v2.count(), count2);
});
QUnit.test('downvote indepently', assert => {
const count1 = 5;
const v1 = gen({count: count1});
const count2 = 5;
const v2 = gen({count: count2});
v1.downvote();
assert.equal(v1.count(), count1 - 1);
assert.equal(v2.count(), count2);
});
QUnit.test('star indepently', assert => {
const v1 = gen();
const v2 = gen();
v1.star();
assert.equal(v1.starred(), true);
assert.equal(v2.starred(), false);
});
QUnit.test('call callback on value changes', assert => {
var receivedPayload;
const callback = payload => receivedPayload = payload;
const obj1_id = 100;
const obj1_origCount = 10;
const obj1 = gen({id: obj1_id, count: obj1_origCount, callback: callback});
const obj2_id = 200;
const obj2_origCount = 20;
const obj2 = gen({id: obj2_id, count: obj2_origCount, callback: callback});
obj1.upvote();
assert.deepEqual(receivedPayload, {
id: obj1_id,
action: 'upvote',
newState: {
count: obj1_origCount + 1,
downvoted: false,
upvoted: true,
starred: false
}
});
obj2.upvote();
assert.deepEqual(receivedPayload, {
id: obj2_id,
action: 'upvote',
newState: {
count: obj2_origCount + 1,
downvoted: false,
upvoted: true,
starred: false
}
});
obj1.upvote();
assert.deepEqual(receivedPayload, {
id: obj1_id,
action: 'unupvote',
newState: {
count: obj1_origCount,
downvoted: false,
upvoted: false,
starred: false
}
});
obj2.star();
assert.deepEqual(receivedPayload, {
id: obj2_id,
action: 'star',
newState: {
count: obj2_origCount + 1,
downvoted: false,
upvoted: true,
starred: true
}
});
obj2.star();
assert.deepEqual(receivedPayload, {
id: obj2_id,
action: 'unstar',
newState: {
count: obj2_origCount + 1,
downvoted: false,
upvoted: true,
starred: false
}
});
obj2.downvote();
assert.deepEqual(receivedPayload, {
id: obj2_id,
action: 'downvote',
newState: {
count: obj2_origCount - 1,
downvoted: true,
upvoted: false,
starred: false
}
});
obj2.downvote();
assert.deepEqual(receivedPayload, {
id: obj2_id,
action: 'undownvote',
newState: {
count: obj2_origCount,
downvoted: false,
upvoted: false,
starred: false
}
});
});
QUnit.test('update model updates UI', assert => {
const obj = gen();
const ui = uiTester(obj);
obj.upvote();
assert.equal(ui.count(), 1);
assert.equal(ui.upvoted(), true);
obj.downvote();
assert.equal(ui.count(), -1);
assert.equal(ui.upvoted(), false);
assert.equal(ui.downvoted(), true);
obj.upvote();
assert.equal(ui.count(), 1);
assert.equal(ui.upvoted(), true);
assert.equal(ui.downvoted(), false);
obj.star();
assert.equal(ui.starred(), true);
obj.star();
assert.equal(ui.starred(), false);
});
QUnit.test('update UI updates model', assert => {
const obj = gen();
const ui = uiTester(obj);
ui.upvote();
assert.equal(obj.count(), 1);
assert.equal(obj.upvoted(), true);
ui.downvote();
assert.equal(obj.count(), -1);
assert.equal(obj.upvoted(), false);
assert.equal(obj.downvoted(), true);
ui.upvote();
assert.equal(obj.count(), 1);
assert.equal(obj.upvoted(), true);
assert.equal(obj.downvoted(), false);
ui.star();
assert.equal(obj.starred(), true);
ui.star();
assert.equal(obj.starred(), false);
});
QUnit.test('cannot associate multiple models to the same id', assert => {
const orig = gen();
assert.throws(() => gen({id: orig.id}));
});
QUnit.test('widget stops responding to clicks after destroyed', assert => {
const obj = gen({count: 99});
const ui = uiTester(obj);
ui.upvote();
assert.equal(ui.count(), 100);
ui.upvote();
assert.equal(ui.count(), 99);
obj.destroy();
ui.upvote();
assert.equal(ui.count(), 99);
assert.throws(() => obj.upvote());
assert.throws(() => obj.downvote());
assert.throws(() => obj.star());
assert.throws(() => obj.count());
assert.throws(() => obj.upvoted());
assert.throws(() => obj.downvoted());
assert.throws(() => obj.starred());
const reused = gen({id: obj.id});
assert.equal(reused.count(), 99);
ui.upvote();
assert.equal(reused.count(), 100);
});
QUnit.test('all sub-elements (upvote/downvote/count/star) are optional in the HTML markup', assert => {
['upvote', 'downvote', 'count', 'star'].forEach(cls => {
const obj0 = gen();
obj0.destroy();
const jqdom = $('#' + obj0.id);
jqdom.find('.' + cls).remove();
const obj = create(obj0.id, {}, jqdom);
assert.equal(obj.count(), 0);
obj.upvote();
assert.equal(obj.count(), 1);
assert.equal(obj.upvoted(), true);
obj.downvote();
assert.equal(obj.count(), -1);
assert.equal(obj.downvoted(), true);
assert.equal(obj.upvoted(), false);
obj.downvote();
assert.equal(obj.count(), 0);
assert.equal(obj.downvoted(), false);
obj.star();
assert.equal(obj.starred(), true);
obj.star();
assert.equal(obj.starred(), false);
});
}
Code review
I'm looking for a review of the posted code in any and all aspects.
Some particular areas do come to mind I'm especially curious about:
- Bad practices: is there anything to fix
- Usability: is there anything that looks hard to use, and how to make it better
- Are the tests sufficient? Did I miss any important cases?
- Are the tests overcomplicated? Do you see ways to simplify?
- I'm used to using QUnit for testing JavaScript packages. Do you think I could benefit greatly by using something else?
javascript
$endgroup$
add a comment |
$begingroup$
I implemented a simple voting widget like the one used on Stack Exchange sites, to use for other purposes (see live on bashoneliners.com), as a reusable package, dubbed UpvoteJS.
Here's how it works:
Upvote.create('topic-123');
<link rel="stylesheet" href="https://janosgyerik.github.io/upvotejs/dist/upvotejs/upvotejs.css">
<script src="https://janosgyerik.github.io/upvotejs/dist/upvotejs/upvotejs.vanilla.js"></script>
<div id="topic-123" class="upvotejs upvotejs-serverfault">
<a class="upvote upvote-on"></a>
<span class="count">101</span>
<a class="downvote"></a>
<a class="star star-on"></a>
</div>
How to use
As you can see in the above snippet, the typical use has the following elements:
Include the stylesheet
upvotejs.css
with a<link>
tag. The companionupvotejs.svg
is expected in the same directory on the web server.Add HTML markup with the fields you need. With the stylesheet included, this is enough to get a nicely rendered widget, read-only, since without JavaScript, the clicks will have no effect.
To make the widget interactive, responding to user clicks, include the JavaScript package
upvotejs.vanilla.js
with a<script>
tag, and activate the widget using the providedUpvote.create(topicId)
function in JavaScript (to happen after the DOM is loaded). ThetopicId
parameter is mandatory, and it must refer to a unique id in the DOM.To make the widget save state to a backend (not featured in the snippet), pass to
Upvote.create
as second parameter a JSON object, with a field namedcallback
, which must be a JavaScript function, for example:Upvote.create(topicId, {callback: your_callback_function})
. It is up to the developer to implementyour_callback_function
to persist its payload. The function will get called on any state change.
The above should be enough for typical use cases.
Additional notes:
All sub-elements in the HTML markup are optional, for example it's possible to have a widget without a star button, or any other element.
The initial state of a widget can be set either with HTML markup (recommended), or using basic markup (without
upvote-on
,downvote-on
,star-on
classes) and setting values in the second parameter of theUpvote.create
call, for example:Upvote.create(topicId, {count: 123, upvoted: true})
Upvote.create
does some sanity checks, and throws an exception when it detects illegal state, for example (not an exhaustive list):
- The specified ID doesn't exist in the DOM
- The parameter types in the JSON object don't match what's expected (
count
must be integer, and so on)
upvoted
anddownvoted
are bothtrue
Upvote.create
returns an object that represents the widget, and can be used to inspect and control its state. In the typical use case, this is probably not necessary.
Implementation
I'm most interested in a review of the JavaScript part (upvotejs.vanilla.js
(2.1.0)),
the complete project is available on GitHub.
const Upvote = function() {
const upvoteClass = 'upvote';
const enabledClass = 'upvotejs-enabled';
const upvoteOnClass = 'upvote-on';
const downvoteClass = 'downvote';
const downvoteOnClass = 'downvote-on';
const starClass = 'star';
const starOnClass = 'star-on';
const countClass = 'count';
const Utils = {
combine: function() {
const combined = {};
for (let i = 0; i < arguments.length; i++) {
Object.entries(arguments[i])
.filter(e => e[1] !== undefined)
.forEach(e => combined[e[0]] = e[1]);
}
return combined;
},
isBoolean: v => typeof v === "boolean",
isFunction: v => typeof v === "function",
classes: dom => dom.className.split(/ +/).filter(x => x),
removeClass: (dom, className) => {
dom.className = dom.className.split(/ +/)
.filter(x => x)
.filter(c => c !== className)
.join(' ');
},
noop: () => {}
};
const Model = function() {
const validate = params => {
if (!Number.isInteger(params.count)) {
throw 'error: parameter "count" must be a valid integer';
}
if (!Utils.isBoolean(params.upvoted)) {
throw 'error: parameter "upvoted" must be a boolean';
}
if (!Utils.isBoolean(params.downvoted)) {
throw 'error: parameter "downvoted" must be a boolean';
}
if (!Utils.isBoolean(params.starred)) {
throw 'error: parameter "starred" must be a boolean';
}
if (params.callback && !Utils.isFunction(params.callback)) {
throw 'error: parameter "callback" must be a function';
}
if (params.upvoted && params.downvoted) {
throw 'error: parameters "upvoted" and "downvoted" must not be true at the same time';
}
};
const create = params => {
validate(params);
const data = Utils.combine(params);
const upvote = () => {
if (data.upvoted) {
data.count--;
} else {
data.count++;
if (data.downvoted) {
data.downvoted = false;
data.count++;
}
}
data.upvoted = !data.upvoted;
};
const downvote = () => {
if (data.downvoted) {
data.count++;
} else {
data.count--;
if (data.upvoted) {
data.upvoted = false;
data.count--;
}
}
data.downvoted = !data.downvoted;
};
return {
count: () => data.count,
upvote: upvote,
upvoted: () => data.upvoted,
downvote: downvote,
downvoted: () => data.downvoted,
star: () => data.starred = !data.starred,
starred: () => data.starred,
data: () => Utils.combine(data)
};
};
return {
create: create
};
}();
const View = function() {
const create = id => {
const dom = document.getElementById(id);
if (dom === null) {
throw 'error: could not find element with ID ' + id + ' in the DOM';
}
if (Utils.classes(dom).includes(enabledClass)) {
throw 'error: element with ID ' + id + ' is already in use by another upvote controller';
}
dom.className += ' ' + enabledClass;
const firstElementByClass = className => {
const list = dom.getElementsByClassName(className);
if (list === null) {
throw 'error: could not find element with class ' + className + ' within element with ID ' + id + ' in the DOM';
}
return list[0];
};
const createCounter = className => {
const dom = firstElementByClass(className);
if (dom === undefined) {
return {
count: () => undefined,
set: Utils.noop
};
}
return {
count: () => parseInt(dom.innerHTML || 0, 10),
set: value => dom.innerHTML = value
};
};
const createToggle = (className, activeClassName) => {
const createClasses = () => {
const classes = {
[className]: true,
[activeClassName]: false,
};
item.className.split(/ +/)
.filter(x => x)
.forEach(className => classes[className] = true);
return classes;
};
const formatClassName = () => {
return Object.entries(classes)
.filter(e => e[1])
.map(e => e[0])
.join(' ');
};
const item = firstElementByClass(className);
if (item === undefined) {
return {
get: () => false,
set: Utils.noop,
onClick: Utils.noop
};
}
const classes = createClasses();
return {
get: () => classes[activeClassName],
set: value => {
classes[activeClassName] = value;
item.className = formatClassName();
},
onClick: fun => item.onclick = fun
};
};
const render = model => {
counter.set(model.count());
upvote.set(model.upvoted());
downvote.set(model.downvoted());
star.set(model.starred());
};
const parseParamsFromDom = () => {
return {
count: counter.count(),
upvoted: upvote.get(),
downvoted: downvote.get(),
starred: star.get()
};
};
const destroy = () => {
Utils.removeClass(dom, enabledClass);
upvote.onClick(null);
downvote.onClick(null);
star.onClick(null);
};
const counter = createCounter(countClass);
const upvote = createToggle(upvoteClass, upvoteOnClass);
const downvote = createToggle(downvoteClass, downvoteOnClass);
const star = createToggle(starClass, starOnClass);
return {
render: render,
parseParamsFromDom: parseParamsFromDom,
onClickUpvote: fun => upvote.onClick(fun),
onClickDownvote: fun => downvote.onClick(fun),
onClickStar: fun => star.onClick(fun),
destroy: destroy
};
};
return {
create: create
};
}();
const create = (id, params = {}) => {
var destroyed = false;
const view = View.create(id);
const domParams = view.parseParamsFromDom();
const defaults = {
id: id,
count: 0,
upvoted: false,
downvoted: false,
starred: false,
callback: () => {}
};
const combinedParams = Utils.combine(defaults, domParams, params);
const model = Model.create(combinedParams);
const callback = combinedParams.callback;
const throwIfDestroyed = () => {
if (destroyed) {
throw "fatal: unexpected call to destroyed controller";
}
};
const upvote = () => {
throwIfDestroyed();
model.upvote();
view.render(model);
callback(model.data());
};
const downvote = () => {
throwIfDestroyed();
model.downvote();
view.render(model);
callback(model.data());
};
const star = () => {
throwIfDestroyed();
model.star();
view.render(model);
callback(model.data());
};
const destroy = () => {
throwIfDestroyed();
destroyed = true;
view.destroy();
};
view.render(model);
view.onClickUpvote(upvote);
view.onClickDownvote(downvote);
view.onClickStar(star);
return {
id: id,
count: () => {
throwIfDestroyed();
return model.count();
},
upvote: upvote,
upvoted: () => {
throwIfDestroyed();
return model.upvoted();
},
downvote: downvote,
downvoted: () => {
throwIfDestroyed();
return model.downvoted();
},
star: star,
starred: () => {
throwIfDestroyed();
return model.starred();
},
destroy: destroy
};
};
return {
create: create
};
}();
Unit tests
I'm also interested in a review of the unit tests (live tests of current version):
const create = (id, params) => {
return Upvote.create(id, params);
};
QUnit.test('throw exception if destroy is called twice', assert => {
const obj = gen();
obj.destroy();
assert.throws(() => obj.destroy());
});
const gen = function() {
var idcount = 0;
return (params = {}) => {
++idcount;
const id = params.id || ('u' + idcount);
const jqdom = $('#templates div.upvotejs').clone();
jqdom.attr('id', id);
$('#tests').append(jqdom);
params.callback = params.callback || (data => {});
return create(id, params, jqdom);
};
}();
const uiTester = obj => {
const widget = $('#' + obj.id);
const count = widget.find('.count');
const upvote = widget.find('.upvote');
const downvote = widget.find('.downvote');
const star = widget.find('.star');
return {
count: () => parseInt(count.text(), 10),
upvoted: () => upvote.hasClass('upvote-on'),
downvoted: () => downvote.hasClass('downvote-on'),
starred: () => star.hasClass('star-on'),
upvote: () => upvote.click(),
downvote: () => downvote.click(),
star: () => star.click()
};
};
QUnit.test('initialize from params', assert => {
const obj = gen();
assert.equal(obj.count(), 0);
assert.equal(obj.upvoted(), false);
assert.equal(obj.downvoted(), false);
assert.equal(obj.starred(), false);
assert.equal(gen({count: 17}).count(), 17);
assert.equal(gen({upvoted: true}).upvoted(), true);
assert.equal(gen({downvoted: true}).downvoted(), true);
assert.equal(gen({starred: true}).starred(), true);
assert.throws(() => gen({count: 'foo'}), 'throw if count param is not an integer');
assert.throws(() => gen({upvoted: 'foo'}), 'throw if upvoted param is not a boolean');
assert.throws(() => gen({downvoted: 'foo'}), 'throw if downvoted param is not a boolean');
assert.throws(() => gen({starred: 'foo'}), 'throw if starred param is not a boolean');
assert.throws(() => gen({callback: 'foo'}), 'throw if callback param is not a function');
assert.throws(() => gen({upvoted: true, downvoted: true}), 'throw if upvoted=true and downvoted=true');
});
QUnit.test('initialize from dom', assert => {
const v1 = Upvote.create('count-1');
assert.equal(v1.count(), 1);
assert.equal(v1.upvoted(), false);
assert.equal(v1.downvoted(), false);
assert.equal(v1.starred(), false);
const v2 = Upvote.create('count-2-upvoted');
assert.equal(v2.count(), 2);
assert.equal(v2.upvoted(), true);
assert.equal(v2.downvoted(), false);
assert.equal(v2.starred(), false);
const v3 = Upvote.create('count-3-upvoted-starred');
assert.equal(v3.count(), 3);
assert.equal(v3.upvoted(), true);
assert.equal(v3.downvoted(), false);
assert.equal(v3.starred(), true);
const v4 = Upvote.create('count-4-downvoted');
assert.equal(v4.count(), 4);
assert.equal(v4.upvoted(), false);
assert.equal(v4.downvoted(), true);
assert.equal(v4.starred(), false);
const v5 = Upvote.create('count-5-downvoted-starred');
assert.equal(v5.count(), 5);
assert.equal(v5.upvoted(), false);
assert.equal(v5.downvoted(), true);
assert.equal(v5.starred(), true);
const vLarge = Upvote.create('count-456789');
assert.equal(vLarge.count(), 456789);
assert.equal(vLarge.upvoted(), false);
assert.equal(vLarge.downvoted(), false);
assert.equal(vLarge.starred(), false);
const vNegativeLarge = Upvote.create('count-minus-456789');
assert.equal(vNegativeLarge.count(), -456789);
assert.equal(vNegativeLarge.upvoted(), false);
assert.equal(vNegativeLarge.downvoted(), false);
assert.equal(vNegativeLarge.starred(), false);
assert.throws(() => Upvote.create('upvoted-downvoted'));
});
QUnit.test('UI updated from params', assert => {
const obj = uiTester(gen());
assert.equal(obj.count(), 0);
assert.equal(obj.upvoted(), false);
assert.equal(obj.downvoted(), false);
assert.equal(obj.starred(), false);
assert.equal(uiTester(gen({count: 17})).count(), 17);
assert.equal(uiTester(gen({upvoted: true})).upvoted(), true);
assert.equal(uiTester(gen({downvoted: true})).downvoted(), true);
assert.equal(uiTester(gen({starred: true})).starred(), true);
});
QUnit.test('upvote non-downvoted non-upvoted', assert => {
const count = 5;
const obj = gen({count: count});
assert.equal(obj.count(), count);
obj.upvote();
assert.equal(obj.count(), count + 1);
});
QUnit.test('upvote downvoted', assert => {
const count = 6;
const obj = gen({count: count, downvoted: true});
assert.equal(obj.count(), count);
obj.upvote();
assert.equal(obj.count(), count + 2);
});
QUnit.test('upvote upvoted', assert => {
const count = 7;
const obj = gen({count: count, upvoted: true});
assert.equal(obj.count(), count);
obj.upvote();
assert.equal(obj.count(), count - 1);
});
QUnit.test('downvote non-downvoted non-upvoted', assert => {
const count = 5;
const obj = gen({count: count});
assert.equal(obj.count(), count);
obj.downvote();
assert.equal(obj.count(), count - 1);
});
QUnit.test('downvote upvoted', assert => {
const count = 6;
const obj = gen({count: count, upvoted: true});
assert.equal(obj.count(), count);
obj.downvote();
assert.equal(obj.count(), count - 2);
});
QUnit.test('downvote downvoted', assert => {
const count = 7;
const obj = gen({count: count, downvoted: true});
assert.equal(obj.count(), count);
obj.downvote();
assert.equal(obj.count(), count + 1);
});
QUnit.test('star non-starred', assert => {
const obj = gen();
obj.star();
assert.ok(obj.starred(), 'should be starred');
});
QUnit.test('star starred', assert => {
const obj = gen({starred: true});
obj.star();
assert.ok(!obj.starred(), 'should not be starred');
});
QUnit.test('upvote indepently', assert => {
const count1 = 5;
const v1 = gen({count: count1});
const count2 = 5;
const v2 = gen({count: count2});
v1.upvote();
assert.equal(v1.count(), count1 + 1);
assert.equal(v2.count(), count2);
});
QUnit.test('downvote indepently', assert => {
const count1 = 5;
const v1 = gen({count: count1});
const count2 = 5;
const v2 = gen({count: count2});
v1.downvote();
assert.equal(v1.count(), count1 - 1);
assert.equal(v2.count(), count2);
});
QUnit.test('star indepently', assert => {
const v1 = gen();
const v2 = gen();
v1.star();
assert.equal(v1.starred(), true);
assert.equal(v2.starred(), false);
});
QUnit.test('call callback on value changes', assert => {
var receivedPayload;
const callback = payload => receivedPayload = payload;
const obj1_id = 100;
const obj1_origCount = 10;
const obj1 = gen({id: obj1_id, count: obj1_origCount, callback: callback});
const obj2_id = 200;
const obj2_origCount = 20;
const obj2 = gen({id: obj2_id, count: obj2_origCount, callback: callback});
obj1.upvote();
assert.deepEqual(receivedPayload, {
id: obj1_id,
action: 'upvote',
newState: {
count: obj1_origCount + 1,
downvoted: false,
upvoted: true,
starred: false
}
});
obj2.upvote();
assert.deepEqual(receivedPayload, {
id: obj2_id,
action: 'upvote',
newState: {
count: obj2_origCount + 1,
downvoted: false,
upvoted: true,
starred: false
}
});
obj1.upvote();
assert.deepEqual(receivedPayload, {
id: obj1_id,
action: 'unupvote',
newState: {
count: obj1_origCount,
downvoted: false,
upvoted: false,
starred: false
}
});
obj2.star();
assert.deepEqual(receivedPayload, {
id: obj2_id,
action: 'star',
newState: {
count: obj2_origCount + 1,
downvoted: false,
upvoted: true,
starred: true
}
});
obj2.star();
assert.deepEqual(receivedPayload, {
id: obj2_id,
action: 'unstar',
newState: {
count: obj2_origCount + 1,
downvoted: false,
upvoted: true,
starred: false
}
});
obj2.downvote();
assert.deepEqual(receivedPayload, {
id: obj2_id,
action: 'downvote',
newState: {
count: obj2_origCount - 1,
downvoted: true,
upvoted: false,
starred: false
}
});
obj2.downvote();
assert.deepEqual(receivedPayload, {
id: obj2_id,
action: 'undownvote',
newState: {
count: obj2_origCount,
downvoted: false,
upvoted: false,
starred: false
}
});
});
QUnit.test('update model updates UI', assert => {
const obj = gen();
const ui = uiTester(obj);
obj.upvote();
assert.equal(ui.count(), 1);
assert.equal(ui.upvoted(), true);
obj.downvote();
assert.equal(ui.count(), -1);
assert.equal(ui.upvoted(), false);
assert.equal(ui.downvoted(), true);
obj.upvote();
assert.equal(ui.count(), 1);
assert.equal(ui.upvoted(), true);
assert.equal(ui.downvoted(), false);
obj.star();
assert.equal(ui.starred(), true);
obj.star();
assert.equal(ui.starred(), false);
});
QUnit.test('update UI updates model', assert => {
const obj = gen();
const ui = uiTester(obj);
ui.upvote();
assert.equal(obj.count(), 1);
assert.equal(obj.upvoted(), true);
ui.downvote();
assert.equal(obj.count(), -1);
assert.equal(obj.upvoted(), false);
assert.equal(obj.downvoted(), true);
ui.upvote();
assert.equal(obj.count(), 1);
assert.equal(obj.upvoted(), true);
assert.equal(obj.downvoted(), false);
ui.star();
assert.equal(obj.starred(), true);
ui.star();
assert.equal(obj.starred(), false);
});
QUnit.test('cannot associate multiple models to the same id', assert => {
const orig = gen();
assert.throws(() => gen({id: orig.id}));
});
QUnit.test('widget stops responding to clicks after destroyed', assert => {
const obj = gen({count: 99});
const ui = uiTester(obj);
ui.upvote();
assert.equal(ui.count(), 100);
ui.upvote();
assert.equal(ui.count(), 99);
obj.destroy();
ui.upvote();
assert.equal(ui.count(), 99);
assert.throws(() => obj.upvote());
assert.throws(() => obj.downvote());
assert.throws(() => obj.star());
assert.throws(() => obj.count());
assert.throws(() => obj.upvoted());
assert.throws(() => obj.downvoted());
assert.throws(() => obj.starred());
const reused = gen({id: obj.id});
assert.equal(reused.count(), 99);
ui.upvote();
assert.equal(reused.count(), 100);
});
QUnit.test('all sub-elements (upvote/downvote/count/star) are optional in the HTML markup', assert => {
['upvote', 'downvote', 'count', 'star'].forEach(cls => {
const obj0 = gen();
obj0.destroy();
const jqdom = $('#' + obj0.id);
jqdom.find('.' + cls).remove();
const obj = create(obj0.id, {}, jqdom);
assert.equal(obj.count(), 0);
obj.upvote();
assert.equal(obj.count(), 1);
assert.equal(obj.upvoted(), true);
obj.downvote();
assert.equal(obj.count(), -1);
assert.equal(obj.downvoted(), true);
assert.equal(obj.upvoted(), false);
obj.downvote();
assert.equal(obj.count(), 0);
assert.equal(obj.downvoted(), false);
obj.star();
assert.equal(obj.starred(), true);
obj.star();
assert.equal(obj.starred(), false);
});
}
Code review
I'm looking for a review of the posted code in any and all aspects.
Some particular areas do come to mind I'm especially curious about:
- Bad practices: is there anything to fix
- Usability: is there anything that looks hard to use, and how to make it better
- Are the tests sufficient? Did I miss any important cases?
- Are the tests overcomplicated? Do you see ways to simplify?
- I'm used to using QUnit for testing JavaScript packages. Do you think I could benefit greatly by using something else?
javascript
$endgroup$
add a comment |
$begingroup$
I implemented a simple voting widget like the one used on Stack Exchange sites, to use for other purposes (see live on bashoneliners.com), as a reusable package, dubbed UpvoteJS.
Here's how it works:
Upvote.create('topic-123');
<link rel="stylesheet" href="https://janosgyerik.github.io/upvotejs/dist/upvotejs/upvotejs.css">
<script src="https://janosgyerik.github.io/upvotejs/dist/upvotejs/upvotejs.vanilla.js"></script>
<div id="topic-123" class="upvotejs upvotejs-serverfault">
<a class="upvote upvote-on"></a>
<span class="count">101</span>
<a class="downvote"></a>
<a class="star star-on"></a>
</div>
How to use
As you can see in the above snippet, the typical use has the following elements:
Include the stylesheet
upvotejs.css
with a<link>
tag. The companionupvotejs.svg
is expected in the same directory on the web server.Add HTML markup with the fields you need. With the stylesheet included, this is enough to get a nicely rendered widget, read-only, since without JavaScript, the clicks will have no effect.
To make the widget interactive, responding to user clicks, include the JavaScript package
upvotejs.vanilla.js
with a<script>
tag, and activate the widget using the providedUpvote.create(topicId)
function in JavaScript (to happen after the DOM is loaded). ThetopicId
parameter is mandatory, and it must refer to a unique id in the DOM.To make the widget save state to a backend (not featured in the snippet), pass to
Upvote.create
as second parameter a JSON object, with a field namedcallback
, which must be a JavaScript function, for example:Upvote.create(topicId, {callback: your_callback_function})
. It is up to the developer to implementyour_callback_function
to persist its payload. The function will get called on any state change.
The above should be enough for typical use cases.
Additional notes:
All sub-elements in the HTML markup are optional, for example it's possible to have a widget without a star button, or any other element.
The initial state of a widget can be set either with HTML markup (recommended), or using basic markup (without
upvote-on
,downvote-on
,star-on
classes) and setting values in the second parameter of theUpvote.create
call, for example:Upvote.create(topicId, {count: 123, upvoted: true})
Upvote.create
does some sanity checks, and throws an exception when it detects illegal state, for example (not an exhaustive list):
- The specified ID doesn't exist in the DOM
- The parameter types in the JSON object don't match what's expected (
count
must be integer, and so on)
upvoted
anddownvoted
are bothtrue
Upvote.create
returns an object that represents the widget, and can be used to inspect and control its state. In the typical use case, this is probably not necessary.
Implementation
I'm most interested in a review of the JavaScript part (upvotejs.vanilla.js
(2.1.0)),
the complete project is available on GitHub.
const Upvote = function() {
const upvoteClass = 'upvote';
const enabledClass = 'upvotejs-enabled';
const upvoteOnClass = 'upvote-on';
const downvoteClass = 'downvote';
const downvoteOnClass = 'downvote-on';
const starClass = 'star';
const starOnClass = 'star-on';
const countClass = 'count';
const Utils = {
combine: function() {
const combined = {};
for (let i = 0; i < arguments.length; i++) {
Object.entries(arguments[i])
.filter(e => e[1] !== undefined)
.forEach(e => combined[e[0]] = e[1]);
}
return combined;
},
isBoolean: v => typeof v === "boolean",
isFunction: v => typeof v === "function",
classes: dom => dom.className.split(/ +/).filter(x => x),
removeClass: (dom, className) => {
dom.className = dom.className.split(/ +/)
.filter(x => x)
.filter(c => c !== className)
.join(' ');
},
noop: () => {}
};
const Model = function() {
const validate = params => {
if (!Number.isInteger(params.count)) {
throw 'error: parameter "count" must be a valid integer';
}
if (!Utils.isBoolean(params.upvoted)) {
throw 'error: parameter "upvoted" must be a boolean';
}
if (!Utils.isBoolean(params.downvoted)) {
throw 'error: parameter "downvoted" must be a boolean';
}
if (!Utils.isBoolean(params.starred)) {
throw 'error: parameter "starred" must be a boolean';
}
if (params.callback && !Utils.isFunction(params.callback)) {
throw 'error: parameter "callback" must be a function';
}
if (params.upvoted && params.downvoted) {
throw 'error: parameters "upvoted" and "downvoted" must not be true at the same time';
}
};
const create = params => {
validate(params);
const data = Utils.combine(params);
const upvote = () => {
if (data.upvoted) {
data.count--;
} else {
data.count++;
if (data.downvoted) {
data.downvoted = false;
data.count++;
}
}
data.upvoted = !data.upvoted;
};
const downvote = () => {
if (data.downvoted) {
data.count++;
} else {
data.count--;
if (data.upvoted) {
data.upvoted = false;
data.count--;
}
}
data.downvoted = !data.downvoted;
};
return {
count: () => data.count,
upvote: upvote,
upvoted: () => data.upvoted,
downvote: downvote,
downvoted: () => data.downvoted,
star: () => data.starred = !data.starred,
starred: () => data.starred,
data: () => Utils.combine(data)
};
};
return {
create: create
};
}();
const View = function() {
const create = id => {
const dom = document.getElementById(id);
if (dom === null) {
throw 'error: could not find element with ID ' + id + ' in the DOM';
}
if (Utils.classes(dom).includes(enabledClass)) {
throw 'error: element with ID ' + id + ' is already in use by another upvote controller';
}
dom.className += ' ' + enabledClass;
const firstElementByClass = className => {
const list = dom.getElementsByClassName(className);
if (list === null) {
throw 'error: could not find element with class ' + className + ' within element with ID ' + id + ' in the DOM';
}
return list[0];
};
const createCounter = className => {
const dom = firstElementByClass(className);
if (dom === undefined) {
return {
count: () => undefined,
set: Utils.noop
};
}
return {
count: () => parseInt(dom.innerHTML || 0, 10),
set: value => dom.innerHTML = value
};
};
const createToggle = (className, activeClassName) => {
const createClasses = () => {
const classes = {
[className]: true,
[activeClassName]: false,
};
item.className.split(/ +/)
.filter(x => x)
.forEach(className => classes[className] = true);
return classes;
};
const formatClassName = () => {
return Object.entries(classes)
.filter(e => e[1])
.map(e => e[0])
.join(' ');
};
const item = firstElementByClass(className);
if (item === undefined) {
return {
get: () => false,
set: Utils.noop,
onClick: Utils.noop
};
}
const classes = createClasses();
return {
get: () => classes[activeClassName],
set: value => {
classes[activeClassName] = value;
item.className = formatClassName();
},
onClick: fun => item.onclick = fun
};
};
const render = model => {
counter.set(model.count());
upvote.set(model.upvoted());
downvote.set(model.downvoted());
star.set(model.starred());
};
const parseParamsFromDom = () => {
return {
count: counter.count(),
upvoted: upvote.get(),
downvoted: downvote.get(),
starred: star.get()
};
};
const destroy = () => {
Utils.removeClass(dom, enabledClass);
upvote.onClick(null);
downvote.onClick(null);
star.onClick(null);
};
const counter = createCounter(countClass);
const upvote = createToggle(upvoteClass, upvoteOnClass);
const downvote = createToggle(downvoteClass, downvoteOnClass);
const star = createToggle(starClass, starOnClass);
return {
render: render,
parseParamsFromDom: parseParamsFromDom,
onClickUpvote: fun => upvote.onClick(fun),
onClickDownvote: fun => downvote.onClick(fun),
onClickStar: fun => star.onClick(fun),
destroy: destroy
};
};
return {
create: create
};
}();
const create = (id, params = {}) => {
var destroyed = false;
const view = View.create(id);
const domParams = view.parseParamsFromDom();
const defaults = {
id: id,
count: 0,
upvoted: false,
downvoted: false,
starred: false,
callback: () => {}
};
const combinedParams = Utils.combine(defaults, domParams, params);
const model = Model.create(combinedParams);
const callback = combinedParams.callback;
const throwIfDestroyed = () => {
if (destroyed) {
throw "fatal: unexpected call to destroyed controller";
}
};
const upvote = () => {
throwIfDestroyed();
model.upvote();
view.render(model);
callback(model.data());
};
const downvote = () => {
throwIfDestroyed();
model.downvote();
view.render(model);
callback(model.data());
};
const star = () => {
throwIfDestroyed();
model.star();
view.render(model);
callback(model.data());
};
const destroy = () => {
throwIfDestroyed();
destroyed = true;
view.destroy();
};
view.render(model);
view.onClickUpvote(upvote);
view.onClickDownvote(downvote);
view.onClickStar(star);
return {
id: id,
count: () => {
throwIfDestroyed();
return model.count();
},
upvote: upvote,
upvoted: () => {
throwIfDestroyed();
return model.upvoted();
},
downvote: downvote,
downvoted: () => {
throwIfDestroyed();
return model.downvoted();
},
star: star,
starred: () => {
throwIfDestroyed();
return model.starred();
},
destroy: destroy
};
};
return {
create: create
};
}();
Unit tests
I'm also interested in a review of the unit tests (live tests of current version):
const create = (id, params) => {
return Upvote.create(id, params);
};
QUnit.test('throw exception if destroy is called twice', assert => {
const obj = gen();
obj.destroy();
assert.throws(() => obj.destroy());
});
const gen = function() {
var idcount = 0;
return (params = {}) => {
++idcount;
const id = params.id || ('u' + idcount);
const jqdom = $('#templates div.upvotejs').clone();
jqdom.attr('id', id);
$('#tests').append(jqdom);
params.callback = params.callback || (data => {});
return create(id, params, jqdom);
};
}();
const uiTester = obj => {
const widget = $('#' + obj.id);
const count = widget.find('.count');
const upvote = widget.find('.upvote');
const downvote = widget.find('.downvote');
const star = widget.find('.star');
return {
count: () => parseInt(count.text(), 10),
upvoted: () => upvote.hasClass('upvote-on'),
downvoted: () => downvote.hasClass('downvote-on'),
starred: () => star.hasClass('star-on'),
upvote: () => upvote.click(),
downvote: () => downvote.click(),
star: () => star.click()
};
};
QUnit.test('initialize from params', assert => {
const obj = gen();
assert.equal(obj.count(), 0);
assert.equal(obj.upvoted(), false);
assert.equal(obj.downvoted(), false);
assert.equal(obj.starred(), false);
assert.equal(gen({count: 17}).count(), 17);
assert.equal(gen({upvoted: true}).upvoted(), true);
assert.equal(gen({downvoted: true}).downvoted(), true);
assert.equal(gen({starred: true}).starred(), true);
assert.throws(() => gen({count: 'foo'}), 'throw if count param is not an integer');
assert.throws(() => gen({upvoted: 'foo'}), 'throw if upvoted param is not a boolean');
assert.throws(() => gen({downvoted: 'foo'}), 'throw if downvoted param is not a boolean');
assert.throws(() => gen({starred: 'foo'}), 'throw if starred param is not a boolean');
assert.throws(() => gen({callback: 'foo'}), 'throw if callback param is not a function');
assert.throws(() => gen({upvoted: true, downvoted: true}), 'throw if upvoted=true and downvoted=true');
});
QUnit.test('initialize from dom', assert => {
const v1 = Upvote.create('count-1');
assert.equal(v1.count(), 1);
assert.equal(v1.upvoted(), false);
assert.equal(v1.downvoted(), false);
assert.equal(v1.starred(), false);
const v2 = Upvote.create('count-2-upvoted');
assert.equal(v2.count(), 2);
assert.equal(v2.upvoted(), true);
assert.equal(v2.downvoted(), false);
assert.equal(v2.starred(), false);
const v3 = Upvote.create('count-3-upvoted-starred');
assert.equal(v3.count(), 3);
assert.equal(v3.upvoted(), true);
assert.equal(v3.downvoted(), false);
assert.equal(v3.starred(), true);
const v4 = Upvote.create('count-4-downvoted');
assert.equal(v4.count(), 4);
assert.equal(v4.upvoted(), false);
assert.equal(v4.downvoted(), true);
assert.equal(v4.starred(), false);
const v5 = Upvote.create('count-5-downvoted-starred');
assert.equal(v5.count(), 5);
assert.equal(v5.upvoted(), false);
assert.equal(v5.downvoted(), true);
assert.equal(v5.starred(), true);
const vLarge = Upvote.create('count-456789');
assert.equal(vLarge.count(), 456789);
assert.equal(vLarge.upvoted(), false);
assert.equal(vLarge.downvoted(), false);
assert.equal(vLarge.starred(), false);
const vNegativeLarge = Upvote.create('count-minus-456789');
assert.equal(vNegativeLarge.count(), -456789);
assert.equal(vNegativeLarge.upvoted(), false);
assert.equal(vNegativeLarge.downvoted(), false);
assert.equal(vNegativeLarge.starred(), false);
assert.throws(() => Upvote.create('upvoted-downvoted'));
});
QUnit.test('UI updated from params', assert => {
const obj = uiTester(gen());
assert.equal(obj.count(), 0);
assert.equal(obj.upvoted(), false);
assert.equal(obj.downvoted(), false);
assert.equal(obj.starred(), false);
assert.equal(uiTester(gen({count: 17})).count(), 17);
assert.equal(uiTester(gen({upvoted: true})).upvoted(), true);
assert.equal(uiTester(gen({downvoted: true})).downvoted(), true);
assert.equal(uiTester(gen({starred: true})).starred(), true);
});
QUnit.test('upvote non-downvoted non-upvoted', assert => {
const count = 5;
const obj = gen({count: count});
assert.equal(obj.count(), count);
obj.upvote();
assert.equal(obj.count(), count + 1);
});
QUnit.test('upvote downvoted', assert => {
const count = 6;
const obj = gen({count: count, downvoted: true});
assert.equal(obj.count(), count);
obj.upvote();
assert.equal(obj.count(), count + 2);
});
QUnit.test('upvote upvoted', assert => {
const count = 7;
const obj = gen({count: count, upvoted: true});
assert.equal(obj.count(), count);
obj.upvote();
assert.equal(obj.count(), count - 1);
});
QUnit.test('downvote non-downvoted non-upvoted', assert => {
const count = 5;
const obj = gen({count: count});
assert.equal(obj.count(), count);
obj.downvote();
assert.equal(obj.count(), count - 1);
});
QUnit.test('downvote upvoted', assert => {
const count = 6;
const obj = gen({count: count, upvoted: true});
assert.equal(obj.count(), count);
obj.downvote();
assert.equal(obj.count(), count - 2);
});
QUnit.test('downvote downvoted', assert => {
const count = 7;
const obj = gen({count: count, downvoted: true});
assert.equal(obj.count(), count);
obj.downvote();
assert.equal(obj.count(), count + 1);
});
QUnit.test('star non-starred', assert => {
const obj = gen();
obj.star();
assert.ok(obj.starred(), 'should be starred');
});
QUnit.test('star starred', assert => {
const obj = gen({starred: true});
obj.star();
assert.ok(!obj.starred(), 'should not be starred');
});
QUnit.test('upvote indepently', assert => {
const count1 = 5;
const v1 = gen({count: count1});
const count2 = 5;
const v2 = gen({count: count2});
v1.upvote();
assert.equal(v1.count(), count1 + 1);
assert.equal(v2.count(), count2);
});
QUnit.test('downvote indepently', assert => {
const count1 = 5;
const v1 = gen({count: count1});
const count2 = 5;
const v2 = gen({count: count2});
v1.downvote();
assert.equal(v1.count(), count1 - 1);
assert.equal(v2.count(), count2);
});
QUnit.test('star indepently', assert => {
const v1 = gen();
const v2 = gen();
v1.star();
assert.equal(v1.starred(), true);
assert.equal(v2.starred(), false);
});
QUnit.test('call callback on value changes', assert => {
var receivedPayload;
const callback = payload => receivedPayload = payload;
const obj1_id = 100;
const obj1_origCount = 10;
const obj1 = gen({id: obj1_id, count: obj1_origCount, callback: callback});
const obj2_id = 200;
const obj2_origCount = 20;
const obj2 = gen({id: obj2_id, count: obj2_origCount, callback: callback});
obj1.upvote();
assert.deepEqual(receivedPayload, {
id: obj1_id,
action: 'upvote',
newState: {
count: obj1_origCount + 1,
downvoted: false,
upvoted: true,
starred: false
}
});
obj2.upvote();
assert.deepEqual(receivedPayload, {
id: obj2_id,
action: 'upvote',
newState: {
count: obj2_origCount + 1,
downvoted: false,
upvoted: true,
starred: false
}
});
obj1.upvote();
assert.deepEqual(receivedPayload, {
id: obj1_id,
action: 'unupvote',
newState: {
count: obj1_origCount,
downvoted: false,
upvoted: false,
starred: false
}
});
obj2.star();
assert.deepEqual(receivedPayload, {
id: obj2_id,
action: 'star',
newState: {
count: obj2_origCount + 1,
downvoted: false,
upvoted: true,
starred: true
}
});
obj2.star();
assert.deepEqual(receivedPayload, {
id: obj2_id,
action: 'unstar',
newState: {
count: obj2_origCount + 1,
downvoted: false,
upvoted: true,
starred: false
}
});
obj2.downvote();
assert.deepEqual(receivedPayload, {
id: obj2_id,
action: 'downvote',
newState: {
count: obj2_origCount - 1,
downvoted: true,
upvoted: false,
starred: false
}
});
obj2.downvote();
assert.deepEqual(receivedPayload, {
id: obj2_id,
action: 'undownvote',
newState: {
count: obj2_origCount,
downvoted: false,
upvoted: false,
starred: false
}
});
});
QUnit.test('update model updates UI', assert => {
const obj = gen();
const ui = uiTester(obj);
obj.upvote();
assert.equal(ui.count(), 1);
assert.equal(ui.upvoted(), true);
obj.downvote();
assert.equal(ui.count(), -1);
assert.equal(ui.upvoted(), false);
assert.equal(ui.downvoted(), true);
obj.upvote();
assert.equal(ui.count(), 1);
assert.equal(ui.upvoted(), true);
assert.equal(ui.downvoted(), false);
obj.star();
assert.equal(ui.starred(), true);
obj.star();
assert.equal(ui.starred(), false);
});
QUnit.test('update UI updates model', assert => {
const obj = gen();
const ui = uiTester(obj);
ui.upvote();
assert.equal(obj.count(), 1);
assert.equal(obj.upvoted(), true);
ui.downvote();
assert.equal(obj.count(), -1);
assert.equal(obj.upvoted(), false);
assert.equal(obj.downvoted(), true);
ui.upvote();
assert.equal(obj.count(), 1);
assert.equal(obj.upvoted(), true);
assert.equal(obj.downvoted(), false);
ui.star();
assert.equal(obj.starred(), true);
ui.star();
assert.equal(obj.starred(), false);
});
QUnit.test('cannot associate multiple models to the same id', assert => {
const orig = gen();
assert.throws(() => gen({id: orig.id}));
});
QUnit.test('widget stops responding to clicks after destroyed', assert => {
const obj = gen({count: 99});
const ui = uiTester(obj);
ui.upvote();
assert.equal(ui.count(), 100);
ui.upvote();
assert.equal(ui.count(), 99);
obj.destroy();
ui.upvote();
assert.equal(ui.count(), 99);
assert.throws(() => obj.upvote());
assert.throws(() => obj.downvote());
assert.throws(() => obj.star());
assert.throws(() => obj.count());
assert.throws(() => obj.upvoted());
assert.throws(() => obj.downvoted());
assert.throws(() => obj.starred());
const reused = gen({id: obj.id});
assert.equal(reused.count(), 99);
ui.upvote();
assert.equal(reused.count(), 100);
});
QUnit.test('all sub-elements (upvote/downvote/count/star) are optional in the HTML markup', assert => {
['upvote', 'downvote', 'count', 'star'].forEach(cls => {
const obj0 = gen();
obj0.destroy();
const jqdom = $('#' + obj0.id);
jqdom.find('.' + cls).remove();
const obj = create(obj0.id, {}, jqdom);
assert.equal(obj.count(), 0);
obj.upvote();
assert.equal(obj.count(), 1);
assert.equal(obj.upvoted(), true);
obj.downvote();
assert.equal(obj.count(), -1);
assert.equal(obj.downvoted(), true);
assert.equal(obj.upvoted(), false);
obj.downvote();
assert.equal(obj.count(), 0);
assert.equal(obj.downvoted(), false);
obj.star();
assert.equal(obj.starred(), true);
obj.star();
assert.equal(obj.starred(), false);
});
}
Code review
I'm looking for a review of the posted code in any and all aspects.
Some particular areas do come to mind I'm especially curious about:
- Bad practices: is there anything to fix
- Usability: is there anything that looks hard to use, and how to make it better
- Are the tests sufficient? Did I miss any important cases?
- Are the tests overcomplicated? Do you see ways to simplify?
- I'm used to using QUnit for testing JavaScript packages. Do you think I could benefit greatly by using something else?
javascript
$endgroup$
I implemented a simple voting widget like the one used on Stack Exchange sites, to use for other purposes (see live on bashoneliners.com), as a reusable package, dubbed UpvoteJS.
Here's how it works:
Upvote.create('topic-123');
<link rel="stylesheet" href="https://janosgyerik.github.io/upvotejs/dist/upvotejs/upvotejs.css">
<script src="https://janosgyerik.github.io/upvotejs/dist/upvotejs/upvotejs.vanilla.js"></script>
<div id="topic-123" class="upvotejs upvotejs-serverfault">
<a class="upvote upvote-on"></a>
<span class="count">101</span>
<a class="downvote"></a>
<a class="star star-on"></a>
</div>
How to use
As you can see in the above snippet, the typical use has the following elements:
Include the stylesheet
upvotejs.css
with a<link>
tag. The companionupvotejs.svg
is expected in the same directory on the web server.Add HTML markup with the fields you need. With the stylesheet included, this is enough to get a nicely rendered widget, read-only, since without JavaScript, the clicks will have no effect.
To make the widget interactive, responding to user clicks, include the JavaScript package
upvotejs.vanilla.js
with a<script>
tag, and activate the widget using the providedUpvote.create(topicId)
function in JavaScript (to happen after the DOM is loaded). ThetopicId
parameter is mandatory, and it must refer to a unique id in the DOM.To make the widget save state to a backend (not featured in the snippet), pass to
Upvote.create
as second parameter a JSON object, with a field namedcallback
, which must be a JavaScript function, for example:Upvote.create(topicId, {callback: your_callback_function})
. It is up to the developer to implementyour_callback_function
to persist its payload. The function will get called on any state change.
The above should be enough for typical use cases.
Additional notes:
All sub-elements in the HTML markup are optional, for example it's possible to have a widget without a star button, or any other element.
The initial state of a widget can be set either with HTML markup (recommended), or using basic markup (without
upvote-on
,downvote-on
,star-on
classes) and setting values in the second parameter of theUpvote.create
call, for example:Upvote.create(topicId, {count: 123, upvoted: true})
Upvote.create
does some sanity checks, and throws an exception when it detects illegal state, for example (not an exhaustive list):
- The specified ID doesn't exist in the DOM
- The parameter types in the JSON object don't match what's expected (
count
must be integer, and so on)
upvoted
anddownvoted
are bothtrue
Upvote.create
returns an object that represents the widget, and can be used to inspect and control its state. In the typical use case, this is probably not necessary.
Implementation
I'm most interested in a review of the JavaScript part (upvotejs.vanilla.js
(2.1.0)),
the complete project is available on GitHub.
const Upvote = function() {
const upvoteClass = 'upvote';
const enabledClass = 'upvotejs-enabled';
const upvoteOnClass = 'upvote-on';
const downvoteClass = 'downvote';
const downvoteOnClass = 'downvote-on';
const starClass = 'star';
const starOnClass = 'star-on';
const countClass = 'count';
const Utils = {
combine: function() {
const combined = {};
for (let i = 0; i < arguments.length; i++) {
Object.entries(arguments[i])
.filter(e => e[1] !== undefined)
.forEach(e => combined[e[0]] = e[1]);
}
return combined;
},
isBoolean: v => typeof v === "boolean",
isFunction: v => typeof v === "function",
classes: dom => dom.className.split(/ +/).filter(x => x),
removeClass: (dom, className) => {
dom.className = dom.className.split(/ +/)
.filter(x => x)
.filter(c => c !== className)
.join(' ');
},
noop: () => {}
};
const Model = function() {
const validate = params => {
if (!Number.isInteger(params.count)) {
throw 'error: parameter "count" must be a valid integer';
}
if (!Utils.isBoolean(params.upvoted)) {
throw 'error: parameter "upvoted" must be a boolean';
}
if (!Utils.isBoolean(params.downvoted)) {
throw 'error: parameter "downvoted" must be a boolean';
}
if (!Utils.isBoolean(params.starred)) {
throw 'error: parameter "starred" must be a boolean';
}
if (params.callback && !Utils.isFunction(params.callback)) {
throw 'error: parameter "callback" must be a function';
}
if (params.upvoted && params.downvoted) {
throw 'error: parameters "upvoted" and "downvoted" must not be true at the same time';
}
};
const create = params => {
validate(params);
const data = Utils.combine(params);
const upvote = () => {
if (data.upvoted) {
data.count--;
} else {
data.count++;
if (data.downvoted) {
data.downvoted = false;
data.count++;
}
}
data.upvoted = !data.upvoted;
};
const downvote = () => {
if (data.downvoted) {
data.count++;
} else {
data.count--;
if (data.upvoted) {
data.upvoted = false;
data.count--;
}
}
data.downvoted = !data.downvoted;
};
return {
count: () => data.count,
upvote: upvote,
upvoted: () => data.upvoted,
downvote: downvote,
downvoted: () => data.downvoted,
star: () => data.starred = !data.starred,
starred: () => data.starred,
data: () => Utils.combine(data)
};
};
return {
create: create
};
}();
const View = function() {
const create = id => {
const dom = document.getElementById(id);
if (dom === null) {
throw 'error: could not find element with ID ' + id + ' in the DOM';
}
if (Utils.classes(dom).includes(enabledClass)) {
throw 'error: element with ID ' + id + ' is already in use by another upvote controller';
}
dom.className += ' ' + enabledClass;
const firstElementByClass = className => {
const list = dom.getElementsByClassName(className);
if (list === null) {
throw 'error: could not find element with class ' + className + ' within element with ID ' + id + ' in the DOM';
}
return list[0];
};
const createCounter = className => {
const dom = firstElementByClass(className);
if (dom === undefined) {
return {
count: () => undefined,
set: Utils.noop
};
}
return {
count: () => parseInt(dom.innerHTML || 0, 10),
set: value => dom.innerHTML = value
};
};
const createToggle = (className, activeClassName) => {
const createClasses = () => {
const classes = {
[className]: true,
[activeClassName]: false,
};
item.className.split(/ +/)
.filter(x => x)
.forEach(className => classes[className] = true);
return classes;
};
const formatClassName = () => {
return Object.entries(classes)
.filter(e => e[1])
.map(e => e[0])
.join(' ');
};
const item = firstElementByClass(className);
if (item === undefined) {
return {
get: () => false,
set: Utils.noop,
onClick: Utils.noop
};
}
const classes = createClasses();
return {
get: () => classes[activeClassName],
set: value => {
classes[activeClassName] = value;
item.className = formatClassName();
},
onClick: fun => item.onclick = fun
};
};
const render = model => {
counter.set(model.count());
upvote.set(model.upvoted());
downvote.set(model.downvoted());
star.set(model.starred());
};
const parseParamsFromDom = () => {
return {
count: counter.count(),
upvoted: upvote.get(),
downvoted: downvote.get(),
starred: star.get()
};
};
const destroy = () => {
Utils.removeClass(dom, enabledClass);
upvote.onClick(null);
downvote.onClick(null);
star.onClick(null);
};
const counter = createCounter(countClass);
const upvote = createToggle(upvoteClass, upvoteOnClass);
const downvote = createToggle(downvoteClass, downvoteOnClass);
const star = createToggle(starClass, starOnClass);
return {
render: render,
parseParamsFromDom: parseParamsFromDom,
onClickUpvote: fun => upvote.onClick(fun),
onClickDownvote: fun => downvote.onClick(fun),
onClickStar: fun => star.onClick(fun),
destroy: destroy
};
};
return {
create: create
};
}();
const create = (id, params = {}) => {
var destroyed = false;
const view = View.create(id);
const domParams = view.parseParamsFromDom();
const defaults = {
id: id,
count: 0,
upvoted: false,
downvoted: false,
starred: false,
callback: () => {}
};
const combinedParams = Utils.combine(defaults, domParams, params);
const model = Model.create(combinedParams);
const callback = combinedParams.callback;
const throwIfDestroyed = () => {
if (destroyed) {
throw "fatal: unexpected call to destroyed controller";
}
};
const upvote = () => {
throwIfDestroyed();
model.upvote();
view.render(model);
callback(model.data());
};
const downvote = () => {
throwIfDestroyed();
model.downvote();
view.render(model);
callback(model.data());
};
const star = () => {
throwIfDestroyed();
model.star();
view.render(model);
callback(model.data());
};
const destroy = () => {
throwIfDestroyed();
destroyed = true;
view.destroy();
};
view.render(model);
view.onClickUpvote(upvote);
view.onClickDownvote(downvote);
view.onClickStar(star);
return {
id: id,
count: () => {
throwIfDestroyed();
return model.count();
},
upvote: upvote,
upvoted: () => {
throwIfDestroyed();
return model.upvoted();
},
downvote: downvote,
downvoted: () => {
throwIfDestroyed();
return model.downvoted();
},
star: star,
starred: () => {
throwIfDestroyed();
return model.starred();
},
destroy: destroy
};
};
return {
create: create
};
}();
Unit tests
I'm also interested in a review of the unit tests (live tests of current version):
const create = (id, params) => {
return Upvote.create(id, params);
};
QUnit.test('throw exception if destroy is called twice', assert => {
const obj = gen();
obj.destroy();
assert.throws(() => obj.destroy());
});
const gen = function() {
var idcount = 0;
return (params = {}) => {
++idcount;
const id = params.id || ('u' + idcount);
const jqdom = $('#templates div.upvotejs').clone();
jqdom.attr('id', id);
$('#tests').append(jqdom);
params.callback = params.callback || (data => {});
return create(id, params, jqdom);
};
}();
const uiTester = obj => {
const widget = $('#' + obj.id);
const count = widget.find('.count');
const upvote = widget.find('.upvote');
const downvote = widget.find('.downvote');
const star = widget.find('.star');
return {
count: () => parseInt(count.text(), 10),
upvoted: () => upvote.hasClass('upvote-on'),
downvoted: () => downvote.hasClass('downvote-on'),
starred: () => star.hasClass('star-on'),
upvote: () => upvote.click(),
downvote: () => downvote.click(),
star: () => star.click()
};
};
QUnit.test('initialize from params', assert => {
const obj = gen();
assert.equal(obj.count(), 0);
assert.equal(obj.upvoted(), false);
assert.equal(obj.downvoted(), false);
assert.equal(obj.starred(), false);
assert.equal(gen({count: 17}).count(), 17);
assert.equal(gen({upvoted: true}).upvoted(), true);
assert.equal(gen({downvoted: true}).downvoted(), true);
assert.equal(gen({starred: true}).starred(), true);
assert.throws(() => gen({count: 'foo'}), 'throw if count param is not an integer');
assert.throws(() => gen({upvoted: 'foo'}), 'throw if upvoted param is not a boolean');
assert.throws(() => gen({downvoted: 'foo'}), 'throw if downvoted param is not a boolean');
assert.throws(() => gen({starred: 'foo'}), 'throw if starred param is not a boolean');
assert.throws(() => gen({callback: 'foo'}), 'throw if callback param is not a function');
assert.throws(() => gen({upvoted: true, downvoted: true}), 'throw if upvoted=true and downvoted=true');
});
QUnit.test('initialize from dom', assert => {
const v1 = Upvote.create('count-1');
assert.equal(v1.count(), 1);
assert.equal(v1.upvoted(), false);
assert.equal(v1.downvoted(), false);
assert.equal(v1.starred(), false);
const v2 = Upvote.create('count-2-upvoted');
assert.equal(v2.count(), 2);
assert.equal(v2.upvoted(), true);
assert.equal(v2.downvoted(), false);
assert.equal(v2.starred(), false);
const v3 = Upvote.create('count-3-upvoted-starred');
assert.equal(v3.count(), 3);
assert.equal(v3.upvoted(), true);
assert.equal(v3.downvoted(), false);
assert.equal(v3.starred(), true);
const v4 = Upvote.create('count-4-downvoted');
assert.equal(v4.count(), 4);
assert.equal(v4.upvoted(), false);
assert.equal(v4.downvoted(), true);
assert.equal(v4.starred(), false);
const v5 = Upvote.create('count-5-downvoted-starred');
assert.equal(v5.count(), 5);
assert.equal(v5.upvoted(), false);
assert.equal(v5.downvoted(), true);
assert.equal(v5.starred(), true);
const vLarge = Upvote.create('count-456789');
assert.equal(vLarge.count(), 456789);
assert.equal(vLarge.upvoted(), false);
assert.equal(vLarge.downvoted(), false);
assert.equal(vLarge.starred(), false);
const vNegativeLarge = Upvote.create('count-minus-456789');
assert.equal(vNegativeLarge.count(), -456789);
assert.equal(vNegativeLarge.upvoted(), false);
assert.equal(vNegativeLarge.downvoted(), false);
assert.equal(vNegativeLarge.starred(), false);
assert.throws(() => Upvote.create('upvoted-downvoted'));
});
QUnit.test('UI updated from params', assert => {
const obj = uiTester(gen());
assert.equal(obj.count(), 0);
assert.equal(obj.upvoted(), false);
assert.equal(obj.downvoted(), false);
assert.equal(obj.starred(), false);
assert.equal(uiTester(gen({count: 17})).count(), 17);
assert.equal(uiTester(gen({upvoted: true})).upvoted(), true);
assert.equal(uiTester(gen({downvoted: true})).downvoted(), true);
assert.equal(uiTester(gen({starred: true})).starred(), true);
});
QUnit.test('upvote non-downvoted non-upvoted', assert => {
const count = 5;
const obj = gen({count: count});
assert.equal(obj.count(), count);
obj.upvote();
assert.equal(obj.count(), count + 1);
});
QUnit.test('upvote downvoted', assert => {
const count = 6;
const obj = gen({count: count, downvoted: true});
assert.equal(obj.count(), count);
obj.upvote();
assert.equal(obj.count(), count + 2);
});
QUnit.test('upvote upvoted', assert => {
const count = 7;
const obj = gen({count: count, upvoted: true});
assert.equal(obj.count(), count);
obj.upvote();
assert.equal(obj.count(), count - 1);
});
QUnit.test('downvote non-downvoted non-upvoted', assert => {
const count = 5;
const obj = gen({count: count});
assert.equal(obj.count(), count);
obj.downvote();
assert.equal(obj.count(), count - 1);
});
QUnit.test('downvote upvoted', assert => {
const count = 6;
const obj = gen({count: count, upvoted: true});
assert.equal(obj.count(), count);
obj.downvote();
assert.equal(obj.count(), count - 2);
});
QUnit.test('downvote downvoted', assert => {
const count = 7;
const obj = gen({count: count, downvoted: true});
assert.equal(obj.count(), count);
obj.downvote();
assert.equal(obj.count(), count + 1);
});
QUnit.test('star non-starred', assert => {
const obj = gen();
obj.star();
assert.ok(obj.starred(), 'should be starred');
});
QUnit.test('star starred', assert => {
const obj = gen({starred: true});
obj.star();
assert.ok(!obj.starred(), 'should not be starred');
});
QUnit.test('upvote indepently', assert => {
const count1 = 5;
const v1 = gen({count: count1});
const count2 = 5;
const v2 = gen({count: count2});
v1.upvote();
assert.equal(v1.count(), count1 + 1);
assert.equal(v2.count(), count2);
});
QUnit.test('downvote indepently', assert => {
const count1 = 5;
const v1 = gen({count: count1});
const count2 = 5;
const v2 = gen({count: count2});
v1.downvote();
assert.equal(v1.count(), count1 - 1);
assert.equal(v2.count(), count2);
});
QUnit.test('star indepently', assert => {
const v1 = gen();
const v2 = gen();
v1.star();
assert.equal(v1.starred(), true);
assert.equal(v2.starred(), false);
});
QUnit.test('call callback on value changes', assert => {
var receivedPayload;
const callback = payload => receivedPayload = payload;
const obj1_id = 100;
const obj1_origCount = 10;
const obj1 = gen({id: obj1_id, count: obj1_origCount, callback: callback});
const obj2_id = 200;
const obj2_origCount = 20;
const obj2 = gen({id: obj2_id, count: obj2_origCount, callback: callback});
obj1.upvote();
assert.deepEqual(receivedPayload, {
id: obj1_id,
action: 'upvote',
newState: {
count: obj1_origCount + 1,
downvoted: false,
upvoted: true,
starred: false
}
});
obj2.upvote();
assert.deepEqual(receivedPayload, {
id: obj2_id,
action: 'upvote',
newState: {
count: obj2_origCount + 1,
downvoted: false,
upvoted: true,
starred: false
}
});
obj1.upvote();
assert.deepEqual(receivedPayload, {
id: obj1_id,
action: 'unupvote',
newState: {
count: obj1_origCount,
downvoted: false,
upvoted: false,
starred: false
}
});
obj2.star();
assert.deepEqual(receivedPayload, {
id: obj2_id,
action: 'star',
newState: {
count: obj2_origCount + 1,
downvoted: false,
upvoted: true,
starred: true
}
});
obj2.star();
assert.deepEqual(receivedPayload, {
id: obj2_id,
action: 'unstar',
newState: {
count: obj2_origCount + 1,
downvoted: false,
upvoted: true,
starred: false
}
});
obj2.downvote();
assert.deepEqual(receivedPayload, {
id: obj2_id,
action: 'downvote',
newState: {
count: obj2_origCount - 1,
downvoted: true,
upvoted: false,
starred: false
}
});
obj2.downvote();
assert.deepEqual(receivedPayload, {
id: obj2_id,
action: 'undownvote',
newState: {
count: obj2_origCount,
downvoted: false,
upvoted: false,
starred: false
}
});
});
QUnit.test('update model updates UI', assert => {
const obj = gen();
const ui = uiTester(obj);
obj.upvote();
assert.equal(ui.count(), 1);
assert.equal(ui.upvoted(), true);
obj.downvote();
assert.equal(ui.count(), -1);
assert.equal(ui.upvoted(), false);
assert.equal(ui.downvoted(), true);
obj.upvote();
assert.equal(ui.count(), 1);
assert.equal(ui.upvoted(), true);
assert.equal(ui.downvoted(), false);
obj.star();
assert.equal(ui.starred(), true);
obj.star();
assert.equal(ui.starred(), false);
});
QUnit.test('update UI updates model', assert => {
const obj = gen();
const ui = uiTester(obj);
ui.upvote();
assert.equal(obj.count(), 1);
assert.equal(obj.upvoted(), true);
ui.downvote();
assert.equal(obj.count(), -1);
assert.equal(obj.upvoted(), false);
assert.equal(obj.downvoted(), true);
ui.upvote();
assert.equal(obj.count(), 1);
assert.equal(obj.upvoted(), true);
assert.equal(obj.downvoted(), false);
ui.star();
assert.equal(obj.starred(), true);
ui.star();
assert.equal(obj.starred(), false);
});
QUnit.test('cannot associate multiple models to the same id', assert => {
const orig = gen();
assert.throws(() => gen({id: orig.id}));
});
QUnit.test('widget stops responding to clicks after destroyed', assert => {
const obj = gen({count: 99});
const ui = uiTester(obj);
ui.upvote();
assert.equal(ui.count(), 100);
ui.upvote();
assert.equal(ui.count(), 99);
obj.destroy();
ui.upvote();
assert.equal(ui.count(), 99);
assert.throws(() => obj.upvote());
assert.throws(() => obj.downvote());
assert.throws(() => obj.star());
assert.throws(() => obj.count());
assert.throws(() => obj.upvoted());
assert.throws(() => obj.downvoted());
assert.throws(() => obj.starred());
const reused = gen({id: obj.id});
assert.equal(reused.count(), 99);
ui.upvote();
assert.equal(reused.count(), 100);
});
QUnit.test('all sub-elements (upvote/downvote/count/star) are optional in the HTML markup', assert => {
['upvote', 'downvote', 'count', 'star'].forEach(cls => {
const obj0 = gen();
obj0.destroy();
const jqdom = $('#' + obj0.id);
jqdom.find('.' + cls).remove();
const obj = create(obj0.id, {}, jqdom);
assert.equal(obj.count(), 0);
obj.upvote();
assert.equal(obj.count(), 1);
assert.equal(obj.upvoted(), true);
obj.downvote();
assert.equal(obj.count(), -1);
assert.equal(obj.downvoted(), true);
assert.equal(obj.upvoted(), false);
obj.downvote();
assert.equal(obj.count(), 0);
assert.equal(obj.downvoted(), false);
obj.star();
assert.equal(obj.starred(), true);
obj.star();
assert.equal(obj.starred(), false);
});
}
Code review
I'm looking for a review of the posted code in any and all aspects.
Some particular areas do come to mind I'm especially curious about:
- Bad practices: is there anything to fix
- Usability: is there anything that looks hard to use, and how to make it better
- Are the tests sufficient? Did I miss any important cases?
- Are the tests overcomplicated? Do you see ways to simplify?
- I'm used to using QUnit for testing JavaScript packages. Do you think I could benefit greatly by using something else?
Upvote.create('topic-123');
<link rel="stylesheet" href="https://janosgyerik.github.io/upvotejs/dist/upvotejs/upvotejs.css">
<script src="https://janosgyerik.github.io/upvotejs/dist/upvotejs/upvotejs.vanilla.js"></script>
<div id="topic-123" class="upvotejs upvotejs-serverfault">
<a class="upvote upvote-on"></a>
<span class="count">101</span>
<a class="downvote"></a>
<a class="star star-on"></a>
</div>
Upvote.create('topic-123');
<link rel="stylesheet" href="https://janosgyerik.github.io/upvotejs/dist/upvotejs/upvotejs.css">
<script src="https://janosgyerik.github.io/upvotejs/dist/upvotejs/upvotejs.vanilla.js"></script>
<div id="topic-123" class="upvotejs upvotejs-serverfault">
<a class="upvote upvote-on"></a>
<span class="count">101</span>
<a class="downvote"></a>
<a class="star star-on"></a>
</div>
javascript
javascript
asked 6 mins ago
janosjanos
97.4k12125350
97.4k12125350
add a comment |
add a comment |
0
active
oldest
votes
Your Answer
StackExchange.ifUsing("editor", function () {
return StackExchange.using("mathjaxEditing", function () {
StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix) {
StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
});
});
}, "mathjax-editing");
StackExchange.ifUsing("editor", function () {
StackExchange.using("externalEditor", function () {
StackExchange.using("snippets", function () {
StackExchange.snippets.init();
});
});
}, "code-snippets");
StackExchange.ready(function() {
var channelOptions = {
tags: "".split(" "),
id: "196"
};
initTagRenderer("".split(" "), "".split(" "), channelOptions);
StackExchange.using("externalEditor", function() {
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled) {
StackExchange.using("snippets", function() {
createEditor();
});
}
else {
createEditor();
}
});
function createEditor() {
StackExchange.prepareEditor({
heartbeatType: 'answer',
autoActivateHeartbeat: false,
convertImagesToLinks: false,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
imageUploader: {
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
},
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
});
}
});
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f213172%2fupvotejs-a-simple-voting-widget-in-vanilla-javascript%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
0
active
oldest
votes
0
active
oldest
votes
active
oldest
votes
active
oldest
votes
Thanks for contributing an answer to Code Review Stack Exchange!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
Use MathJax to format equations. MathJax reference.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function () {
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f213172%2fupvotejs-a-simple-voting-widget-in-vanilla-javascript%23new-answer', 'question_page');
}
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function () {
StackExchange.helpers.onClickDraftSave('#login-link');
});
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown