3

I have two Ajax calls on the same page, the first one worked perfectly so I duplicated it and changed the necessary values so they were independent, however, the second Ajax call doesn't work. I've tested it on a separate page and it works fine, it only doesn't work when both calls are on the same page, am I missing something?

jQuery('#lampbase').on('change', function(){
  var basevalue = $(this).val();
  jQuery.ajax({
    url: 'basesfetch.php',
    method: 'post',
    data: {basevalue1: basevalue},
    success: function(result){                    
      $("#baseresponse").html(result);
      $( "select" ).on('change',function() {
        var str = "";
        // For multiple choice
        $( "select option:selected" ).each(function() {
          str += $( this ).val() + " "; 
          console.log(str);
        });
      });

      jQuery('#lampbase').on('change', function(){
        $(".lampoptions").fadeIn();
      });
      
      jQuery("#lampcolor").change(function(e){
        var DefaultOption = $('option:selected', this).data('code');
        $("#detailcolor option[data-code='" + DefaultOption + "']").prop("selected", true);
        $("#lampholder option[data-code='" + DefaultOption + "']").prop("selected", true);
        $("#lampholder option[data-code='" + DefaultOption + "']").prop("selected", true);
        $("#FlexColour option[data-code='" + DefaultOption + "']").prop("selected", true);
        $("#Switch option[data-code='" + DefaultOption + "']").prop("selected", true);
      });
      
      $("select option").val(function(idx, val) {
        $(this).siblings("[value='"+ val +"']").hide();
      });          
    }
  });
});

jQuery('#lampshade').on('change', function(){
  var shadevalue = $(this).val();
  jQuery.ajax({
    url: 'shadefetch.php',
    method: 'post',
    data: {shadevalue1: shadevalue},
    success: function(result){                    
      $("#shaderesponse").html(result);
      $( "select" ).on('change',function() {
        var str = "";
        // For multiple choice
        $( "select option:selected" ).each(function() {
          str += $( this ).val() + " "; 
          console.log(str);
        });
      });
    }
  });
});

HTML Code:

<select id="lampbase">
  <option value="first">Select</option>
  <?php 
    $sql = mysqli_query($connection, "SELECT DISTINCT ProductName FROM TableLamps_CSV");
    while ($row = $sql->fetch_assoc()){
      $productname = $row['ProductName'];
      echo "<option value=\"$productname\">" . $row['ProductName'] . "</option>";
    }
  ?>
</select>

<select id="lampshade">
  <option value="first">Select</option>
  <?php 
    $sql = mysqli_query($connection, "SELECT DISTINCT ShapeName FROM shadetable");
    while ($row = $sql->fetch_assoc()){
      $shapename = $row['ShapeName'];
      echo "<option value=\"$shapename\">" . $row['ShapeName'] . "</option>";
    }
  ?>
</select>
Cjmarkham
  • 9,484
  • 5
  • 48
  • 81
RuFFCuT
  • 307
  • 11
  • 31
  • can you show your html code? if your `#lampshade` is a `select` then on your first ajax success, you are over writing the `change` event of it. – roullie Dec 10 '21 at 06:23
  • Thanks for the reply! They are indeed both select boxes, I've added the HTML. – RuFFCuT Dec 10 '21 at 06:27
  • 4
    Define "doesn't work". Any errors in your developer console? – Cjmarkham Dec 31 '21 at 01:32
  • Nope no errors in console relating to this, and they work independently. By doesn't work I mean, nothing happens it's strange - the stuff in on success just doesn't fire but they both work fine when on separate pages. Live example is here: https://whiz.cz/ItQ – RuFFCuT Dec 31 '21 at 01:43
  • So you can see the request happen in the network tab of the console? Are the requests both successful (2XX)? – Cjmarkham Dec 31 '21 at 01:51
  • I can see the first request works but the lamp shade doesn't (because you commented it out in your live example...). A big issue with your code (which was mentioned before) is that you have several event listeners overwriting the others such as `$('select').on('change'` and `change` listeners on the elements with ID's. TLDR: Change `$( "select" ).on` to target specific selects, not every select on the page – Cjmarkham Dec 31 '21 at 01:58
  • Event handlers do not overwrite each other, they stack up - they will all run simultaneously. Depending on what they do, their actions might cancel each other or conflict etc, but they all run. – Don't Panic Dec 31 '21 at 02:01
  • Each form element should have `name` attribute, this is how form knows what changed. https://www.w3schools.com/tags/att_select_name.asp Also, you override `change` event inside your success handler. If it's intentional and you want to replace entire HTML with AJAX response, you would need to create a reusable method that not only rewrites HTML, but also recreates AJAX bindings. It looks like after the first call you create only static HTML. – Anonymous Dec 31 '21 at 03:15
  • @RuFFCuT, where are you, does my answer help? :-) – Don't Panic Jan 07 '22 at 22:39
  • What a shame to waste half the bounty ... – Don't Panic Jan 08 '22 at 06:23

1 Answers1

3

There are a few problems with the code, some of which you may not have bumped into yet. But the one stopping your 2nd AJAX call from working is quite simple:

Problem 1: duplicate IDs

It isn't shown here in the code you've shared on SO, but I checked the site you linked to (https://whiz.cz/ItQ) and found you have two elements with the same ID - lampshade.

There's the <select> that we're looking at:

<select id="lampshade">

But a div a few levels up containing that select also has the same ID:

<div class="container-fluid" ... id="lampshade">

IDs must be unique on the page. If you have duplicates, and use the ID as a jQuery selector, only the first one will match. In this case the change event handler which you want to run when the select is changed is actually bound to the div, and only the div. divs don't have a change event, so that never fires. The event handler is not attached to the select at all.

Problem 2: nested event handlers

Currently the code adds event handlers inside other event handlers. Eg inside the #lampbase change handler, you add a select change handler, another #lampbase change handler, and a #lampcolor handler. The problem with this is that event handlers don't overwrite, or update, or replace previous handlers. They all just stack up, and they all stay active, and they will all run. In this particular case, it means that every time you change the #lampbase select, 3 new handlers are added.

Here's a simple JSFiddle demo showing how event handlers defined like this stack up.

If you go back and change that #lampbase select again, 3 new handlers are added on top of the existing 4, leaving 7 total, and they'll all run every time the relevant selectors are triggered. Next time you change #lampbase, the original change handler runs, $(".lampoptions") is faded in 3 times, and the code building the str from selected options runs 4 times.

In this particular case that's probably not causing any serious problems, as long as your users don't go back and change selects very often. But you can imagine this can get very messy very easily, and you might start to notice your page getting sluggish as all those duplicated handlers run.

The simplest fix is to define your handlers just once, outside any user interaction that might run repeatedly. If they need to use the current state of the page - eg currently selected options - have them determine that when they run (rather than defining them at the point in the flow when the state is what you want).

Note it is also possible to dynamically add/remove handlers with .on(), and .off() as users interact with the page, but there's no need for that extra complexity and code here.

Minor thing 1:

No need to add a separate change handler for #lampbase to do the fade in - you already have a change handler for that element, just do it in there.

Minor thing 2:

No need to define the same select handler twice, if you define it outside the other event handlers. You're not actually doing anything with the str that it generates, but maybe that's the next thing you're working on :-)

Minor thing 3:

This line:

$("#lampholder option[data-code='" + DefaultOption + "']").prop("selected", true);

is duplicated, probably just a typo.

Here's your code with these suggestions/fixes applied:

jQuery('#lampbase').on('change', function () {

    var basevalue = $(this).val();

    jQuery.ajax({
        url: 'basesfetch.php',
        method: 'post',
        data: {
            basevalue1: basevalue
        },
        success: function (result) {
            $("#baseresponse").html(result);

            // No need for a separate handler to do this
            $(".lampoptions").fadeIn();

            // I am not sure what this is doing, but note it is running 
            // for every select on the page.
            $("select option").val(function (idx, val) {
                $(this).siblings("[value='" + val + "']").hide();
            });
        }
    });
});

jQuery('#lampshade').on('change', function () {

    var shadevalue = $(this).val();

    jQuery.ajax({
        url: 'shadefetch.php',
        method: 'post',
        data: {
            shadevalue1: shadevalue
        },
        success: function (result) {
            $("#shaderesponse").html(result);
        }
    });
});

jQuery("#lampcolor").change(function (e) {
    var DefaultOption = $('option:selected', this).data('code');
    $("#detailcolor option[data-code='" + DefaultOption + "']").prop("selected", true);
    $("#lampholder option[data-code='" + DefaultOption + "']").prop("selected", true);
    // Duplicated, typo?
    // $("#lampholder option[data-code='" + DefaultOption + "']").prop("selected", true);
    $("#FlexColour option[data-code='" + DefaultOption + "']").prop("selected", true);
    $("#Switch option[data-code='" + DefaultOption + "']").prop("selected", true);
});

// Only necessary to define this once, and outside any other handler
$("select").on('change', function () {
    var str = "";
    // For multiple choice
    $("select option:selected").each(function () {
        str += $(this).val() + " ";
        console.log(str);
    });
});
Don't Panic
  • 13,965
  • 5
  • 32
  • 51