Simple inbox functionality using JavaScript and Ajax

I've been developing a simple CMS both for practical use and learning about Laravel, and I decided to implement a simple service that makes it possible for the administrators to send messages to one another. However, the finished JavaScript code, while completely functional, ended up being a bit sketchy so I'm looking for ways to improve its performance and security. Any feedback is greatly appreciated.

This is the JavaScript code that sends Ajax calls to server (based on search term, whether we want the sent messages or the incoming messages) and formats and reloads the table shown in my CMS:

```
var page = 1;
var perPage = 10;
var lastPage = 1;
var searchTerm = '';
var category = 'inbox';

function nextPage () {
    // Incrementing the global 'page' variable
    page++;
    reloadList();
}

function prevPage () {
   // Decrementing the global 'page' variable
    page--;
    reloadList();
}

function search () {
    // Resetting the global 'page' variable
    page = 1;
    /* reloadList() automatically detects the search terms and sends the appropriate AJAX request
based on the value of the 'searchTerm' variable */
    searchTerm = $("#searchBox").val();
    reloadList();
}

function changeCat (cat) {

    // Resetting the global 'page' variable
    page = 1;
    // Resetting the global 'searchTerm' variable
    $("#searchBox").val('');
    searchTerm = '';
    // Setting the global 'category' variable
    category = cat;

    // Hackish way of setting the correct header for the page
    $("#tab-header").html($("#"+cat+"-pill-header").html());

    // Deactivating the old pill menu item
    $("#category-pills>li.active").removeClass("active");
    // Activating the selected pill menu item
    $("#"+cat+"-pill").addClass('active');
    reloadList();
}

function toggleStar (id) {
    // Calling the server to toggle the 'starred' field for 'id'
    var data = 'id=' + id + '&_token=' + pageToken;

    $.ajax({
        type: "POST",
        url: '/admin/messages/toggl

Style guide

I'd recommend picking a style guide to keep all of your whitespace consistent. I've formatted yours according to semistandard. Having a style guide could be considered a form of bike-shedding, but it lets you have your IDE focus on making your code readable while you can just focus on making it work.

Try to limit the use of jQuery

You're using jQuery quite a lot in your code, namely to access elements. If you can afford to (and, really, you should be able to), use standard JavaScript to access DOM elements. One of the main benefits of using the standard methods is that the browser will cache repeated accesses to document.getElementById() cheaply: jQuery won't do this (at least, not the last time I checked).

Comments

You have some comments that are describing what you're doing, like: // Incrementing the global 'page' variable. These bring absolutely no value to your code; it's pretty obvious when you see the -- operator that you're doing that. Stick to having your comments describe why they are doing what they are doing, rather than what they are doing.

Conversely, as your script is quite long you should try and add some descriptions to each function (if it is not already obvious what they do) so that other people (including yourself) understand what your code is doing in 6 months when you have to come to maintain it. We have a standard format for that called jsdoc. There are other versions of this standard, but they all use the same sort of format.

Double equals

Use triple equals instead. Here is why. You almost never want to use the double equals and many linters will yell at you for using it.

Limit use of variables to their appropriate scope

One of your variables (var i = 0) is initialised outside of a loop, but only used inside that loop. It's best practice to try and limit the amount of scope a variable touches (especially a mutable one) as it makes it easier to reason about where that variable is used.

javascript:void(0)/onclick

No! This breaks semantic HTML. If you have `, what you actually want is a instead. A link and a button have two very different semantics.

Do not use inline JavaScript like onclick: use event handlers anyway. Any good content security policy (which helps protect against XSS) will disable inline javascript and thus those attributes will not work.

Generating HTML elements

I'd strongly recommend generating HTML elements based on template tags in your HTML body and using Document.cloneNode` instead of pulling together JavaScript strings in your code to help with separation of concerns. This would also work very nicely with something like lodash's template elements.

I did originally start refactoring your code to take into account all of these things, but unfortunately it just started taking far too much time. There's a lot of room for improvement here, but I hope what I've given you helps.

I'd strongly suggest trying to reduce the amount of usage of jQuery and refactoring your templates out of your JavaScript, though. This code is very long and is quite hard to follow and it will easily become that sort of code that in 6 months you wish you could just scrap it and start over. This is primarily because your view logic is very intertwined with your business logic.

StackExchange Code Review Q#134364, answer score: 5