4
\$\begingroup\$

I have a select box that in default state looks as default (white, etc) but when I select a different value, I want the entire table row to highlight in yellow.

I did this, but I want to make it better and more concise and more efficient. Can you help?

Also I will have several such boxes, not just one. Feel free to change/add/remove identifiers, I am looking for a better solution overall.

$(document).ready(function() {


  $("#closedRow").on('change', function() {
    if ($("#closedRow").val() != 0)
      $("#trRow").css('background-color', 'yellow');
    else
      $("#trRow").css('background-color', 'white');
  })
  
    $("#serviceRow").on('change', function() {
    if ($("#serviceRow").val() != 0)
      $("#trRow2").css('background-color', 'yellow');
    else
      $("#trRow2").css('background-color', 'white');

  })
})
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<table>
  <tr id="trRow">
    <td>Closed:</td>
    <td><select name="closed" id="closedRow">
            <option value="0">Show All</option>
            <option value="1">Hide Closed</option>
        </select></td>
  </tr>
  <tr id="trRow2">
    <td>Service:</td>
    <td><select name="service" id="serviceRow">
            <option value="0">Show All</option>
            <option value="1">Hide Service</option>
            <option value="2">Another Service</option>
        </select></td>
  </tr>
  <table>

\$\endgroup\$
3
\$\begingroup\$

Maybe use a generic function to set the css on change if you don't have anything other than colors. It also helps with dry.

function evt_highlightRow() {
    row = $(this).parent().parent()
    if ($(this).val() != 0)
      row.css('background-color', 'yellow');
    else
      row.css('background-color', 'white');
}

Then use it like so:

$(some_target_nested_2_deep).on('change', evt_highlightRow);

// such as your select
$('select').on('change', evt_highlightRow);
// or a generic class applied to inputs in the same general location
$('.highlightRowTarget').on('change', evt_highlightRow);

I created the example here: https://codepen.io/anon/pen/bWraNB

\$\endgroup\$
-2
\$\begingroup\$

$(document).ready(function() {


  $("#closedRow,#serviceRow").on('change', function() { // you can use multiple selector to factorize your code
  // here $(this) = $(idOfChangedElement) 
  // $(idOfChangedElement).closest("tr") will select the parent tr of the changed element
 // then you can run with your code
   /* if ($(this).val() != 0)
      $(this).closest("tr").css('background-color', 'yellow');
    else
      $(this).closest("tr").css('background-color', 'white');
  })
*/
// or use css add toggle or add and remove class

  ($(this).val() != 0)? $(this).closest("tr").addClass("notShowed"):$(this).closest("tr").removeClass("notShowed");
})
});
.showed{
  background-color:white;
}
.notShowed{
  background-color:yellow;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<table>
  <tr id="trRow">
    <td>Closed:</td>
    <td><select name="closed" id="closedRow">
            <option value="0">Show All</option>
            <option value="1">Hide Closed</option>
        </select></td>
  </tr>
  <tr id="trRow2">
    <td>Service:</td>
    <td><select name="service" id="serviceRow">
            <option value="0">Show All</option>
            <option value="1">Hide Service</option>
            <option value="2">Another Service</option>
        </select></td>
  </tr>
  <table>

\$\endgroup\$
  • 3
    \$\begingroup\$ Welcome to StackExchange Code Review! Please see: How do I write a good answer?, where you will find: "Every answer must make at least one insightful observation about the code in the question. Answers that merely provide an alternate solution with no explanation or justification do not constitute valid Code Review answers and may be deleted". \$\endgroup\$ – Stephen Rauch May 3 '17 at 23:00
  • \$\begingroup\$ did you read comments in my code?! this is my first answer on this site! \$\endgroup\$ – scraaappy May 4 '17 at 7:25
  • \$\begingroup\$ Hi, I know it is your first post, because I saw it in the first post review queue. As such, it is my job to let you know how things work here, so I left the note. I did not down vote your post, but others did. It is customary here to use prose to explain code, and why the posted code is superior to the code in the question. Please keep in mind that the primary point of this site to to help the questioner understand their code, not just to demonstrate a better solution. \$\endgroup\$ – Stephen Rauch May 4 '17 at 7:32
  • \$\begingroup\$ hey I like the ternary expression but went with the other answer for the time being. But I left you an upvote as your answer has value. I'm thinking people didn't like the bulk of your answer being my old commented out code + comments, which looked messy, + no clear explanation outside of the code block \$\endgroup\$ – Dennis May 4 '17 at 14:29

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.