atom / tabs

Tabs in Atom
MIT License
111 stars 118 forks source link

Delay activation of item on mousedown event #439

Closed matthewwithanm closed 7 years ago

matthewwithanm commented 7 years ago

Because the item's activation could result in a change of focus, we need to wait to take it until the event has finished propagating. Otherwise, the focus will be set by the browser as part of the default action for the mousedown event.

Sorry there's no test but it doesn't seem possible to simulate the problematic browser behavior and still test what we want to.

matthewwithanm commented 7 years ago

The failure must be related but it doesn't repro for me on master or 1.17-beta5 😖

matthewwithanm commented 7 years ago

Okay so @zertosh just told me that it actually does make sense that the timeout could fire before setImmediate() (TIL). Using the same scheduler makes us green. (The promise brings in the microtask queue too, but that's not important…I just did it because we don't have a waitsForCallback 😝)

Here's his proof script, if you're interested!

setTimeout(() => {
  console.log('setTimeout-1');
}, 1);

setImmediate(() => {
  console.log('setImmediate');
});

const soon = Date.now() + 100;
while (soon > Date.now()) {}
as-cii commented 7 years ago

This looks good, @matthewwithanm! Thanks for this pull request. ✨

I only tweaked tests a little bit in a295987 to rely more on the state of the system rather than waiting for the next setImmediate callback. I will 🚢 this as soon as we get a green build.