Refactoring Some Amazing jQuery Code

11 min readOct 11, 2021

These are some beautiful pictures I took of the Arizona sunrise, starting with a pinkish orange glow to a full on bright finish. Similarly in this post I’m going to be showing you some jQuery from a software ticket I worked on recently. It starts off being repetitive/long, but then it gets turned it into a few simple lines.

jQuery was something brand new to me that I had to learn within my software engineering job here at Home Chef. It’s basically just a JavaScript library that adds in some syntax unique to it here and there all to grab certain things from the DOM, much more firmly than your average claw machine does. I don’t know about you but claw machines always disappointed me as a little girl when they’d randomly drop what I was after.

The problem

Anyways here was the situation within the ticket I was working on to give some context. I should say my role is to code warehouse apps specifically so naturally we’ll be talking about things that are in that realm. Also we’ll be coding with Ruby on Rails and Haml template system using the Active Admin Ruby gem.

We had a form that was supposed to create a warehouse storage slot with the following required attributes

packing_facility_id
storage_area_name
storage_slot_name

And the optional ones

length
width
height

The problem was the storage_area_name field was disabled, so we couldn’t make more storage slot records in general.

My job was to figure out why and get it so that:

1 Based on which packing facility id that gets selected (1, 2, 3), the right storage area names show up in that storage area name drop down because a storage area belongs to a packing facility based on these lines from our StorageArea model.

class Ops::Warehouse::StorageArea < ApplicationRecord   belongs_to :packing_facility, inverse_of: :storage_areas

2 It would make a StorageSlot Ruby object with each of the attributes when the submit button is hit.

=> #<Ops::Warehouse::StorageSlot:0x00007f8b8e8ed688
 id: 23518,
 name: "Unknown Slot",
 active: true,
 length: 0,
 width: 0,
 height: 0,
 created_at: Wed, 06 Oct 2021 12:58:14 CDT -05:00,
 updated_at: Wed, 06 Oct 2021 12:58:14 CDT -05:00,
 storage_area_id: 68>

After some digging, I found out the cause of the problem was the way we had our selection drop down set up for that storage area name attribute. Not only was it disabled but it also had no Rails model giving it the storage area data it needed and there was no reference to letting that be influenced by the packing facility id the user selects.

Why would someone disable it in the first place? I have no idea and still don’t know to this day. That’s okay though, what matters more is fixing this.

This is the original code in the Haml file for what the form looks like behind the scenes before I even touched it. You can see that the storage area is required, but the object that represents it has the attribute of disabled.

%tbody
        %tr
          %td
            = f.select :packing_facility_id, options_for_select(PackingFacility.internal.map { |pf| [pf.name, pf.id] }), {include_blank: true},  {required: true, class: 'required packing-facility'}
          %td
            = f.select :storage_area_id, [], {}, { required: true, disabled: true, class: "storage-area" }

The solution

My approach was to make 4 different versions of a storage area name selection drop down and have each of those boxes use the appropriate Rails storage area model and map the storage area instances after they are filtered out by their packing facility ids.

So this means one drop down is made for our Chicago packing facility with all the storage area names only associated with that packing facility. Then one for our Atlanta packing facility and so on. Also each of these boxes was going to have an id attached to it like

id:"chicagoDropDown" or id:"atlantaDropDown"

What one of the drop down boxes look like

The first drop down would be blank though because the form initially has no values in the form when the user loads the page and I wanted that to be the default of what showed up until they selected a packing facility.

Each one of these boxes would also be hidden initially with the “is--hidden” class added.

Then I would eventually write the jQuery that was going to grab the value out of the packing facility id selection box, read and save it into a variable called selected.

Next the jQuery would call a function that was going to check if the packing facility id the user selects matches with the appropriate number that represents each packing facility. And if it did find a match, to render the appropriate storage area drop down that was associated with that number by removing the “is — hidden” class.

What the original solution looked like

I’ve broken this down for you if you’re unfamiliar with how a Haml or jQuery file looks like. Here our form starts and what you’re seeing is our packing facility id input drop down:

%tbody
        %tr
          %td
            = f.select :packing_facility_id, options_for_select(PackingFacility.internal.map { |pf| [pf.name, pf.id] }), {include_blank: true},  {required: true, class: 'required packing-facility', id:"packing-facility"}

Then here is our 4 storage area name selection drop downs (each time you see f.select :storage_area_id) holding all the filtered out instances from our models (Ops::Warehouse::StorageArea) so that each one is only showing one packing facility’s storage area name collection. It starts with our blank disabled box.

%td
            = f.select :storage_area_id, [], {}, { required: true, disabled: true, class: "storage-area", id:"blank"}
            = f.select :storage_area_id, options_for_select(Ops::Warehouse::StorageArea.all.filter_map { |storage_area| if storage_area.packing_facility_id == 1 then [storage_area.name, storage_area.id] end }), {include_blank: true}, { required: true, class: "storage-area is--hidden", id:"chicagoDropDown"}
            = f.select :storage_area_id, options_for_select(Ops::Warehouse::StorageArea.all.filter_map { |storage_area|  if storage_area.packing_facility_id == 2 then [storage_area.name, storage_area.id] end }), {include_blank: true}, { required: true, class: "storage-area is--hidden", id:"sanBernDropDown"}
            = f.select :storage_area_id, options_for_select(Ops::Warehouse::StorageArea.all.filter_map { |storage_area|  if storage_area.packing_facility_id == 3 then [storage_area.name, storage_area.id] end }), {include_blank: true}, { required: true, class: "storage-area is--hidden", id:"atlantaDropDown"}

And finally we have our long jQuery doing the things I described earlier after it finds the right drop down by the id.

$('#packing-facility').on('change', function() {
      $('.storage-area').addClass("is--hidden")
      $('.storage-area').prop('disabled',false)
      let selected = $(this).val()if(selected === "1"){
        $('#chicagoDropDown').removeClass("is--hidden")$('#sanBernDropDown').prop('required',false)
        $('#atlantaDropDown').prop('required',false)
        $('#blank').prop('required',false)
        $('#sanBernDropDown').prop('disabled',true)
        $('#atlantaDropDown').prop('disabled',true)
        $('#blank').prop('disabled',true)
      } else if (selected === "2"){
       $('#sanBernDropDown').removeClass("is--hidden")$('#chicagoDropDown').prop('required',false)
       $('#atlantaDropDown').prop('required',false)
       $('#blank').prop('required',false)
       $('#chicagoDropDown').prop('disabled',true)
       $('#atlantaDropDown').prop('disabled',true)
       $('#blank').prop('disabled',true)
      } else if (selected === "3"){
       $('#atlantaDropDown').removeClass("is--hidden")$('#chicagoDropDown').prop('required',false)
       $('#sanBernDropDown').prop('required',false)
       $('#blank').prop('required',false)
       $('#chicagoDropDown').prop('disabled',true)
       $('#sanBernDropDown').prop('disabled',true)
       $('#blank').prop('disabled',true)
      }
    });

I wanted to make it clear that when this line runs…

let selected = $(this).val()

It is selecting from the option tag’s value you see above after the user picks an option.

You’ll notice that I’ve manually assigned the required and disabled attributes using the built in methods from jQuery (like .prop). The jQuery needed to set the drop down boxes that weren’t selected from the packing facility id to be disabled and not be required.

I found that if the selected box was not the only box that was marked as required, then Rails would try to read all three of them behind the scenes for the all the storage area name drop down values when I hit the submit button. Then it would throw me a focus error that tells me that “storage area name fields aren’t focusable” which would make sense. How can it focus when the computer doesn’t know which one to choose?

While this code works, you can see that it is unnecessarily long and is doing the same thing each time just with different boxes. My team gave me some great inspiration for certain parts of this and I thank them wholeheartedly.

Second Draft

This time I logically took a step back and made all the boxes disabled in the beginning (disabled: true value in each f.select below) and that way I would only need to ever change the selected drop down with new true/false values for the required and disabled attributes.

%td
            = f.select :storage_area_id, [], {}, { disabled: true, class: "storage-area", id:"blank"}
            = f.select :storage_area_id, options_for_select(Ops::Warehouse::StorageArea.all.filter_map { |storage_area| if storage_area.packing_facility_id == 1 then [storage_area.name, storage_area.id] end }), {include_blank: true}, { disabled: true, class: "storage-area is--hidden", id:"chicagoDropDown"}
            = f.select :storage_area_id, options_for_select(Ops::Warehouse::StorageArea.all.filter_map { |storage_area|  if storage_area.packing_facility_id == 2 then [storage_area.name, storage_area.id] end }), {include_blank: true}, { disabled: true, class: "storage-area is--hidden", id:"sanBernDropDown"}
            = f.select :storage_area_id, options_for_select(Ops::Warehouse::StorageArea.all.filter_map { |storage_area|  if storage_area.packing_facility_id == 3 then [storage_area.name, storage_area.id] end }), {include_blank: true}, { disabled: true, class: "storage-area is--hidden", id:"atlantaDropDown"}....$('#packing-facility').on('change', function() {
      $('.storage-area').addClass("is--hidden")
      let selected = $(this).val()
  
      if(selected === "1"){
        $('#chicagoDropDown').removeClass("is--hidden")
        $('#chicagoDropDown').prop('required',true)
        $('#chicagoDropDown').prop('disabled',false)
      } else if (selected === "2"){
        $('#sanBernDropDown').removeClass("is--hidden")
        $('#sanBernDropDown').prop('required',true)
        $('#sanBernDropDown').prop('disabled',false)
      } else if (selected === "3"){
        $('#atlantaDropDown').removeClass("is--hidden")
        $('#atlantaDropDown').prop('required',true)
        $('#atlantaDropDown').prop('disabled',false)
      }
    });

So the code has gotten shorter, but still feels too repetitive.

Third Draft

This was whenever I started asking a more senior engineer what I could do to make my code even more efficient. She told me to go with mapping out the facilities (basically making a dictionary) and writing some loops to do the assigning for me after the dictionary gets checked. This sounded like a more classic algorithm problem type approach.

I gave it my best shot though and here’s what I ended up coding out that night. Note: I was able to get rid of the “disabled: true” value in each f.select below.

%td
            = f.select :storage_area_id, [], {}, { disabled: true, class: "storage-area", id:"blank"}
            = f.select :storage_area_id, options_for_select(Ops::Warehouse::StorageArea.all.filter_map { |storage_area| if storage_area.packing_facility_id == 1 then [storage_area.name, storage_area.id] end }), {include_blank: true}, { class: "storage-area is--hidden", id:"chicagoDropDown"}
            = f.select :storage_area_id, options_for_select(Ops::Warehouse::StorageArea.all.filter_map { |storage_area|  if storage_area.packing_facility_id == 2 then [storage_area.name, storage_area.id] end }), {include_blank: true}, { class: "storage-area is--hidden", id:"sanBernDropDown"}
            = f.select :storage_area_id, options_for_select(Ops::Warehouse::StorageArea.all.filter_map { |storage_area|  if storage_area.packing_facility_id == 3 then [storage_area.name, storage_area.id] end }), {include_blank: true}, { class: "storage-area is--hidden", id:"atlantaDropDown"}....$('#packing-facility').on('change', function() {
    $('.storage-area').addClass("is--hidden")
    let selected = $(this).val()
    let dropDownDictionary = { 0: "blank", 1: "chicago", 2:  "sanBernardino", 3: "atlanta" }
    let dropDownsArray = []
    dropDownsArray.push($('.storage-area'))    //sets the dictionary values as dropDown objects from the drop dropDownsArray    for(let i = 0; i<dropDownsArray.length; i++){
      Object.assign(dropDownDictionary, dropDownsArray[i])
    }    //go through the dictionary and add/delete the right attributes to show right select box    for (let dropDownId in dropDownDictionary) {
      let idOfDropDown = dropDownDictionary[dropDownId]["id"]
      if(selected == dropDownId){
        $('#' + idOfDropDown).removeClass("is--hidden")
        $('#' + idOfDropDown).prop('required',true)
        $('#' + idOfDropDown).prop('disabled',false)
      } else {
        $('#' + idOfDropDown).prop('required',false)
        $('#' + idOfDropDown).prop('disabled',true)
      }
    }
  });

To explain: what you’re seeing here is still what the user selects from the packing facility id field saved in “selected” as a variable.

Then a dropDownDictionary that maps out each of our id’s, including 0 representing the blank one, as our keys and they are all set to string values.

Those values will eventually become saved jQuery drop down objects representing each of our drop downs. The jQuery does the selecting by class because each of the drop downs belongs to a class called a storage-area. Then it puts all the drop downs into our array. To make it more clear, this means the array looks like this if you console.log it that looks like:

Next the first for loop you see is changing our dictionary strings to hold each of those jQuery objects from our array as the values assigned to each of our keys. After that loop runs, if you console.log dropDownDictionary you’d see:

Finally we get to the last for loop and this loop goes through the jQuery drop down objects in our dictionary and is doing the attribute removal or assignment. This happens after it checks whether it is dealing with the selected drop down or not based on what the user selected.

It finds the right drop down based on selecting the id of the jQuery object and saving that into a variable appropriately called “idOfDropDown”.

In other words, if the user-selected packing facility id matches the key which is a packing facility number then it will remove the “is — hidden class”, mark it as a required select box, and make sure it is not disabled.

Otherwise it will mark the box as not required and disable it.

Fourth and final draft

Once I got to this point, my other team mate noticed that we could make the code even better. This time it would shrink the jQuery code down to 6 lines (of course Medium makes the code formatting look a bit different).

%td
            = f.select :packing_facility_id, options_for_select(PackingFacility.internal.map { |pf| [pf.name, pf.id] }), {include_blank: true},  {required: true, class: 'required packing-facility', id:"packing-facility"}
          %td
            = f.select :storage_area_id, [], {}, { disabled: true, class: "storage-area", id:"blank"}
            = f.select :storage_area_id, options_for_select(Ops::Warehouse::StorageArea.all.filter_map { |storage_area| if storage_area.packing_facility_id == 1 then [storage_area.name, storage_area.id] end }), {include_blank: true}, { class: "storage-area is--hidden", id:"chicagoDropDown"}
            = f.select :storage_area_id, options_for_select(Ops::Warehouse::StorageArea.all.filter_map { |storage_area|  if storage_area.packing_facility_id == 2 then [storage_area.name, storage_area.id] end }), {include_blank: true}, { class: "storage-area is--hidden", id:"sanBernDropDown"}
            = f.select :storage_area_id, options_for_select(Ops::Warehouse::StorageArea.all.filter_map { |storage_area|  if storage_area.packing_facility_id == 3 then [storage_area.name, storage_area.id] end }), {include_blank: true}, { class: "storage-area is--hidden", id:"atlantaDropDown"}
          %td
.... 1 $('#packing-facility').on('change', function() {
    //gives all dropDowns the default attributes  2  $('.storage-area').addClass("is--hidden").prop('required', false).prop('disabled', true);
 3  let selected = $(this).val(); 4  const allStorageAreas = $('.storage-area');   //does the right adding/deleting for attributes based on the packingFacilityId that gets passed in 5 allStorageAreas.eq(selected).removeClass("is--hidden").prop('required', true).prop('disabled', false);
 6 });

What makes this code better/shorter is

We’ve taken advantage of chaining jQuery functions
Only saved what we needed
One line of code runs per category of dropDowns; category meaning selected or not selected

First, as I’ve noted in the comments, we assign all drop downs the default attributes of not being required and all disabled with everything being hidden in the beginning.

Then we’ve saved all our storage areas into one variable which ultimately makes a storage-area array behind the scenes with jQuery. Of course we also save what the user selected.

Then we just use the .eq selector from jQuery which takes in an argument of the id we want to select and change those attributes in the end.

Finally we’ve got it how it should be

If you select the Chicago packing facility then…

Selecting the San Bern packing facility…

Refactoring code was always something I struggled with when I was first learning how to code and it’s still something tricky for me to tackle to this day.

Ultimately though I recognize it builds programming skill in a special kind of way because you’re using the information you already know and seeing how it can be modified more efficiently so you can write better code for other solutions later on.

If you’ve read this far I hope that you’ve learned something from taking a look at how this solution evolved over time or maybe my explanations will help you to write more efficient solutions on your own.