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

Stamp order / preview form for a site

Submitted by: @import:stackexchange-codereview··
0
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

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

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.