HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Multiple jQuery onClick events

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
eventsjquerymultipleonclick

Problem

I have a ton of jQuery onClick events. onClick I hide/show different UI elements. I was wondering, how can I tidy the code up and make multiple onClick events more readable?

$('.info_2').on('click', function() {
  $('#nav-wrapper').toggleClass('hidden_nav');
  $('#card-wrapper').toggleClass('centre_share');
  $('.E_info').toggleClass('display');
  $('#info-btn').css('opacity', '0');
  $('#nav-wrapper').delay(300).toggleClass('hidden');
  $('#nav-wrapper').removeClass('display_nav');
  $('#nav-wrapper').removeClass('display');
});

$('.info_back').on('click', function() {
  $('#nav-wrapper').removeClass('hidden_nav');
  $('#nav-wrapper').addClass('display_nav');
  $('#nav-wrapper').addClass('display');
  $('#info-btn').css('opacity', '1');
  $('#nav-wrapper').removeClass('hidden');
  $('.E_info').removeClass('display');
  $('.E_info').addClass('hidden');
  $('#card-wrapper').removeClass('centre_share');
});

$('#info-btn').on('click', function(){
  $('#info-btn').toggleClass('close_btn');
  $('.o-card_border').toggleClass('info_display card_active');
  $('.start_title').toggleClass('hidden remove_flow')
  $('#svg_full').attr('class', 'test');
  $('#svg_top').attr('class', 'test');
  $('#svg_bot').attr('class', 'test');
  $('#svg_bot_bot').attr('class', 'test');
  $('#svg_bot_right').attr('class', 'test');
  $('.rectangle_style_frame3 display').toggleClass('hidden');
  $('.triangle_style').toggleClass('hidden');

  $('.bg-info').toggleClass('display');
  $('.info_CharactersInvolved').toggleClass('display');
  $('.info_themes').toggleClass('display');
  $('.E_info').toggleClass('display');
});


The code works fine. I just think it looks really ugly. And the readability of it is painful, especially if you're trying to jump onto the project and learn the codebase.

Solution

The worst issues are cases of the same element being selected multiple times in order to apply various methods. Repeat selection of the same element should be avoided because selection of DOM element(s) is expensive.

There are also are cases where multiple selections have the same method applied. This isn't so bad for performance, but leads to unnecessarily bulky source.

The code can be improved with the following techniques :

  • using method chaining.



  • using comma separated selectors.



  • using $(this) to select the same element as the one to which an event handler is attached.



  • passing space-separated lists of class names to addClass() and .removeClass().



You might end up with something like this :

$('.info_2').on('click', function() {
    $('#card-wrapper').toggleClass('centre_share');
    $('.E_info').toggleClass('display');
    $('#info-btn').css('opacity', '0');
    $('#nav-wrapper').toggleClass('hidden_nav').delay(300).toggleClass('hidden').removeClass('display_nav').removeClass('display');
});

$('.info_back').on('click', function() {
    $('#nav-wrapper').removeClass('hidden_nav hidden').addClass('display_nav display');
    $('#info-btn').css('opacity', '1');
    $('.E_info').removeClass('display').addClass('hidden');
    $('#card-wrapper').removeClass('centre_share');
});

$('#info-btn').on('click', function(){
    $(this).toggleClass('close_btn');
    $('.o-card_border').toggleClass('info_display card_active');
    $('.start_title').toggleClass('hidden remove_flow');
    $('#svg_full, #svg_top, #svg_bot, #svg_bot_bot, #svg_bot_right').attr('class', 'test');
    $('.rectangle_style_frame3 display, .triangle_style').toggleClass('hidden');
    $('.bg-info, .info_CharactersInvolved, .info_themes, .E_info').toggleClass('display');
});

Code Snippets

$('.info_2').on('click', function() {
    $('#card-wrapper').toggleClass('centre_share');
    $('.E_info').toggleClass('display');
    $('#info-btn').css('opacity', '0');
    $('#nav-wrapper').toggleClass('hidden_nav').delay(300).toggleClass('hidden').removeClass('display_nav').removeClass('display');
});

$('.info_back').on('click', function() {
    $('#nav-wrapper').removeClass('hidden_nav hidden').addClass('display_nav display');
    $('#info-btn').css('opacity', '1');
    $('.E_info').removeClass('display').addClass('hidden');
    $('#card-wrapper').removeClass('centre_share');
});

$('#info-btn').on('click', function(){
    $(this).toggleClass('close_btn');
    $('.o-card_border').toggleClass('info_display card_active');
    $('.start_title').toggleClass('hidden remove_flow');
    $('#svg_full, #svg_top, #svg_bot, #svg_bot_bot, #svg_bot_right').attr('class', 'test');
    $('.rectangle_style_frame3 display, .triangle_style').toggleClass('hidden');
    $('.bg-info, .info_CharactersInvolved, .info_themes, .E_info').toggleClass('display');
});

Context

StackExchange Code Review Q#119390, answer score: 2

Revisions (0)

No revisions yet.