current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 3 down vote favorite

I've a lot of code blocks that hide/show some block.

$('#Back').on('click', function () {
    $('#FormRow').hide();
    $('#TableRow').show();
});

$('#AddLanguageShow').on('click', function () {
    $('#FormRow').show();
    $("#TableRow").hide();
});


$('table').on('click', 'button.edit', function (evt) {
    $('#FormRow').show();
    $('#TableRow').hide();
    languageManager.load(getRowId($(this)));
});

Is it possible to split logic of changing blocks visibility ?

share|improve this question
1  
Can you provide us more code, it is not clear why you would show or hide stuff right now. –  konijn May 15 '14 at 18:21
    
Not sure if applicable to your case, but did you consider the toggle() function? api.jquery.com/toggle –  Pevara May 15 '14 at 19:09

3 Answers 3

up vote 3 down vote

Extract the duplicated code to a function:

function showOneHideOther(elToShow, elToHide) {
    $(elToShow).show();
    $(elToHide).hide();
}

You can reuse the function anyplace now:

$('#Back').on('click', function() {
    showOneHideOther('#TableRow', '#FormRow')
});
$('#AddLanguageShow').on('click', function() {
    showOneHideOther('#FormRow', '#TableRow')
});

$('table').on('click', 'button.edit', function (evt) {
    showOneHideOther('#FormRow', '#TableRow')
    languageManager.load(getRowId($(this)));
});
share|improve this answer
    
I believe this won't work. You're setting undefined to the handlers for #Back and #AddLanguageShow. –  Joseph the Dreamer May 15 '14 at 19:02
1  
You are right @JosephtheDreamer, I already fixed it. Thanks for pointing out my error! –  Juliano May 15 '14 at 19:15
up vote 1 down vote

You could do this:

var fn = {
  // Cache
  formRow : $('#FormRow'),
  tableRow : $('#TableRow'),

  // Factor out
  formHideTableShow : function(){
    this.formRow.hide();
    this.tableRow.show();
  },
  formShowTableHide : function(){
    this.formRow.show();
    this.tableRow.hide();
  }
}

$('#Back').on('click', function(){
  fn.formHideTableShow();
});

$('#AddLanguageShow').on('click', function(){
  fn.formShowTableHide();
});

$('table').on('click', 'button.edit', function (evt) {
    fn.formShowTableHide();
    languageManager.load(getRowId($(this)));
});
share|improve this answer
up vote 1 down vote

It looks like you have one set of interactions that show the form, and another that show the table. How about something like this?

var uiThatHidesTable = [$('#button1'), $('#button3')];
var uiThatHidesForm  = [$('#button2')];

var hideTable = function() { $('#TableRow').hide(); $('#FormRow').show(); }
var hideForm = function() { $('#FormRow').hide(); $('#TableRow').show(); }

uiThatHidesTable.forEach(function(el) { el.on('click', hideTable); });   
uiThatHidesForm.forEach(function(el) { el.on('click', hideForm); });

// you can still bind additional things to do out here    
$('#button2').on('click', function() { alert("pushed 2"); });

Fiddle here: http://jsfiddle.net/T7swj/

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.