hotosm / osm-tasking-manager2

Designed and built for Humanitarian OpenStreetMap Team collaborative emergency/disaster mapping, the OSM Tasking Manager 2.0 divides an area into individual squares that can be rapidly mapped by thousands of volunteers.
http://tasks.hotosm.org
Other
425 stars 156 forks source link

Don't check for updates when previous request is pending #925

Closed pgiraud closed 7 years ago

pgiraud commented 7 years ago

Fixes https://github.com/hotosm/osm-tasking-manager2/issues/924

@CloCkWeRX Please review.

CloCkWeRX commented 7 years ago

The type of pattern I tend to use to debounce ajax queries is something like

    search_whatevers: function (query) {
      var self = this;
      self.set("search_results", []);
      if (self.pending_request) {
        clearTimeout(self.pending_request);
        self.pending_request = null;
      }

      // Default to waiting 50ms before executing the request, incase a user continues typing.
      self.pending_request = setTimeout(function () {
       AjaxQueryMethodOrJqueryCall(query, onSuccess = function (data) {
          self.set("search_results", data);
        });
      }, 50);
    },

That basically lets you do pretty responsive type-ahead behaviour and clears pending requests on the next keystroke; but executes pretty quickly after you stop typing.

CloCkWeRX commented 7 years ago

LGTM, it'll improve things

pgiraud commented 7 years ago

That basically lets you do pretty responsive type-ahead behaviour and clears pending requests on the next keystroke; but executes pretty quickly after you stop typing.

We're not in the case of user interactions (type-ahead with keystrokes). We are not supposed to cancel any pending request, we just want to make sure that we don't send a new request if there's already a pending one.

However, I agree the current code is not clear and still needs work. I'll try to improve it a bit more.

CloCkWeRX commented 7 years ago

we're not in the case of user interactions

Particularly when it was slow, I had the habit of being uncertain if a click succeeded or of my attention flitting to another tile ("KEEP CALM AND KEEP HITTING F5"). I think when its performing well, not much of a problem - more when under heavy load and it becomes a case of a lot of pending requests.

For what its worth, here's the xhr requests from current production with me triggering a few different cells quickly. The ~20ms check for updates is me clicking on the 'wrong' tile, then clicking on the next; with check-for-updates being triggered from both tile GET calls I think (guessing, haven't looked at code).

image

I reckon merge what you have, it'll improve stuff; but try opening up the UI and rapidly switching between tiles - if its not producing a heap of requests anymore, hurray :)