5
\$\begingroup\$

I'm wondering if there is a better (cleaner?) method than my current implementation. I'm currently encoding a PHP SimpleXMLObject (USPS Tracking API) to JSON and looping through said JSON object via JavaScript to operate the front-end.

Examples from my current implementation below:

Function to display dialog implemented anonymously outside of .ready():

var moreInfo_popup = function(i) {
$('#moreinfo'+i).dialog({
        modal:false,
        autoOpen:false,
        height:555,
        title: 'Detailed View',
        width:500,
        draggable:false,
        buttons: {
            Ok: function(){
                $(this).dialog("close");
        }
    }
    });

        $('#moreinfo'+i).dialog('open');

        }

Main loop for displaying Tracking ID, Most Recent Event, and Mail Class for each API response- I'm currently generating a content div appended to #modal_container, then calling moreInfo_popup() inline via <input onClick="">:

for(var key in obj) {
                    if(obj.hasOwnProperty(key)) {
                    if(key % 2 === 0) {
                    $('#page-nav').append("<div id=\"results_table\"><table class=\"data_table\"id=\"data_table_id\"border=\"0\"width=\"60%\"align=\"center\"><tr><td align=center width=20%>"+obj[key].TrackInfo.Attributes.ID+"</td><td align=\"center\"width=\"35%\">"+obj[key].TrackInfo.StatusSummary+"</td><td align=\"center\"width=\"20%\">"+obj[key].TrackInfo.Class+"</td><td align=\"center\"><input type=\"button\"class=\"moreInfo\"value=\"Detail\"id=\"_buttonMoreInfo\"onClick=\"moreInfo_popup("+key+")\"></td></tr></table></div>");

                    $('#modal_container').append("<div id=\"moreinfo" + key + "\"><table><tr><td>" + obj[key].TrackInfo.Attributes.ID +"</td></tr></table>").hide();
                  }
                else {
                  $('#page-nav').append("<div id=\"results_table\"><table class=\"data_table_even\" id=\"data_table_id\" border=0 width=60% align=center><tr><td align=center width=20%>" 
                                      + obj[key].TrackInfo.Attributes.ID + "</td><td align=center width=35%>" + obj[key].TrackInfo.StatusSummary + "</td><td align=center width=20%>" 
                                      + obj[key].TrackInfo.Class + "</td><td align=\"center\"><input type=\"button\" value=\"Detail\" class=\"moreInfo\" id=\"_buttonMoreInfo\"onClick=\"moreInfo_popup("+key+")\"></td></tr></table></div>");

                  $('#modal_container').append("<div id=\"moreinfo" + key + "\"><table><tr><td>" + obj[key].TrackInfo.Attributes.ID +"</td></tr><tr><td> <button>OK</button></td></tr></table>");


                }

            }

      $("#page-nav td:contains('undefined')").html("Invalid");    
  }

As I'm sure you can see, this feels like an incredibly tedious way of accomplishing the desired outcome, to which there is surely a better alternative. As a newcomer to JavaScript/jQuery, I've done plenty of searching on this subject, but haven't really understood much of what I found, if indeed I was asking the right questions in the first place.

\$\endgroup\$
  • \$\begingroup\$ Hi, could you give an example of the parsed JSON response? \$\endgroup\$ – Dan Pantry Sep 5 '15 at 10:17
  • \$\begingroup\$ or, better yet, could you provide a working version of this on codepen.io? I'm having some difficulty understanding the role in how the tables work here, because it would appear you have nested tables - which are invalid HTML \$\endgroup\$ – Dan Pantry Sep 5 '15 at 11:23
5
\$\begingroup\$

The key problem I have with your code right now is that it doesn't separate data from presentation. This is a maintainability issue because it means now your data is inexplicably tied to your view, and you cannot change one without altering the other. How can we fix this?

As someone earlier suggested, you could use a template library. When I'm doing stand-alone development (i.e, not using a framework), I really like rivets. It's lightweight (6.2kb), simple and unopinionated.

I would have recommended dropping jQuery, but I see that you're using a plugin for jQuery, so I can't really suggest that. I would say that if you can find a way to avoid using jQuery here, that would be great. The reason I don't want you using jQuery is because it's a heavyweight library; it's 34kb, which is over 5x the size of Rivets.

The time-to-last-byte is very important in JavaScript development because until your consumer has received the last byte, your JavaScript is doing nothing, which potentially makes part of your site unusuable. As such, if you can remove any cruft from your site to make it load all of the script tags faster, the better off you will be.

First, we'll define a HTML layout for your tracking info. As your tracking info is tangential to the current information, we can make this use the <aside> HTML element.

<aside>
  <table>
    <tr>
      <td>ID</td>
      <td>Summary</td>
      <td>Class</td>
      <td>
        <button>Detail</button>
      </td>
    </tr>
  </table>
</aside>

So above is the basic structure of your dialog. There's a lot of inline styling, and the only thing your "if" statement is for is to add alternating classes. No need to do that here! We can do that in CSS just fine. First of all, lets apply your styles with a bit of CSS. We'll add a .data-table class to your table, although I would definitely recommend naming it something more descriptive; .data-table is pretty ambigious. You want to describe the role this thing fulfills, not what it is. In other words, be more declarative with your css names. In addition, we'll add a .results-table class to the encapsulating <aside>. Whilst I'm not sure this is completely correct, it appears to be closest to what you are doing.

First, let's tackle the overarching styles of each of the tables:

.data-table {
  border: 0;
  width: 60%;
  align: center;
}

You'll notice I have not included the <td> styling. This is because the width of a <td> is decided by the browser and not the CSS. This makes sense if you are using truly tabular data. If you're not using tabular data (and I suspect you're not), then you should use a <dl>. <dl> are definition lists which may be more suited for this sort of thing.

You appear to have some kind of alternate styling based on whether or not a .data-table is an even .data-table. I'm not sure what this styling is because it isn't supplied, but you can replicate this class by using the following CSS selector:

.results-table:nth-child(even) {
  // some CSS styling here
}

So far, we have this:

<aside class='results-table'>
  <table class='data-table'>
    <tr>
      <td></td>
      <td></td>
      <td></td>
      <td>
        <button>Detail</button>
      </td>
    </tr>
  </table>
</aside>

Now, your data model looks something like this:

var model = {
  someNumericValue: {
    TrackInfo: {
      Attributes: {
        ID: 0
      },
      StatusSummary: "summary",
      Class: "first"
    }
  }
};

Wow, this looks a lot like an array! It might be, or it might not be. It's hard to tell because arrays are actually objects. This means that they have access to the hasOwnProperty of an object. Lets assume it is an array because that makes more sense.

var models = [
  { 
    TrackInfo: {
      Attributes: {
        ID: 0
      },
      StatusSummary: "summary",
      Class: "first"
    }
  }
];

With this data model, we can easily slot this into our template using rivets bindings. Lets take a look at how it pans out (I've skipped the popup at the moment, we'll get into that in a bit).

<aside class='results-table'>
  <table class='data-table' rv-each-order='model.orders'>
    <tr>
      <td rv-text='order.TrackInfo.Attributes.ID'></td>
      <td rv-text='order.TrackInfo.StatusSummary'></td>
      <td rv-text='order.TrackInfo.Class'></td>
      <td>
        <button>Detail</button>
      </td>
    </tr>
  </table>
</aside>

Now when we set up rivets on this element, as long as we pass in all of our models to "models" - voila, you'll have data binding. Of course, this isn't 100% perfect because we're still relying on our JavaScript to populate the data, however it's a good start; the HTML structure is there and it's declarative. Anyone reading over this file with some knowledge of how rivets works would be able to tell what this does, and the HTML isn't hidden away in JavaScript - the only thing we're getting from JavaScript is the repeating behaviour and the values. This is a good shift!

How would you use this, you might ask? Well, to bind the <aside> to data, it really is as simple as this:

var dialog = document.querySelector('aside.results-table');
rivets.bind(dialog, { order: models });

Where, obviously, models is your array of data. This will ensure that all of your data is bound correctly. Note that this is only one-way data binding though, so any changes you make (DOM -> Model, such as a form) will not be reflected in the change to the view - only model changes will update the view.

Alright, so we have the basic setup done. Now, how do we tackle the fact that we have a pop up that is displayed as well? Well, this gets a bit tricky because you're using a jQuery plugin. It's going to be difficult to neatly display a popup based on DOM elements without doing some nasty fudging. The easiest way would be to continue using jQuery like you're doing now. However, this presents some nasty stuff, namely that you're using numbered IDs to access the dialogs. Ouch! Instead, I would suggest creating a Dialog in HTML and not using JQuery. Like this:

<dialog>
  <title>Detailed View</title>
  <table>
    <tr>
      <td rv-text='order.TrackInfo.Attributes.ID'></td>
    </tr>
    <tr>
      <td>
        <button>OK</button>
      </td>
    </tr>
  </table>
</dialog>

Note that the <dialog> element is a HTML5 element, so you may want to change this to a <div> if you need to support browsers that aren't HTML5. However, given that I've used <aside> and <section> you would need to do that for everything else as well. Worst case scenario, use a HTML5 polyfill. Let's put this all together and add the functionality for the pop-up to be closed/opened after the button is clicked..

<aside class='results-table'>
  <section rv-each-order='orders'>
    <table class='data-table'>
      <tr>
        <td rv-text='order.TrackInfo.Attributes.ID'></td>
        <td rv-text='order.TrackInfo.StatusSummary'></td>
        <td rv-text='order.TrackInfo.Class'></td>
        <td>
            <button rv-on-click='showMoreInfo'>Detail</button>
        </td>
      </tr>
    </table>
    <dialog rv-if='order.displayMoreInfo'>
      <title>Detailed View</title>
      <table>
        <tr>
          <td rv-text='order.TrackInfo.Attributes.ID'></td>
        </tr>
        <tr>
          <td>
            <button rv-on-click='hideMoreInfo'>OK</button>
          </td>
        </tr>
      </table>
    </dialog>
  </section>
</aside>

Note how I've moved the rv-each-order to a <section> that encapsulates both the <table> and the <dialog>. This makes it easier to group the elements together and better represents that they are a single, isolated section of content compared to the other orders you've put in here. On clicking of the "Detail" button, the dialog will be shown. When the dialog "OK" button is clicked, the dialog will be hidden. Simple stuff, right? (Note: Shown and hidden is not a good analogy. What actually happens is that the entire <dialog> will be removed/added to the DOM. If you wish to instead actually show/hide the <dialog> - for performance reasons, maybe - then use rv-show/rv-hide).

Now, lets add the final bootstrapping logic..

function hideMoreInfo(event, context) {
  var currentOrder = context.order;
  currentOrder.displayMoreInfo = false;
}

function showMoreInfo(event, context) {
  var currentOrder = context.order;
  currentOrder.displayMoreInfo = true;
}

var resultsTable = document.querySelector('.results-table');
rivets.bind(resultsTable, {
  hideMoreInfo: hideMoreInfo,
  showMoreInfo: showMoreInfo,
  orders: models
});

That's it - we're done with the logic side of things! The only thing left would be to do CSS. Unfortunately, I can't do more CSS than what I've done for you because you haven't actually linked any, but if you would like some more help on tht please let me know and I'll gladly oblige.

To summarize, here's the finalised code:

HTML

<aside class='results-table'>
  <section rv-each-order='orders'>
    <table class='data-table'>
      <tr>
        <td rv-text='order.TrackInfo.Attributes.ID'></td>
        <td rv-text='order.TrackInfo.StatusSummary'></td>
        <td rv-text='order.TrackInfo.Class'></td>
        <td>
            <button rv-on-click='showMoreInfo'>Detail</button>
        </td>
      </tr>
    </table>
    <dialog rv-if='order.displayMoreInfo'>
      <title>Detailed View</title>
      <table>
        <tr>
          <td rv-text='order.TrackInfo.Attributes.ID'></td>
        </tr>
        <tr>
          <td>
            <button rv-on-click='hideMoreInfo'>OK</button>
          </td>
        </tr>
      </table>
    </dialog>
  </section>
</aside>

JavaScript

function hideMoreInfo(event, context) {
  var currentOrder = context.order;
  currentOrder.displayMoreInfo = false;
}

function showMoreInfo(event, context) {
  var currentOrder = context.order;
  currentOrder.displayMoreInfo = true;
}

var resultsTable = document.querySelector('.results-table');
rivets.bind(resultsTable, {
  hideMoreInfo: hideMoreInfo,
  showMoreInfo: showMoreInfo,
  orders: models
});

CSS

.data-table {
  border: 0;
  width: 60%;
  align: center;
}
\$\endgroup\$
  • 1
    \$\begingroup\$ As an addendum, remember that you don't always need jQuery. For simple things, there's no need for it. Of course, it's always worth knowing what jQuery is doing for you, but you don't want to have a 34kB download for something that can be done using less kB. \$\endgroup\$ – Dan Pantry Sep 5 '15 at 13:19
  • \$\begingroup\$ I really didn't expect such a detailed response. Thank you! I had actually started rewriting everything in Angular last night, but I do definitely intend to give Rivets a try. In the context of your answer, would you suggest just performing an traditional AJAX request? What about pagination? Is there a way to, sans use of jQuery, perform results pagination without sending the user to another page? I'm currently using a jQuery plugin for that functionality. \$\endgroup\$ – Dustin Gulley Sep 5 '15 at 23:34
  • \$\begingroup\$ Angular only makes sense if you're making a web app -not a website. be wary. Yes, I would use traditional AJAX requests. jQuery will use ajax under the hood; the plugin you're using just gives you a 'nicer' interface to do pagination. In order rto perform pagination without refreshing the page you'd just send a request to the server with a query string (say, ?page=5&pageSize=10) using AJAX and then react to that when it comes backl. \$\endgroup\$ – Dan Pantry Sep 5 '15 at 23:43
1
\$\begingroup\$

For the page rendering part, I'd recommend using some templating library like Handlebars.js, for example.

\$\endgroup\$
1
\$\begingroup\$

You should use a back-end templating to avoid HTML in your JS file. That way you will be able to manage your HTML always server side and not front side (never a good idea unless you're doing an app). It will also speedup your frontend display as you won't have to treat those data.

So at the end, your Ajax will only be the HTML already treated.

\$\endgroup\$
  • 1
    \$\begingroup\$ While your advice sounds good, given that the OP is a beginner, I'm not sure this will help him much. It would be better if you could name some templating software to give OP something to search and move forward. A concrete example (rewriting the posted code to use the templating software) would be most helpful. After you improved your post, feel free to ping me, I'll be happy to circle back and +1 \$\endgroup\$ – janos Sep 5 '15 at 8:37

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.