Drawing a LineString prematurely completes final double tap on touch devices

[jaybo]

mapbox-gl-js version: 3.1.2
mapbox-gl-draw version: 1.4.3

Steps to Trigger Behavior

1. Begin drawing a LineString.
2. Note that completing the line with a mouse click twice on the same vertex is a precisely triggered operation and works great.
3. Note that completing the line with a touch device often happens prematurely. This happens because multiple touchStart messages often happen within the 25 pixel touchBuffer quite accidentally in the course of attempting to make a single tap. Reducing the size of touchBuffer slightly improves the situation, but it is still a significant problem on an iPhone 13, and presumably most touch devices.

Expected Behavior

Drawing lines should work on touch devices.

Actual Behavior

Drawing lines terminates prematurely in ~25% of attempts.

Diagnosis

In draw_line_string.js line drawing completion is handled identically for both mouse click and touch.

DrawLineString.onTap = DrawLineString.onClick = function(state, e) {
  if (CommonSelectors.isVertex(e)) return this.clickOnVertex(state, e);
  this.clickAnywhere(state, e);
};

I believe that to solve this problem, a debounce needs to be applied to only the onTap path, so that additional taps within (say) 300mS of the first tap are ignored.

[jaybo]

I tried adding the following tap debounce code to draw_line_string.js and it seems to have completely solved the problem:

DrawLineString.lastTapTime = Date.now();

DrawLineString.onTap = function (state, e) {
  const now = Date.now();
  const tapDebounceTimeMS = 300;
  if (now - this.lastTapTime < tapDebounceTimeMS) {
    return;
  }
  this.lastTapTime = now;
  if (CommonSelectors.isVertex(e)) return this.clickOnVertex(state, e);
  this.clickAnywhere(state, e);
};

To get tests to run, I had to add a 400mS delay immediately before every instance of touchTap(...) in draw_line_string.test.js. I'm sure my naive sleep() below isn't the best solution, but I don't really understand how to best implement such a delay within the testing framework:

const tapDebounceDelayMS = 400;

function sleep(milliseconds) {
  const date = Date.now();
  let currentDate = null;
  do {
    currentDate = Date.now();
  } while (currentDate - date < milliseconds);
}

Used like:

      sleep(tapDebounceDelayMS);
      touchTap(map, makeTouchEvent(100, 200));

Although I didn't try it, all of the Polygon code should have the same treatment.

[jaybo]

An update on the above fix in draw_line_string.js. It improved the situation but didn't fix it completely. Here's an improved version that seems pretty bombproof.

const tapDebounceTimeMS = 1200;
DrawLineString.lastTapTime = Date.now();

DrawLineString.onTap = function (state, e) {
  if (CommonSelectors.isVertex(e)) {
    const now = Date.now();
    const elapsed = now - this.lastTapTime;
    this.lastTapTime = now;
    // tapping initial point again?
    if (elapsed < tapDebounceTimeMS || state.currentVertexPosition === 1) {
      return;
    }
    return this.clickOnVertex(state, e);
  }
  this.clickAnywhere(state, e);
};

1. Prevents accidentally tapping the initial point twice.
2. Increases the delay so that accidental line completions just never happen.
3. To get tests to run in draw-line-string-test.js, increase the sleep delay to (say) 1400.