8
\$\begingroup\$

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.

    <SCRIPT LANGUAGE="JavaScript">
        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 fontFamily2(family2) {
            document.getElementById("lineThree").style.fontFamily = family2
        }

        function fontStyle2(style2) {
            document.getElementById("lineThree").style.fontStyle = style2
        }

        function fontWeight2(weight2) {
            document.getElementById("lineThree").style.fontWeight = weight2
        }

        function align2(align2) {
            document.getElementById("lineThree").style.textAlign = align2;
        }

        function fontSize3(size3) {
            document.getElementById("lineFour").style.fontSize = size3
        }

        function fontFamily3(family3) {
            document.getElementById("lineFour").style.fontFamily = family3
        }

        function fontStyle3(style3) {
            document.getElementById("lineFour").style.fontStyle = style3
        }

        function fontWeight3(weight3) {
            document.getElementById("lineFour").style.fontWeight = weight3
        }

        function align3(align3) {
            document.getElementById("lineFour").style.textAlign = align3;
        }

        function border(border) {
            document.getElementById("myDiv").style.border = border;
        }

        function boldText(checkBox,target) {
            if(checkBox.checked) {
                document.getElementById(target).style.fontWeight = "bold";
            }
            else {
                document.getElementById(target).style.fontWeight = "normal";
            }
        }

        function underlineText(checkBox,target) {
            if(checkBox.checked) {
                document.getElementById(target).style.textDecoration = "underline";
            }
            else {
                document.getElementById(target).style.textDecoration = "none";
            }
        }

        function italic(checkBox,target) {
            if(checkBox.checked){
                document.getElementById(target).style.fontStyle = "italic";
            }
            else {
                document.getElementById(target).style.fontStyle = "normal";
            }
        }
    </SCRIPT>
</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="Add content" onClick="addContent('lineOne',     document.myForm.myContent.value); setCookie('content',     document.myForm.myContent.value, 7);">
                    </td>

                    <td width="165">
                        <select id="fontFamilyChanger" onchange="fontFamily(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="fontSize(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="align(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="boldText(this,'lineOne')">
                    </td>

                    <td>
                        <input type="checkbox" onclick="underlineText(this,'lineOne')">
                    </td>

                    <td>
                        <input type="checkbox" onclick="italic(this,'lineOne')">
                    </td>
                </tr>

                <tr>
                    <td>
                        <input name="myContent1"></input>
                        <input type="button" value="Add content" onClick="addContent('lineTwo', document.myForm.myContent1.value); setCookie('content', document.myForm.myContent1.value, 7);">
                    </td>

                    <td width="165">
                        <select id="fontFamilyChanger" onchange="fontFamily1(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="fontSize1(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="align1(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="boldText(this,'lineTwo')">
                    </td>

                    <td>
                        <input type="checkbox" onclick="underlineText(this,'lineTwo')">
                    </td>

                    <td>
                        <input type="checkbox" onclick="italic(this,'lineTwo')">
                    </td>
                </tr>

                <tr>
                    <td>
                        <input name="myContent2"></input>
                        <input type="button" value="Add content" onClick="addContent('lineThree', document.myForm.myContent2.value); setCookie('content', document.myForm.myContent2.value, 7);">
                    </td>

                    <td width="165">
                        <select id="fontFamilyChanger" onchange="fontFamily2(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="fontSize2(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="align2(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="boldText(this,'lineThree')">
                    </td>

                    <td>
                        <input type="checkbox" onclick="underlineText(this,'lineThree')">
                    </td>

                    <td>
                        <input type="checkbox" onclick="italic(this,'lineThree')">
                    </td>
                </tr>

                <tr>
                    <td>
                        <input name="myContent3"></input>
                        <input type="button" value="Add content" onClick="addContent('lineFour', document.myForm.myContent3.value); setCookie('content', document.myForm.myContent3.value, 7);">
                    </td>

                    <td width="165">
                        <select id="fontFamilyChanger" onchange="fontFamily3(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="fontSize3(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="align3(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="boldText(this,'lineFour')">
                    </td>

                    <td>
                        <input type="checkbox" onclick="underlineText(this,'lineFour')">
                    </td>

                    <td>
                        <input type="checkbox" onclick="italic(this,'lineFour')">
                    </td>
                </tr>
            </table>
        </div>

        <br>
        Colour:
        <select id="color" onclick="setColor();">
            <option value="white">white</option>
            <option value="black" selected="selected">black</option>
            <option value="red">red</option>
            <option value="lightblue">light blue</option>
            <option value="darkblue">dark blue</option>
            <option value="lightgreen">light green</option>
            <option value="darkgreen">dark green</option>
            <option value="yellow">yellow</option>
            <option value="orange">orange</option>
            <option value="pink">pink</option>
            <option value="purple">purple</option>
            <option value="gray">gray</option>
        </select>

        <select id="border"  onchange="border(this.value);">
            <option value="1px solid" selected="selected">1px</option>
            <option value="2px solid">2px</option>
            <option value="3px solid">3px</option>
            <option value="4px solid">4px</option>
            <option value="5px solid">5px</option>
        </select>
    </form>

    <br>
    <div id="myDiv">
        <div id="lineOne"></div>
        <div id="lineTwo"></div>
        <div id="lineThree"></div>
        <div id="lineFour"></div>
    </div>

http://jsfiddle.net/ShauniD/ELER2/

Have a look and let me know!

I need coding advice with this code. Please go easy as this is my first code in JavaScript, and I need good criticism.

\$\endgroup\$
  • \$\begingroup\$ I have included the code for you to see (As the FAQ says) \$\endgroup\$ – Shauni Daniels Feb 4 '12 at 12:29
  • \$\begingroup\$ For the font size to work in Chrome I had to append + 'pt' in the fontSize function. On a side note, did you consider using a library like jQuery to make the code a bit shorter and less repeating? On the other hand, using a library might actually take away a bit of the language learning process. Just wanted to suggest this to help you avoid writing repeated parts of code for each row. \$\endgroup\$ – pimvdb Feb 4 '12 at 12:41
  • \$\begingroup\$ I have not thought of using jquery, as you said, it will hinder the learning process. Once i get javascript down, i would assume jquery will be my next step. I need to find out if this script is how it should be done in javascript, if there is an easier way around writing code for each line (in javascript), and how to get the border to show around the containing div? \$\endgroup\$ – Shauni Daniels Feb 4 '12 at 12:46
6
\$\begingroup\$

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 - :

<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);">
                        </td>

                        <td>
                            <input type="checkbox" onclick="DOMModifier.toggleUnderline('lineOne',  this);">
                        </td>

                        <td>
                            <input type="checkbox" onclick="DOMModifier.toggleItalic('lineOne',  this);">
                        </td>
                    </tr>

                    <tr>
                        <td>
                            <input name="myContent1"></input>
                            <input type="button" value="Set content" onclick="DOMModifier.setContent('lineTwo', document.myForm.myContent1.value);">
                        </td>

                        <td width="165">
                            <select id="fontFamilyChanger" onchange="DOMModifier.setFontFamily('lineTwo', 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('lineTwo', 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('lineTwo', 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('lineTwo',  this)">
                        </td>

                        <td>
                            <input type="checkbox" onclick="DOMModifier.toggleUnderline('lineTwo',  this)">
                        </td>

                        <td>
                            <input type="checkbox" onclick="DOMModifier.toggleItalic('lineTwo',  this)">
                        </td>
                    </tr>

                    <tr>
                        <td>
                            <input name="myContent2"></input>
                            <input type="button" value="Set content" onclick="DOMModifier.setContent('lineThree', document.myForm.myContent2.value);">
                        </td>

                        <td width="165">
                            <select id="fontFamilyChanger" onchange="DOMModifier.setFontFamily('lineThree', 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('lineThree', 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('lineThree', 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('lineThree', this);">
                        </td>

                        <td>
                            <input type="checkbox" onclick="DOMModifier.toggleUnderline('lineThree', this);">
                        </td>

                        <td>
                            <input type="checkbox" onclick="DOMModifier.toggleItalic('lineThree', this);">
                        </td>
                    </tr>

                    <tr>
                        <td>
                            <input name="myContent3"></input>
                            <input type="button" value="Set content" onclick="DOMModifier.setContent('lineFour', document.myForm.myContent3.value);">
                        </td>

                        <td width="165">
                            <select id="fontFamilyChanger" onchange="DOMModifier.setFontFamily('lineFour', 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('lineFour', 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('lineFour', 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('lineFour', this);">
                        </td>

                        <td>
                            <input type="checkbox" onclick="DOMModifier.toggleUnderline('lineFour', this);">
                        </td>

                        <td>
                            <input type="checkbox" onclick="DOMModifier.toggleItalic('lineFour', this);">
                        </td>
                    </tr>
                </table>


                <br/>
                <span>Colour:</span>
                <select id="color" onchange="DOMModifier.setColor('myDiv', this.value);">
                    <option value="white">white</option>
                    <option value="black" selected="selected">black</option>
                    <option value="red">red</option>
                    <option value="lightblue">light blue</option>
                    <option value="darkblue">dark blue</option>
                    <option value="lightgreen">light green</option>
                    <option value="darkgreen">dark green</option>
                    <option value="yellow">yellow</option>
                    <option value="orange">orange</option>
                    <option value="pink">pink</option>
                    <option value="purple">purple</option>
                    <option value="gray">gray</option>
                </select>

                <select id="border"  onchange="DOMModifier.setBorder('myDiv', this.value);">
                    <option value="1px solid" selected="selected">1px</option>
                    <option value="2px solid">2px</option>
                    <option value="3px solid">3px</option>
                    <option value="4px solid">4px</option>
                    <option value="5px solid">5px</option>
                </select>
            </form>
        </div>
        <br/>
        <div id="myDiv">
            <div id="lineOne"></div>
            <div id="lineTwo"></div>
            <div id="lineThree"></div>
            <div id="lineFour"></div>
        </div>

        <script type="text/javascript">

            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 retrieven from the DOM
                // throw error if element does not exists
                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 it do 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 readonly in IE !
                },

                // 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 "onchange" proprerty
                //but using functions is more maintainable and evoluable
                // 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) {
                    this.setItalic(target, element.checked === true ? 'italic' : 'normal');
                }
            };
        </script>
    </body>
</html>

Just check at the more important thing, the JavaScript code:

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) {
        this.setItalic(target, element.checked === true ? 'italic' : 'normal');
    }
};

Let's talk about what differs from yours:

  • I declared one global object (DOMModifier) containing all the functions. This, in order to not populate the Global scope with a lot of functions.
  • I made the functions "generics" like you did with yours boldText, italic and underlineText. To do that, we just have to pass the target identifier to the function instead of creating one function by target.
  • I created a generic function getElement which cache the DOM elements access. So, for one identifier, it will only call the DOM function "getElementById" the first time, and store the result (or throw an error if it does not exist). Every other call in order to retrieve this element will just return the cached one.
  • I created a generic function setStyle that is used by all the others function which are made to change a style.
  • I changed some function names:
    • addContent become setContent because it was, in fact, setting content. I added an addContent that add content.
    • I added the keyword "set" before a lot of function names to make the code "speaking" about what it is doing.
  • I added three functions: toggleBold, toggleUnderline and toggleItalic and I used them on the checkbox click event, beacause those event don't just set a style, but set or unset -litterally speaking- according to the state of the checkbox.
  • I putted the JavaScript part at the end of the HTML, just before the body close, to ensure that all DOM elements are loaded before adding JavaScript features.

Now let's talk about the JavaScript part in the onchange and onclick properties:

  • Now that I have a global object instead of global functions, I call every function on the object - that is, DOMModifier.setFontFamily() with required arguments (target, value, etc.)

Now what I did can really be improved by:

  • Adding JavaScript EventListeners to elements when the DOM is loaded instead of using the onchange and onclick properties. It will separate HTML and JavaScript code, be more flexible, avoid errors, add features only when the DOM is ready to handle them...
  • Modify some functions to make it cross-browser (that is, innerHTML on some elements won't work in Internet Explorer)
  • Test and modify it to add features, make some actions more generics, etc...

Just remember:

  • What I changed here is good (I hope) for what I wanted it to do, in an other context, maybe I would have make it different, more flexible, more generic, more intrusive for objects nature, etc...
\$\endgroup\$
  • \$\begingroup\$ Thank you for all the work you have put into helping me with learning. I reaally to appreciate it. I am taking each piece you modified and studying it in comparieson with my (not so good)code. Thank you. \$\endgroup\$ – Shauni Daniels Feb 4 '12 at 17:49
  • \$\begingroup\$ If you have any question, i'll be happy to answer. \$\endgroup\$ – dievardump Feb 4 '12 at 18:00
  • \$\begingroup\$ Thank you for offering. I am going through the code and will definatlely ask if i am not sure of anything. \$\endgroup\$ – Shauni Daniels Feb 4 '12 at 18:08
  • \$\begingroup\$ I am still really baffled as to how to get the border to show up from a checkbox, but it must be the same colour as the selection made for the font colour. As this is a stamp, only one colour can be on it, and having a checkbox for border or no border seems simple enough, however i need it to take the colour of the text colour selection. \$\endgroup\$ – Shauni Daniels Feb 4 '12 at 20:05
  • \$\begingroup\$ You want to have the possibility to add a border on a line ? \$\endgroup\$ – dievardump Feb 4 '12 at 20:27
3
\$\begingroup\$

Not in the JavaScript part, but the HTML is not well-formed. Excerpt:

<div id="edit">

    <form name="myForm" enctype="multipart/form-data">

</div>

    </form>

The elements div and form overlap.

The intended structure is probably:

<div id="edit">

    <form name="myForm" enctype="multipart/form-data">

    </form>

</div>
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.