4
\$\begingroup\$

I'm using multiple jQuery filter functions to affect the background color of select objects based on the selected option and it works, but it seems bulky/redundant. Does anyone know how these filter statements could be simplified/combined? I have a JSFiddle here.

<head>

<!-- Load minimum jQuery -->
  <script src="http://ajax.googleapis.com/ajax/libs/jquery/1.10.2/jquery.min.js"></script>
<!-- Load functions -->
  <script type="text/javascript">
<!-- jQuery Functions -->
  $(document).ready(function(){
/* Add 'Contracted' title and change background to green (if not already red) for 'C' services */
    $("select").parent()
    .filter(function(index){
        return $(this).find('option:selected').text() == 'C'; })
    .attr('title','Contracted')
    .addClass(function(index,currentClass) {
        var addedClass;
        if ( currentClass != "cellcolorred" )
            { addedClass = "cellcolorgreen"; }
        return addedClass;
    });
/* Add 'Available' title and change background to green (if not already red) for 'A' services */
    $("select").parent()
    .filter(function(index){
        return $(this).find('option:selected').text() == 'A'; })
    .attr('title','Available')
    .addClass(function(index,currentClass) {
        var addedClass;
        if ( currentClass != "cellcolorred" )
            { addedClass = "cellcolorgreen"; }
        return addedClass;
    });
/* Add 'Expensive' title and change background to orange (if not already red) for 'E' services */
    $("select").parent()
    .filter(function(index){
        return $(this).find('option:selected').text() == 'E'; })
    .attr('title','Expensive')
    .addClass(function(index,currentClass) {
        var addedClass;
        if ( currentClass != "cellcolorred" )
            { addedClass = "cellcolororange"; }
        return addedClass;
    });
/* Add 'Do Not Use' title and change background to pink for 'D' services */
    $("select").parent()
    .filter(function(index){
        return $(this).find('option:selected').text() == 'D'; })
    .attr('title','Do Not Use')
    .addClass("cellcolorpink");
/* Add 'No Service' title to empty services */
    $("select").parent()
    .filter(function(index){
        return $(this).find('option:selected').text() == ''; })
    .attr('title','No Service');
});
</script>

<style type="text/css">

/* body */
    body {
        border: 0;
        margin: 0;
        padding: 0;
        background-color: #A3AFC0;
        font-family: Verdana, Geneva, Arial, sans-serif;
    }

    select::-ms-expand { display: none; }
    select {
        appearance: none;
        -moz-appearance: window;
        -webkit-appearance: none;
        border-style: none;
        background: none;
        color: inherit;
        width: inherit;
        height: 100%;
        vertical-align: middle;
        font: inherit;
        font-size: 13px;
        text-indent: 11.5px;
    } /* Select Cells */
    tbody {
        border-bottom-width: 2px;
        border-bottom-style: solid;
        background-color: #FAFAFA;
    }
    tbody:last-of-type { border-bottom-width: 1px; }
    tbody:nth-child(even) { background-color: #E1E1E1; }
    tbody:hover td[rowspan], tr:hover td { background-color: #FF0; }
    td {
        border-style: solid;
        border-color: inherit;
    }
/* tfoot */
    tfoot {
        border-width: 1px;
        font-weight: bold;
        background-color: #E1E1E1;
    }
/* Classes */
/* table */
    .tablesorter {
        table-layout: fixed;
        font-size: 11px;
        border-collapse: collapse;
        border-spacing: 0;
        border-color: #6E7984;
    }

/* header */
    .tablesorter th,
    .tablesorter thead td {
        height: 24px;
        text-shadow: 0 0 4px #FFF;
        text-align: center;
        color: #000;
        font-size: 11px;
        background-color: #22A5C9 !important;
        padding: 0;
    }
/* tbody */
    .tablesorter td {
        text-align: center;
        width: 33.5px;
    }

    .cellcolorred {
        font-weight:bold;
        color: #FFF;
        background-color: #F00 !important;
    }
    .cellcolorgreen { background-color: #CFC !important; }
    .cellcolororange { background-color: #FC9 !important; }
    .cellcolorpink { background-color: #FCC !important; }
</style>

  </head>
<!-- Body -->
  <body>
<!-- Table -->
  <table border="1" class="tablesorter" id="table-1">
<!-- Table Header -->
    <thead>
        <tr>
            <td>ID</td>
            <td>Alarm</td>
            <td>Bflow</td>
            <td>Burg</td>
            <td>ELt</td>
            <td>Eng</td>
        </tr>
    </thead>
<!-- Table Footer -->
    <tfoot>
        <tr>
            <td colspan='100%'>1 Location</td>
        </tr>
    </tfoot>
<!-- Table Body -->
    <tbody>
        <tr id="1">
            <td>AL08960</td>
            <td id="Alarm">
                <select>
                <option selected>C</option>
                <option>A</option>
                <option>E</option>
                <option disabled>-</option>
                <option>D</option>
                <option></option>
                </select>
            </td>
            <td id="Bflow">
                <select>
                <option>C</option>
                <option selected>A</option>
                <option>E</option>
                <option disabled>-</option>
                <option>D</option>
                <option></option>
                </select>
            </td>
            <td id="Burg">
                <select>
                <option>C</option>
                <option>A</option>
                <option>E</option>
                <option disabled>-</option>
                <option>D</option>
                <option selected></option>
                </select>
            </td>
            <td id="ELt">
                <select>
                <option>C</option>
                <option>A</option>
                <option selected>E</option>
                <option disabled>-</option>
                <option>D</option>
                <option></option>
                </select>
            </td>
            <td id="Eng">
                <select>
                <option>C</option>
                <option>A</option>
                <option>E</option>
                <option disabled>-</option>
                <option selected>D</option>
                <option></option>
                </select>
            </td>
        </tr>
    </tbody>
  </table>
 <table id="header-fixed"></table>
</body>
Improve this question
\$\endgroup\$

2 Answers 2

Reset to default
5
\$\begingroup\$

You seem to overuse !important in your CSS. This is very bad, but I didn't change anything about this.

Your script is extremely repetitive, too. I tried my hand at DRYing it and I got:

$(document).ready(function () {
    var serviceStatuses = {
        'A': {
            title: "Available",
            color: "green"
        },
        'C': {
            title: "Contracted",
            color: "green"
        },
        'D': {
            title: "Do Not Use",
            color: "pink",
            override: true
        },
        'E': {
            title: "Expensive",
            color: "orange"
        }
    };
    $("select").each(function() {
        var $t = $(this);
        var $td = $t.parent();
        var status = serviceStatuses[$t.val()] || {
            title: "No Service",
            color: "Default"
        };
        $td.attr("title", status.title).toggleClass("cellcolor" + status.color, status.override || !$td.hasClass("cellcolorred"));
    });
});

Here's a fiddle demonstrating it. I took the liberty of adding another row with cellcolorred on every cell to demonstrate that D will still override the selection.

Improve this answer
\$\endgroup\$
1
  • \$\begingroup\$ That worked perfect! I took out the override code, because, as I explained in my comment to Alex L, cellcolorred would only be set for A, C, or E cells if the row is considered inactive, so a 'D' cell would never have a cellcolorred class on it. Thanks for your help. \$\endgroup\$
    – Kn1ghtman
    Jul 9, 2014 at 15:16
4
\$\begingroup\$

Because there's no context to your code, and it's difficult to understand what the purpose is, I couldn't do much besides refactor the code. I was hoping I would be able to suggest alternate methods to get the job done, but I didn't feel I had enough info.

JavaScript

I was able to DRY it out and come up with this:

$(document).ready(function () {
    function selector(text, newTitle, newClass) {
        $("select").parent().filter(function () {
            return $(this).find('option:selected').text() == text;
        }).attr('title', newTitle).addClass(newClass);
    }
    selector('C', 'Contracted', function (index, currentClass) {
        return (currentClass != 'cellcolorred') ? 'cellcolorgreen' : '';
    });
    selector('A', 'Available', function (index, currentClass) {
        return (currentClass != 'cellcolorred') ? 'cellcolorgreen' : '';
    });
    selector('E', 'Expensive', function (index, currentClass) {
        return (currentClass != 'cellcolorred') ? 'cellcolororange' : '';
    });
    selector('D', 'Do Not Use', 'cellcolorpink');
    selector('', 'No Service', null);
});

This is exactly a third of what you originally presented, however I feel even this could be brought down a notch.

You'll notice I stuck the repeating code in a function, and was able to pass the class as a closure. This allows for easier expansion and reduction.

I'm confused about the conditional though, because to start off, none of the cells have a class. So I'm not sure why you check if they're cellcolorred because they all fail the condition, therefore coloring it.

HTML

Not too much to comment on. It's pretty simple and nothing is too complex.

  • I don't see a need to ID the table. You never refer to the ID, and if you do need to reference, the class should work fine.
  • table#header-fixed has no use in the code at all. Maybe you just left out that context.

CSS

  • Not sure why you set border: 0 on the body. The body doesn't have a border in the first place.

  • In the selects, it's difficult to there they're actually buttons with the appearance set to none. Perhaps add a cursor or pronounce their button-ness more.

  • Clean up tbody:

    border-bottom-width: 2px;
border-bottom-style: solid;

    to

      border-bottom: 2px auto solid;

  • I think it's easier to read something such as:

     tr .red {

    as opposed to

    .cellcolorred {

    I suggest you clean up these types of things. Hopefully you'll be able to find a better name than red of course!

Improve this answer
\$\endgroup\$
1
  • 1
    \$\begingroup\$ This code is only a relevant chunk of a full table that's viewed through a web viewer in FileMaker. Without going too far into it, it shows data from 3 different data tables, each one step down from the previous, in each row. The reason for cellcolorred is to highlight when a record is inactive, but a service is active. A, C, & E are usable statuses, but 'D' means Do Not Use. If the row is inactive and the status is usable, I set cellcolorred. With the selects not looking like buttons, that was intentional. This replaced a native portal, so I tried to emulate the appearance as best I could. \$\endgroup\$
    – Kn1ghtman
    Jul 9, 2014 at 15:10

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.