3
\$\begingroup\$

I have a button (or icon) which can have three states.

On hover, it changes state and on click, the new state gets transferred to another state. This state is kept until you click again for which it gets back to normal.

The code is working, but it does not look or feel good. Is there any way I can re-factor this?

HTML

<div class="rendering-expert request-list-supported">
    <i class="fa fa-cog">1</i>
    <a class="hidden" rel="request-list-add" href="/add/1">
        <i class="icon fa fa-plus-circle">2</i>
    </a>
    <a class="hidden" rel="request-list-remove" href="/remove/1">
        <i class="icon fa fa-check-circle">3</i>
    </a>
</div>

CSS

.hidden { display: none; }

JQuery

$(function() { 
    $(".rendering-expert.request-list-supported").hover(function() {
        if (!$(this).hasClass("has-request")) {
            $(this).find(".fa-cog").hide();
            $(this).find("[rel='request-list-add']").removeClass("hidden");
        }
    }, function() {
        if (!$(this).hasClass("has-request")) {
            $(this).find(".fa-cog").show();
            $(this).find("[rel='request-list-add']").addClass("hidden");
        }
    }).on("click", "[rel='request-list-add']", function(e) {
        e.preventDefault();
        var toggler = $(this);
        var href = toggler.attr("href");
        toggler.parent().find(".fa-cog").hide();
        toggler.next("[rel='request-list-remove']").removeClass("hidden");
        toggler.toggleClass("hidden");
        toggler.parent().toggleClass("has-request");
    }).on("click", "[rel='request-list-remove']", function(e) {
        e.preventDefault();
        var toggler = $(this);
        var href = toggler.attr("href");
         toggler.parent().find(".fa-cog").show();
         toggler.prev("[rel='request-list-remove']").addClass("hidden");
         toggler.addClass("hidden");
         toggler.parent().removeClass("has-request");
    });
});

Please note that some parts are stripped in this example. It will only change state in my production if an AJAX request succeeds.

\$\endgroup\$
  • \$\begingroup\$ Is there a semantic rationale for writing the HTML that way (<div> with three children)? Would it also be acceptable to just have one element whose class changes for each state? \$\endgroup\$ – 200_success Aug 15 '14 at 6:12
  • \$\begingroup\$ The div has a lot of other properties. Just stripped a lot for this example \$\endgroup\$ – janhartmann Aug 15 '14 at 6:20
  • \$\begingroup\$ May I suggest that you post the real thing, or something very close to it? There's no sense seeking advice that won't be useful to you. \$\endgroup\$ – 200_success Aug 15 '14 at 6:23
  • \$\begingroup\$ Hi again, see: jsfiddle.net/6qwt2vf8/1 \$\endgroup\$ – janhartmann Aug 15 '14 at 6:32
  • \$\begingroup\$ There should be a functionaæ \$\endgroup\$ – janhartmann Aug 15 '14 at 15:21
2
\$\begingroup\$

Since jquery event handler is a memory and processing "eater" because has to listen for the events then I think is better to delegate the hover responsibility to css. The change is just remove class="hidden" from your elements and then set the right selectors to user display:none to hide or display:inline to show - I use inline instead of block since it is applied to anchor tag and the default behavior for this tag is inline.

With this change then javascript can be simplified and the hover handlers can be removed.

BTW you still have to implement the click handler to add the has-request class.

The code should look likes the following,

In your css:

.rendering.rendering-expert.request-list-supported:hover [rel=request-list-add],
.rendering.rendering-expert.request-list-supported.has-request [rel=request-list-remove] 
{
    display: inline
}
.rendering.rendering-expert.request-list-supported [rel=request-list-add],
.rendering.rendering-expert.request-list-supported [rel=request-list-remove],
.rendering.rendering-expert.request-list-supported:hover .fa-cog,
.rendering.rendering-expert.request-list-supported.has-request .fa-cog,
.rendering.rendering-expert.request-list-supported.has-request [rel=request-list-add]
{
    display: none
}

And then replace your js:

$(function() { 
    $(".rendering-expert.request-list-supported").on("click", function(e) {
        e.preventDefault();
        var toggler = $(this);
        toggler.toggleClass("has-request");
    });
});
|improve this answer|||||
\$\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.