patternjavascriptMinor
Stamp order / preview form for a site
Viewed 0 times
orderpreviewstampforsiteform
Problem
I am trying to create a stamp order / preview form for a site and have gotten fairly far on my own, with a little help from Google and of course you all. If you can suggest any other method of going about this, please guide me in the right direction. Also, I need to figure out how to put a border, the same colour as has been chosen in the dropdown selection, around the containing div.
```
function setColor() {
var color = document.getElementById("color").value;
document.getElementById("myDiv").style.color = color;
}
function addContent(divName, content) {
document.getElementById(divName).innerHTML = content;
}
function fontSize(size) {
document.getElementById("lineOne").style.fontSize = size
}
function fontFamily(family) {
document.getElementById("lineOne").style.fontFamily = family
}
function fontStyle(style) {
document.getElementById("lineOne").style.fontStyle = style
}
function fontWeight(weight) {
document.getElementById("lineOne").style.fontWeight = weight
}
function align(align) {
document.getElementById("lineOne").style.textAlign = align;
}
function fontSize1(size1) {
document.getElementById("lineTwo").style.fontSize = size1
}
function fontFamily1(family1) {
document.getElementById("lineTwo").style.fontFamily = family1
}
function fontStyle1(style1) {
document.getElementById("lineTwo").style.fontStyle = style1
}
function fontWeight1(weight1) {
document.getElementById("lineTwo").style.fontWeight = weight1
}
function align1(align1) {
document.getElementById("lineTwo").style.textAlign = align1;
}
function fontSize2(size2) {
document.getElementById("lineThree").style.fontSize = size2
```
function setColor() {
var color = document.getElementById("color").value;
document.getElementById("myDiv").style.color = color;
}
function addContent(divName, content) {
document.getElementById(divName).innerHTML = content;
}
function fontSize(size) {
document.getElementById("lineOne").style.fontSize = size
}
function fontFamily(family) {
document.getElementById("lineOne").style.fontFamily = family
}
function fontStyle(style) {
document.getElementById("lineOne").style.fontStyle = style
}
function fontWeight(weight) {
document.getElementById("lineOne").style.fontWeight = weight
}
function align(align) {
document.getElementById("lineOne").style.textAlign = align;
}
function fontSize1(size1) {
document.getElementById("lineTwo").style.fontSize = size1
}
function fontFamily1(family1) {
document.getElementById("lineTwo").style.fontFamily = family1
}
function fontStyle1(style1) {
document.getElementById("lineTwo").style.fontStyle = style1
}
function fontWeight1(weight1) {
document.getElementById("lineTwo").style.fontWeight = weight1
}
function align1(align1) {
document.getElementById("lineTwo").style.textAlign = align1;
}
function fontSize2(size2) {
document.getElementById("lineThree").style.fontSize = size2
Solution
It is my first code review, I hope I'll be clear enough to explain why I would have done things like that.
Here is how I would have it done - I removed the setCookie part because it was not used in what you show to us - :
```
Edit text as needed
Text
Font Family
Size
Align
B
U
I
Arial
Comic Sans MS
Times New Roman
Verdana
6
8
10
12
14
16
18
20
left
center
right
Arial
Comic Sans MS
Times New Roman
Verdana
6
8
10
12
14
16
18
20
left
center
right
Arial
Comic Sans MS
Times New Roman
Verdana
6
8
10
12
14
16
18
20
left
center
right
Arial
Comic Sans MS
Times New Roman
Verdana
6
8
Here is how I would have it done - I removed the setCookie part because it was not used in what you show to us - :
```
Edit text as needed
Text
Font Family
Size
Align
B
U
I
Arial
Comic Sans MS
Times New Roman
Verdana
6
8
10
12
14
16
18
20
left
center
right
Arial
Comic Sans MS
Times New Roman
Verdana
6
8
10
12
14
16
18
20
left
center
right
Arial
Comic Sans MS
Times New Roman
Verdana
6
8
10
12
14
16
18
20
left
center
right
Arial
Comic Sans MS
Times New Roman
Verdana
6
8
Code Snippets
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=windows-1252">
<link rel="stylesheet" type="text/css" href="style.css">
</head>
<body>
<div id="edit">
<h1>Edit text as needed</h1>
<form name="myForm" enctype="multipart/form-data">
<table border="0" width="100%" style="border-collapse: collapse">
<tr>
<td width="278">Text</td>
<td width="165">Font Family</td>
<td width="74">Size</td>
<td width="86">Align</td>
<td><b>B</b></td>
<td><u>U</u></td>
<td><i>I</i></td>
</tr>
<tr>
<td>
<input name="myContent"></input>
<input type="button" value="Set content" onclick="DOMModifier.setContent('lineOne', document.myForm.myContent.value);">
</td>
<td width="165">
<select id="fontFamilyChanger" onchange="DOMModifier.setFontFamily('lineOne', this.value);">
<option value="Arial">Arial</option>
<option value="Comic Sans MS">Comic Sans MS</option>
<option value="serif" selected="selected">Times New Roman</option>
<option value="Verdana">Verdana</option>
</select>
</td>
<td width="74">
<select name="fontSizeChanger" onchange="DOMModifier.setFontSize('lineOne', this.value);">
<option value="6">6</option>
<option value="8">8</option>
<option value="10">10</option>
<option value="12" selected="selected">12</option>
<option value="14">14</option>
<option value="16">16</option>
<option value="18">18</option>
<option value="20">20</option>
</select>
</td>
<td width="75">
<select id="textAlignChanger" onchange="DOMModifier.setAlign('lineOne', this.value);">
<option value="left">left</option>
<option value="center" selected="selected">center</option>
<option value="right">right</option>
</select>
</td>
<td>
<input type="checkbox" onclick="DOMModifier.toggleBold('lineOne', this);">
var DOMModifier = {
// Method that get an element by is identifier - don't repeat document.getElementById hundred times in your code.
// The method cache elements already retrieved from the DOM.
// It throws an error if an element does not exist.
getElement : (function() {
var elements = {}; // private - element collection, allow to cache DOM acces to div
return function(identifier) {
// check if element was alredy retrieved
if ( typeof elements[identifier] === 'undefined' || elements[identifier] === null) {
// if not, store it
elements[identifier] = document.getElementById(identifier);
if ( elements[identifier] === null ) // throw an error if requested element does not exists
throw new Error('Element ' + identifier + ' does not exists');
}
return elements[identifier];
};
}()),
// Set content
setContent: function(targetIdentifier, content) {
this.getElement(targetIdentifier).innerHTML = content; // innerHTML on some elements is read-only in Internet Explorer!
},
// Add content
addContent: function(targetIdentifier, content) {
this.getElement(targetIdentifier).innerHTML += content;
},
// Set style
setStyle: function(targetIdentifier, style, value) {
this.getElement(targetIdentifier).style[style] = value;
},
//Modifiers - could have used content of thoses functions directly in the "onchange" proprerty,
//but using functions is more maintainable and evolvable.
// ie: add check functions before doing a setStyle
setColor: function(target, value) {
this.setStyle(target, 'color', value);
},
setBorder: function(target, value) {
this.setStyle(target, 'border', value);
},
setFontSize: function(target, value) {
this.setStyle(target, 'fontSize', value);
},
setFontFamily: function(target, value) {
this.setStyle(target, 'fontFamily', value);
},
setFontStyle: function(target, value) {
this.setStyle(target, 'fontStyle', value);
},
setFontWeight: function(target, value) {
this.setStyle(target, 'fontWeight', value);
},
setAlign: function(target, value) {
this.setStyle(target, 'textAlign', value);
},
setBold: function(target, value) {
this.setStyle(target, 'fontWeight', value);
},
setUnderline: function(target, value) {
this.setStyle(target, 'textDecoration', value);
},
setItalic: function(target, value) {
this.setStyle(target, 'fontStyle', value);
},
toggleBold: function(target, element) {
this.setBold(target, element.checked === true ? 'bold' : 'normal');
},
toggleUnderline: function(target, element) {
this.setUnderline(target, element.checked === true ? 'underline' : 'none');
},
toggleItalic: function(target, element) {
Context
StackExchange Code Review Q#8650, answer score: 6
Revisions (0)
No revisions yet.