JSFiddle: http://jsfiddle.net/yMSHa/ (Note: Real code uses JQuery 1.4.1)HTML:<div data-name=action data-id=1>Do It A 1</div><div data-name=action data-id=2>Do It A 2</div><div data-name=action data-id=3>Do It A 3</div><div data-name=action data-id=4>Do It A 4</div><div data-name=action data-id=5>Do It A 5</div><div style=display:none;>    <div id=DoIt2>Do It B</div></div>JS:var RepDiv = $('#DoIt2');var OldDiv = null;$('div[data-name=action]').each(function (i, f) {    var actionid = $(f).attr('data-id');    if (actionid) {        $(f).css('cursor', 'pointer');        var OnClick = function () {            if (OldDiv) {                RepDiv.replaceWith(OldDiv);                OldDiv.click(OnClick);            }            OldDiv = $(this).replaceWith(RepDiv);        };        $(f).click(OnClick);    }});Below is some simple JavaScript which replaces one node at a time when that node is clicked.  The idea behind this code is that a user can click on a node for that node to be expanded with more options.I'm looking for a general review.  My main concern is whether this code is written properly in terms of good practices.
Is this node replacement code reasonable?
javascript;jquery
From a once over;i and f, even for forEach are not very good names, pick something more meaningful.if data-id is the action id, perhaps you could have gotten/set it with $().data('actionId') ?The way your replacement function uses OldDiv and RepDiv smells like global variable abuseConsider using smallCamelCase, actionid -> actionId, OldDiv -> oldDivConsider caching $(f), since you access it several times ( var $f = $(f); and then only access $f )This : $(f).css('cursor', 'pointer'); really belongs in .css fileOther than that, your code looks okay to me. 