8
\$\begingroup\$

Reason for script:

dir="auto" is an attribute value from the HTML 5 spec with current poor support in IE and Opera browsers. The project I am working on only cares about IE, Firefox and Safari browsers and handles text in both left to right LTR and right to left RTL languages. For cases where the directionality of the text coming from the data source is unkown, I would like to use dir="auto" to handle most cases of directionality. For IE, I would like to simulate this functionality using JavaScript.

Goal of review:

I am hopeful that a review of my work will identify areas where I can improve the code to be more in line with its stated purpose as well as aligning the code to best practices and identifying any bugs that may exist.

Implementation:

  • An isLTR function was used as I figured most use cases would be against LTR text as opposed to RTL.

  • In my research, I wandered across a script by Nicholas Zakas which I thought would be useful and have integrated it into my solution.

  • The project uses Dojo 1.8.1.

XHTML:

Script call in head tag after call to Dojo library:

<!--[if IE]><script type="text/javascript" src="./PatternsTools/PatternsBidi.js"></script><![endif]-->

JavaScript:

window.Patterns = window.Patterns || {};
window.Patterns.Bidi = window.Patterns.Bidi || {};
window.Patterns.Bidi.Strings = window.Patterns.Bidi.Strings || {};
window.Patterns.Bidi.Strings.LTRcharacters = "\u0041-\u005A\u0061-\u007A\u00C0-\u00D6\u00D8-\u00F6\u00F8-\u02B8\u0300-\u0590\u0800-\u1FFF\u200E\u202A\u202D\u2C00-\uFB1C\uFDFE-\uFE6F\uFEFD-\uFFFF";
window.Patterns.Bidi.Strings.RTLcharacters = "\u0591-\u07FF\u200F\u202B\u202E\uFB1D-\uFDFD\uFE70-\uFEFC";
window.Patterns.Bidi.Strings.NeutralWeakCharacters = "\u0000-\u0040\u005B-\u0060\u007B-\u00BF\u00D7\u00F7\u02B9-\u02FF\u2000-\u2BFF\u2010-\u2029\u202C\u202F-\u2BFF";

(function(/*Class Object*/ Bidi){
    function getDirection(/*HTML Element*/ element) {
    /*Copyright 2010 Nicholas C. Zakas. All rights reserved.*/
    /*MIT Licensed*/
        var result = null;
        if (element){
            if (window.getComputedStyle){
                result = window.getComputedStyle(element,null).direction;
            } else if (element.currentStyle){
                result = element.currentStyle.direction;
            }
        }

        return result;
    }

    function prependOverride(/*HTML Element*/ element, /*Boolean*/ isLTR) {
        var direction = getDirection(element) === "ltr";
        if (isLTR !== direction) {
            element.innerHTML = (isLTR ? "&#8237;" : "&#8238;") + element.innerHTML + "&#8236;";
        }
    }

    function isLTR(/*String*/ s) {           
        var ltrDirCheck = new RegExp("^[" + Bidi.Strings.NeutralWeakCharacters + "]*?[" + Bidi.Strings.LTRcharacters + "]");
        return ltrDirCheck.test(s);
    };

    (function init() {
        require(["dojo/ready", "dojo/_base/array"], function(/*Function*/ ready, /*Object*/ array) {
            ready(function() {
                array.forEach(dojo.query("[dir=auto]"), function(/*HTML Element*/ element, /*Integer*/ i) {
                    prependOverride(element, isLTR(element.innerHTML));
                });
            });
        });
    })();
})(window.Patterns.Bidi);
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

My 2 cents:

  • Code looks really good overall
  • In prependOverride, the naming is unfortunate
    • The parameter isLTR is really what you request, not what it is
    • The var direction really is a boolean, not the direction of the element
    • If you are going to /** type **/ variables, you might as well go Spartan and use e instead of element
  • prependOverride has 4 magical constants
  • isLTR should cache the regex, not rebuild it in every call

for prependOverride the following might be cleaner:

function prependOverride(/*HTML Element*/ e, /*Boolean*/ requiresLTR) {
    var hasLTR = getDirection(e) === "ltr";
    if (hasLTR !== requiresLTR) {
        e.innerHTML = (requiresLTR? "&#8237;" : "&#8238;") + e.innerHTML + "&#8236;";
    }
}

or, even you could pass the direction you require

function prependOverride(/*HTML Element*/ e, /*String*/ requiredLTR) {
    var actualLTR = getDirection(e);
    if (actualLTR !== requiredLTR) {
        e.innerHTML = (requiredLTR=="ltr"? "&#8237;" : "&#8238;") + e.innerHTML + "&#8236;";
    }
}

I will leave the naming of the magical constants to you.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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