Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I've been working with the RaphaelJS 'australia map' example that displays text in a DIV when a state is clicked on. However I'm having difficulty working with it to create different events and functions.

I want to be able to duplicate the 'display text' function for two different element onClick events (i.e. have two different buttons (elements) that do the same thing). But the current code is a bit too complicated for me. Can somebody please help me simplify the code that displays the text in a DIV when an element is clicked on.

Any help would be much appreciated - this is what I'm currently working with jsfiddle.

 window.onload = function () {
    var R = Raphael("paper", 532, 500);
    var attr = {
        fill: "#0B0",
        stroke: "#fff",
        "stroke-width": 3,
    };
    var ctx = {};
    ctx.btn_1 = R.path("M 180,130 L 340,130, L 392,212, L 128,212 z").attr(attr);
    ctx.btn_2 = R.path("M 128,212 L 392,212, L 435,280, L 85,280 z").attr(attr);

    var current = null;
    for (var state in ctx) {
        (function (st, state) {
            st[0].style.cursor = "pointer";
            st[0].onmousedown = function () {
                current && (document.getElementById(current).style.display = "");
                st.toFront();
                R.safari();
                document.getElementById(state).style.display = "block";
                current = state;
            };
        })(ctx[state], state);
    }
 };
share|improve this question
1  
That is actually reasonably compact, straightforward code. Which part were you hoping to simplify, exactly? One thought: if the click handler is what you're focusing on, you can always declare the function independently and assign it by variable name, rather than reproducing it inline repeatedly. – Kevin Nielsen Sep 10 '12 at 16:38
The part of code I'm hoping to simplify is the click handler (starting from var current = null; to the end of the code). The problem I'm having is trying to rebuild the click handler code - or even calling the function to be used elsewhere. – user1619342 Sep 11 '12 at 9:04
@MarkThomas I'll try codereview too, thanks. – user1619342 Sep 11 '12 at 9:06
You could user Paper.set and make an array of elements. I wouldn't add it to the context as class property. And btw, you can add 'cursor': 'pointer' to your attr. And at last: Don't use for..in, because it's very vulnerable, try Google :) – Dan Lee Sep 11 '12 at 9:41
Great, thanks Dan Lee. I'll look into those more. Google to the rescue! :) – user1619342 Sep 13 '12 at 9:51

migrated from stackoverflow.com Sep 11 '12 at 12:59

1 Answer

I gave it a shot.

window.onload = function () {
    var R = Raphael("paper", 532, 500),
        attrs = {
            fill: "#0B0",
            stroke: "#fff",
            "stroke-width": 3,
            cursor: "pointer"
        },
        paths = [
            {id: 'btn_1', p:"M 180,130 L 340,130, L 392,212, L 128,212 z"},
            {id: 'btn_2', p:"M 128,212 L 392,212, L 435,280, L 85,280 z"}
        ],
        current,
        max,
        i;

    for (i = 0, max = paths.length; i < max; i++) {
        initPath(paths[i].p, attrs, paths[i].id);
    }

    function initPath(pathStr, attrs, targetId) {

        var path = R.path(pathStr).attr(attrs),
            txElm = document.getElementById(targetId);

        path.click(function (e) {
             if (current) {
                 if (current === txElm) {
                     return;
                 }
                 current.style.display = '';
             }
             current = txElm;
             current.style.display = 'block';
             this.toFront();
             R.safari();
        });
     }
 }​

Integrated Dan Lee's comments and defined a function instead of making a new closure in each loop iteration.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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