1

I have a review popup which when closed using the x button should close itself and then call a XMLHttpRequest to call a web service for updating some feedback info.

The issue I have is that when the button is clicked the window popup does not close instantly but seems to wait for the XMLHttpRequest to return 200. Is this due to "EventListener" behaviour or the "XMLHttpRequest"?

I should probably mention that I am specifically making XMLHttpRequest synchronous via the "false" param in xmlHttp.open( "GET", URL, false ); as this seems to be required as when the request is async eg: xmlHttp.open( "GET", URL, true); the if (xmlHttp.status === 200) doesn't run and is required for setting a global variable needed down the line.

I have added my code below.

Is there a way for my feedbackPopup() function to close the popup window without waiting for the web service 200? I need the window close to be instant.

Sorry if im making a silly mistake somewhere, still learning.

Thank you in advance for any help.

//used for running closeFeedback Logic

var close = document.getElementById('closeOverlay');
  close.addEventListener('click', closeFeedback);
  
  
  
function updateFeedback(a, b, c){

  const host = window.location.host;
  let URL = 'someURL${a}${b}${c}'

  var xmlHttp = new XMLHttpRequest();
  xmlHttp.open( "GET", URL, false ); // false for synchronous request
  xmlHttp.send( null );

  if (xmlHttp.status === 200){
  // this is not set for future use if line 14 is set to true 
    SomeGlobal = b;
  }else{
    console.log("xmlHttp.status","",xmlHttp.status)
  }

 }
 
 //this is used for both opening and closing the popup which is why the conditional operator is used
 function feedbackPopup() {
  el = document.getElementById("feedback");
  el.style.visibility = (el.style.visibility == "visible") ? "hidden" : "visible";


}



//should close feedback and call web service function 
function closeFeedback(){

//ISSUE : this closeFeedbackWindow should close the window before updateFeedback runs and should not wait for updateFeedback to retunr 200???
  feedbackPopup();

  updateFeedback(a, b, c);
  
  
  ...
  //More logic here which uses the SomeGlobal variable set in updateFeedback


}
JoeXYZ
  • 47
  • 6
  • 1
    `false for synchronous request` a) deprecated, and b) that's why nothing else runs while waiting for the request to complete – Jaromanda X Sep 12 '22 at 11:03
  • @JaromandaX Thank you for the comment, you may have missed that I have specifically set this to fale in order to ensure my `if (xmlHttp.status === 200)` as this is required for setting a global variable. I was not aware of the deprecated aspect however, what would you suggest as a replacement? – JoeXYZ Sep 12 '22 at 11:07
  • 1
    I missed nothing - synchronous requests on the main thread are deprecated - while that request is in flight, the UI will remain unchanged despite you changing it (try in firefox, you may have a different result) – Jaromanda X Sep 12 '22 at 11:09

1 Answers1

2

As JaromandaX said, the problem is that you're making a synchronous ajax request. That locks up the UI of the browser (at least that tab, in some browsers worse than that) while the network operation is running. Let the request default to being asynchronous instead. (Synchronous requests may even stop working someday.)

this is not set for future use if line 14 is set to true

That's only because you're not handling the request correctly. To handle an asynchronous request, use onload and onerror (or onreadystatechange for more detail); more in MDN's documentation. And since you have further logic after the call to updateFeedback, you need to (if we're doing to do this the old-fashioned way) add a callback to it:

function updateFeedback(a, b, c, callback) {
    const host = window.location.host;
    let URL = 'someURL${a}${b}${c}'

    var xmlHttp = new XMLHttpRequest();
    xmlHttp.onload = () => {
        SomeGlobal = b;
        callback(true); // Worked
    };
    xmlHttp.onerror = () => {
        console.log("xmlHttp.status", "", xmlHttp.status)
        callback(false); // Didn't work
    };
    xmlHttp.open("GET", URL);
    xmlHttp.send(null);
}

The the call to it becomes:

updateFeedback(a, b, c, (success) => {
    //More logic here which uses the SomeGlobal variable set in updateFeedback
});

Or better yet, use fetch, the modern standard replacement for XMLHttpResponse, and make updateFeedback return a promise:

function updateFeedback(a, b, c) {
    const host = window.location.host;
    let URL = 'someURL${a}${b}${c}'

    fetch(URL)
    .then((response) => {
        if (response.ok) {
            SomeGlobal = b;
        } else {
            throw new Error(`HTTP error, status = ${response.status}`);
        }
    })
    .error((error) => {
        console.log(error.message ?? error);
    });
}
T.J. Crowder
  • 1,031,962
  • 187
  • 1,923
  • 1,875
  • Thank you for the support. I have tried to used the first solution but this does not set the global variable. If I run a console.log after the `updateFedback()` function call the global is not as expected. – JoeXYZ Sep 12 '22 at 11:24
  • 1
    @JoeXYZ - I just noticed you had that *"More logic here which uses the SomeGlobal variable set in updateFeedback"* comment and was updating the answer. I've done that now. Once something is async, everything depending on it has to understand and handle that. – T.J. Crowder Sep 12 '22 at 11:25
  • 1
    @JoeXYZ - *"If I run a console.log after the updateFedback() function call the global is not as expected."* Indeed, it wouldn't be if you did that synchronously: [*Why is my variable unaltered after I modify it inside of a function? - Asynchronous code reference*](https://stackoverflow.com/questions/23667086/why-is-my-variable-unaltered-after-i-modify-it-inside-of-a-function-asynchron) – T.J. Crowder Sep 12 '22 at 11:26
  • Thank you this worked. I do have another question. I have a similar function for another popup. This function is basically identical to `updateFeedback()` but call a different web service and also sets a global variable. I cant use the `(a, b, c, (success) => {` notation for this is doesn't call any other function but the global variable set in this call is used in other function later down the line. Is this solution still good for this? – JoeXYZ Sep 12 '22 at 12:14
  • The thing that confuses me is that I have an `onclick` which runs a function which again contains both a close function and a web service call function and the popup closes instantly as expected. However when this function is called from a `click EventListener` waits for the 200 – JoeXYZ Sep 12 '22 at 12:16
  • 1
    @JoeXYZ- I think that must be some kind of observational error, there's no way for the event handler to know there's even an XHR outstanding, much less wait for it, if it's asynchronous. – T.J. Crowder Sep 12 '22 at 12:17
  • 1
    @JoeXYZ - Fundamentally yes this still applies, but beware that while the call is outstanding, that variable won't have been changed, so anything using it in the meantime will see the old value. There are various ways to avoid that being a problem. Probably the simplest is to never make the global variable available synchronously; instead, wrap it in a promise and always use that promise when accessing it. The code updating it would replace the promise with a new one it fulfills with the new value. – T.J. Crowder Sep 12 '22 at 12:18