0
\$\begingroup\$

I'm using the following code to change elements text and classes when a click event is fired. My question is: Is there a better way to go about doing this that limits how repetitive the functions are? It seems extremely messy and like there would be a better way to go about structuring this.

function updateAveragesDropdown(element, text) {
    // Update text
    $("#timeframe_dropdown_container .dropdown-toggle p").text(text);

    // Remove active class
    $("#timeframe_dropdown_container #dropdown_today").parent("li").removeClass("active");
    $("#timeframe_dropdown_container #dropdown_week").parent("li").removeClass("active");
    $("#timeframe_dropdown_container #dropdown_month").parent("li").removeClass("active");

    // Update nav-tabs
    $("#timeframe_pills_container #tab_today").parent("li").removeClass("active");
    $("#timeframe_pills_container #tab_week").parent("li").removeClass("active");
    $("#timeframe_pills_container #tab_month").parent("li").removeClass("active");

    // Add active class
    $(element).parent("li").addClass("active");
}

function removeDropdownClasses() {
    $("#timeframe_dropdown_container #dropdown_today").parent("li").removeClass("active");
    $("#timeframe_dropdown_container #dropdown_week").parent("li").removeClass("active");
    $("#timeframe_dropdown_container #dropdown_month").parent("li").removeClass("active");
}

function updateDropdownElement(element, text) {
     $(element).parent("li").addClass("active");
     $("#timeframe_dropdown_container .dropdown-toggle p").text(text);
}

// Clear active labels on dropdown menu
$("#timeframe_dropdown_container #dropdown_today").click(function(e) {
    updateAveragesDropdown(this, "Today");
    $("#timeframe_pills_container #tab_today").parent("li").addClass("active");
});

$("#timeframe_dropdown_container #dropdown_week").click(function(e) {
    updateAveragesDropdown(this, "7-Day");
    $("#timeframe_pills_container #tab_week").parent("li").addClass("active");
});

$("#timeframe_dropdown_container #dropdown_month").click(function(e) {
    updateAveragesDropdown(this, "30-Day");
    $("#timeframe_pills_container #tab_month").parent("li").addClass("active");
});

$("#timeframe_pills_container #tab_today").click(function(e) {
    removeDropdownClasses();
    updateDropdownElement("#timeframe_dropdown_container #dropdown_today", "Today");
});

$("#timeframe_pills_container #tab_week").click(function(e) {
    removeDropdownClasses();
    updateDropdownElement("#timeframe_dropdown_container #dropdown_week", "7-Day");
});

$("#timeframe_pills_container #tab_month").click(function(e) {
    removeDropdownClasses();
    updateDropdownElement("#timeframe_dropdown_container #dropdown_month", "30-Day");
});

HTML

<div class="col-lg-6 col-md-6 col-sm-6 col-xs-6">
    <ul class="nav nav-pills" id="timeframe_pills_container">
      <li class="active"><a data-toggle="tab" href="#today_tab" id="tab_today">Today</a></li>
      <li><a data-toggle="tab" href="#week_tab" id="tab_week">7-Day</a></li>
      <li><a data-toggle="tab" href="#month_tab" id="tab_month">30-Day</a></li>
    </ul>
    <div class="dropdown" id="timeframe_dropdown_container">
        <button class="btn btn-default dropdown-toggle" type="button" id="timeframe_dropdown" data-toggle="dropdown" aria-haspopup="true" aria-expanded="true">
            <p>Today</p>
            <span class="caret"></span>
        </button>
        <ul class="dropdown-menu dropdown-menu-right" aria-labelledby="timeframe_dropdown">
            <li class="active" data-toggle="tab" href="#today_tab"><a id="dropdown_today">Today</a></li>
            <li data-toggle="tab" href="#week_tab" ><a id="dropdown_week">7-Day</a></li>
            <li data-toggle="tab" href="#month_tab" ><a id="dropdown_month">30-Day</a></li>
        </ul>
    </div>
</div>
\$\endgroup\$
  • \$\begingroup\$ Please show your HTML structure. My guess is that a lot of your repetitive code might be related to how you have your elements structured in DOM. You may need both changes to HTML and javascript to improve your solution. \$\endgroup\$ – Mike Brant Jun 13 '17 at 18:51
  • \$\begingroup\$ @MikeBrant See updated post please. \$\endgroup\$ – Joe Scotto Jun 13 '17 at 18:52
  • \$\begingroup\$ Are you using any JQuery plugins? I ask because the output HTML is reminiscent of a control that is rendering the HTML. \$\endgroup\$ – googabeast Jul 2 '17 at 1:09
1
\$\begingroup\$

As far as I can tell, the code simplifies as follows :

$("#timeframe_pills_container li a").click(function(e) {
  var $li = $(this).closest("li");
  var index = $li.index();
  var $dropdown = $("#timeframe_dropdown_container");
  $li.addClass("active").siblings().removeClass("active");
  $dropdown.find("li").eq(index).addClass("active").siblings().removeClass("active");
  $dropdown.find(".dropdown-toggle p").text($(this).text());
}).eq(0).click();

$("#timeframe_dropdown_container li a").click(function(e) {
  var index = $(this).closest("li").index();
  $("#timeframe_pills_container li a").eq(index).click();
});

.eq(0).click() establishes the intial state, so you don't need to set class="active" (twice) or the button text, in HTML.

\$\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.