I'm fairly new to JavaScript, and wanted some critique on some code I've written based upon what I've learned recently. I have a strong background in OO programming (C/C++, C#, Java), but recently I've had to delve into JavaScript for my job. I'm working with a lot of legacy code, and want to improve it as I go.
I've written a 'toolbar' module to manage a dynamic toolbar using JQuery. I was wondering if anyone could give me some constructive criticisms on my approach. I'm finding it difficult to understand a good style, as I'm used to the very rigid constructs of other languages and the expressiveness JavaScript allows has me a little lost. I feel what I have written is clean, however I'm not sure whether I'm trying to force the OO thing too much, and if that's something I should be getting away from.
The following function creates my toolbar, and uses a closure to ensure some variables are kept private. This improves abstraction and keeps the interface clean.
function createToolbar(spec) {
// ----- PRIVATE VARIABLES -----
var buttons = [];
var parentDivId = '#' + spec.divId;
var divId = '#' + spec.divId + 'Inner';
var stateTransition = spec.stateTransition;
var currentState;
var that = {};
// ----- PRIVATE FUNCTIONS -----
var refreshState = function () {
$.each(buttons, function (index, button) {
$.each(button.states, function (index, s) {
if (currentState === s) {
if (button.showIf === undefined || button.showIf()) {
$(divId).find(button.id).show("slow");
} else {
$(divId).find(button.id).show("hide");
}
if (button.disableIf !== undefined && button.disableIf()) {
$(divId).find(button.id).button("disable");
} else {
$(divId).find(button.id).button("enable");
}
return false;
}
});
});
};
var refreshHTML = function () {
$(divId).empty();
$.each(buttons, function (index, button) {
$(divId).append(button.html);
$(divId).find(button.id).button({
label: button.label,
icons: {
primary: button.primaryIcon
}
})
.click(function (event) {
event.preventDefault();
if(button.func !== undefined) {
button.func();
}
}).hide();
});
$(divId).buttonset();
};
// ----- PUBLIC FUNCTIONS -----
var addButton = function (buttonSpec) {
// Check for duplicate text to modify ID accordingly
var existing = 0;
$.each(buttons, function(index, button) {
if(button.label === buttonSpec.text) {
existing += 1;
}
});
buttons.push({
states: buttonSpec.showStates,
func: buttonSpec.onclick,
id: '#' + buttonSpec.text + 'Btn' + existing.toString(),
showIf: buttonSpec.showCondition,
disableIf: buttonSpec.disableCondition,
label: buttonSpec.text,
html: '<button id="' + buttonSpec.text + 'Btn' + existing.toString() + '"></button>',
primaryIcon: buttonSpec.icon
});
console.log(buttonSpec.icon);
refreshHTML();
};
var setState = function (state) {
currentState = state;
if (stateTransition) {
stateTransition();
}
$.each(buttons, function (index, button) {
$(divId).find(button.id).hide("slow");
$.each(button.states, function (index, s) {
if (state === s) {
if (button.showIf === undefined || button.showIf()) {
$(divId).find(button.id).show("slow");
}
return false;
}
});
});
refreshState();
};
var clear = function () {
buttons = [];
refreshHTML();
};
var hide = function () {
$(divId).hide();
};
var show = function () {
$(divId).show();
};
var getCurrentState = function() {
return currentState;
};
// ----- CONSTRUCTION -----
// Create inner div for toolbar
$('#' + spec.divId).append('<div id=' + spec.divId + 'Inner></div>');
// Add any buttons that are part of the spec
if (spec.buttons) {
$.each(spec.buttons, function (index, button) {
addButton(button);
});
}
// Assign publically accessible functions
that.hide = hide;
that.show = show;
that.addButton = addButton;
that.setState = setState;
that.clear = clear;
that.refresh = refreshState;
that.getCurrentState = getCurrentState;
return that;
};
And here's a simple example of a toolbar on a page:
<html>
<head>
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.4/jquery.min.js"></script>
<script src="https://ajax.aspnetcdn.com/ajax/jquery.ui/1.8.17/jquery-ui.min.js"></script>
<link rel="stylesheet" type="text/css" href="https://ajax.googleapis.com/ajax/libs/jqueryui/1.8.4/themes/smoothness/jquery-ui.css" />
<script src="toolbar.js"></script>
</head>
<body>
<div id="toolbar"></div>
<script>
$(function () {
var tools = createToolbar({
divId: "toolbar",
stateTransition: function () {
},
buttons: [
{
showStates: ["main"],
text: "Draw",
onclick: function () {
console.log("Test");
},
icon: "ui-icon-pencil"
},
{
showStates: ["main"],
text: "Edit",
onclick: function() {
tools.setState("edit");
},
icon: "ui-icon-wrench"
},
{
showStates: ["main"],
text: "Options",
onclick: function() {
tools.setState("options");
},
icon: "ui-icon-gear"
},
{
showStates: ["edit"],
text: "Save",
onclick: function() {
console.log("Save");
},
icon: "ui-icon-document"
},
{
showStates: ["edit"],
text: "Update",
onclick: function() {
console.log("Update");
},
icon: "ui-icon-refresh"
},
{
showStates: ["edit"],
text: "Back",
onclick: function() {
tools.setState("main");
},
icon: "ui-icon-carat-1-w"
},
{
showStates: ["options"],
text: "Sound",
onclick: function() {
console.log("Sound");
},
icon: "ui-icon-volume-on"
},
{
showStates: ["options"],
text: "Graphic",
onclick: function() {
console.log("Graphic");
},
icon: "ui-icon-image"
},
{
showStates: ["options"],
text: "Back",
onclick: function() {
tools.setState("main");
},
icon: "ui-icon-carat-1-w"
}
]
});
tools.setState("main");
console.log(document.documentElement.innerHTML);
});
</script>
</body>
</html>
I was less happy with the construction of the toolbar itself, as it looked a bit repetitive. I'm not really looking for criticisms on my choice of libraries, purely about the style of my JavaScript. What do you think could be improved here?
