Losing input events #214

purcell · 2014-08-19T17:11:50Z

I have a mithril component with a big list of stuff which gets filtered onkeyup as the user types into a search field.

The filtering is pretty slow, but what concerns me is that some key events get dropped as the user types. It seems like events disappear while the view is getting recomputed. Is this behaviour expected, and if so, is there an idiomatic workaround I'm missing?

You can try it out live here.

The relevant snippet is this:

m("input.form-control[type=search]", {placeholder: "Enter filter terms", autofocus: true,
                                 value: ctrl.filterTerms(), onkeyup: m.withAttr("value", ctrl.filterTerms)}),

and the full code for the component is here.

Cheers, and thanks again for mithril!

The text was updated successfully, but these errors were encountered:

lhorie · 2014-08-19T17:59:34Z

From a glance it seems that what is happening is that when a redraw happens, there's a lot of code running synchronously to recompute the visibility of all the rows, during which time the browser is becoming unresponsive.

You can do a few things to try and improve performance:

1 - when filtering out elements, remove them from the DOM instead of hiding them, and use keys on the tr's, i.e. change this

ctrl.sortedPackages().map(function(p) {
            return m("tr", { "class": visible[p.name] ? '' : 'filtered'},

to this

ctrl.sortedPackages().filter(function(p) {return visible[p.name]}).map(function(p) {
            return m("tr", { "class": visible[p.name] ? '' : 'filtered', key: p.name}, //assuming p.name is unique among tr's

This decreases the amount of virtual elements that need to be diffed, and reuses DOM elements.

2 - Consider using the occlusion culling technique from this article: http://lhorie.github.io/mithril-blog/an-exercise-in-awesomeness.html

The recurring idea here is to not render things into the document if you can avoid it. I counted around 24 thousand DOM elements on screen on page load. If you can decrease that number, things should get snappier.

purcell · 2014-08-19T18:57:04Z

Thanks. I'll take a look at that and get back to you. I know I've tried a couple of tricks to minimize the rendering overhead.

What's curious to me, though, is the lost events, because I'm pretty sure that until a very recent version of mithril, I didn't ever lose keypresses: they would just get queued up and then trigger a subsequent redraw cycle.

lhorie · 2014-08-19T19:26:13Z

Re: event queuing/loss: there was a recent change that makes redraw happen asynchronously (basically so that near-simultaneous events like keypress/input don't trigger double redraws. The updates to the model data should still happen synchronous to the event handlers though, so it seems weird that you'd lose the values from those events. It might be a race condition between what the UI is trying queue up and the render trying to update the input with the model value at the same time.

I'd have to look into it more

purcell · 2014-08-19T19:35:44Z

there was a recent change that makes redraw happen asynchronously (basically so that near-simultaneous events like keypress/input don't trigger double redraws.

Sounds like it could be a candidate.

I can easily zip up the application code if it would help: it's just a few static files, including JSON data which isn't available in the repo I linked to.

lhorie · 2014-08-19T19:38:06Z

That'd be helpful. Thanks

purcell · 2014-08-19T19:44:05Z

On its way... :-)

purcell · 2014-08-20T07:50:39Z

Re. the optimisations you suggested: "key" was the trick I was missing. I'd previously been conditionally creating the elements when filtering, and noticed that mithril was recreating the elements as the user backspaced characters out of the filter term. So I switched to toggling a class attribute. "key" is much better, thanks.

I'll try to survive without the extra mental overhead of occlusion culling for as long as I can. :-)

taylorhakes · 2014-09-15T01:05:28Z

I am also losing input events. Basically, the async redraw will overwrite an input value when typing quickly.

I implemented the TODOMVC in Mithril and ran into this when running the browser Selenium tests. I created a branch to show it. https://github.com/taylorhakes/todomvc/tree/mithril-async-bug . If you run the browser tests with -framework=mithril you can see the issue. The only workaround right now is to use oninput, but occasionally this fails as well. It is much more reliable though.

I would submit a pull request, but I am not sure the best fix. My thought process would be to not trigger a redraw when the value property changes. The change in value is already updating the model. I don't think the model should then update the DOM again.

lhorie · 2014-09-15T19:35:22Z

Hmm yeah, this one is tricky. One of the problems is that diffing isn't aware of what triggered the redraw (i.e. it could be an event from an input, but it could also be a number of other things). I'm going to need to try out different things to figure out a reasonable way to fix this.

lhorie · 2014-09-15T19:58:07Z

@taylorhakes on a completely separate note, there's a todomvc implementation here that @pygy and I were working on for a perf benchmark: http://lhorie.github.io/todomvc-perf-comparison/todomvc-benchmark/todomvc/mithril/#/

I just found out yesterday that there's a discussion over at the todomvc repo, as well that is related ( tastejs/todomvc#936 (comment) )

Maybe we can coordinate to get it submitted there

taylorhakes · 2014-09-15T20:55:20Z

@lhorie Sounds good. My TodoMVC is passing all functional tests (when using oninput). Here is a link to mine https://github.com/taylorhakes/todomvc/tree/mithril/architecture-examples/mithril .

I can help out on yours if you feel it is better. Up to you.

lhorie · 2014-09-17T17:42:03Z

@taylorhakes Oh does that mean your implementation is already submittable? If so, your code looks readable and everything, so I'd say go ahead and submit yours.

eddyystop · 2014-09-19T15:57:37Z

In reference to the @lhorie post above, github.com/tastejs/todomvc has agreed to add an implementation in mithril. Nice. It wouldn't have happened without a bunch of +1 being posted.

lhorie · 2014-09-25T11:28:36Z

I've added logic based on @taylorhakes first attempt, combined with an activeElement sniff

This will be in v0.1.22

purcell · 2014-09-25T18:31:20Z

Nice. I'll give it a whirl!

lhorie · 2014-09-25T18:45:52Z

Cool. Let me know how it goes

…olve double-redraws in onkeypress+oninput

Regression tests #214 and #288

bsuh · 2014-10-03T00:30:07Z

@lhorie The fix for #214 caused #288 but the fix for #151 did not cause #214. This is just an inherent issue with asynchronous rendering and still exists.

To actually test the issue, you have to run grunt connect:server:keepalive and visit http://localhost:8000/tests/e2e/test.html

Unfortunately PhantomJS doesn't support requestAnimationFrame and the issue does not show up running grunt teste2e. You need an actual browser.

I see three options:

Adopt something like in Simplify redraw logic to always be async #270 where you keep deferring updating the node.value until cachedAttr matches node.value. I'm playing around with it right now
Encouraging the use of oninput/onchange for bidirectional bindings instead of onkeydown/onkeyup/onkeypress, and discouraging any complex filtering on those events. As @taylorhakes said input events still can be lost if the rendering takes too long.
Synchronous rendering for certain events.

bsuh · 2014-10-03T01:13:40Z

3 doesn't seem like a good option. It makes the #214 regression test fail sometimes.
keyup 'a' -> synchronous render 'a'
keyup 'b' -> synchronous render 'ab'
keyup 'c' -> synchronous render 'abc'
keyup 'd' -> synchronous render 'abcd'
submit event -> asynchronous render ''
keyup 'enter' -> synchronous render to either 'abcd' or '' depending on the timing.

Same thing with option 1, where the submit event and keyup enter fight to set input value.

bsuh · 2014-10-03T01:27:02Z

I checked out MELPA and using the latest commit version on next there's no key input loss. The regression tests might be too strict and this might not be an issue for real world scenarios where humans are typing at sane speeds.

bsuh · 2014-10-03T02:04:35Z

Nevermind this issue is ok and should remain closed. Sorry for the confusion.

kvanbere · 2014-10-03T10:30:59Z

@bsuh This is certainly an issue with selenium testing though. We would want to test without delays ...

"onkeypress" would cause some keypresses to be missed. Potentially related to a mithril issue. MithrilJS/mithril.js#214

naveennagan · 2017-06-12T06:07:14Z

let m = require("mithril");

let keymap = {
  "13": "enter"
};

let keys = {
  onKey: function (keyVal, callback) {
    return function (e) {
      e.redraw = false;
      var key = e.keyCode + "";
      var keyInput = keymap[key];
      if (keyInput && keyInput === keyVal) {
        setTimeout(function () {
          m.redraw();
          callback();
        }, 0);
      }
    };
  }
};

module.exports = keys;

dead-claudia · 2017-06-12T08:07:47Z

@naveennagan Could you file a new issue explaining what went wrong?

naveennagan · 2017-06-12T11:42:31Z

@isiahmeadows i was just giving a solution for losing input events.
My approach was to pause redraw for sometime and then upon capturing enter key, push the callback to event loop(in timeout ) and then trigger the callback preceded by a redraw.

dead-claudia · 2017-06-12T12:44:07Z

Oh okay. Note that this bug was filed for 0.2.x, and we're on v1.1 now.

lhorie added the Type: Bug For bugs and any other unexpected breakage label Sep 15, 2014

lhorie mentioned this issue Sep 17, 2014

separate render from patch diff in the dom #164

Closed

This was referenced Sep 18, 2014

Fix redraw infinite delay #253

Merged

Return id from redraw #260

Closed

taylorhakes mentioned this issue Sep 19, 2014

Fix for losing input events #262

Closed

taylorhakes mentioned this issue Sep 20, 2014

Simplify redraw logic to always be async #270

Closed

lhorie closed this as completed Sep 25, 2014

bsuh mentioned this issue Sep 29, 2014

input value isn't reset if onsubmit event is triggered with enter #288

Closed

bsuh added a commit to bsuh/mithril.js that referenced this issue Oct 1, 2014

added regression test for MithrilJS#214

0bc94a7

lhorie mentioned this issue Oct 2, 2014

Bi-directional binding re-renders everything #151

Closed

lhorie pushed a commit that referenced this issue Oct 2, 2014

break #151 to fix #214 and #288. Use m.redraw.strategy("none") to res…

586591c

…olve double-redraws in onkeypress+oninput

lhorie added a commit that referenced this issue Oct 2, 2014

Merge pull request #293 from bsuh/regression-tests

35d328f

Regression tests #214 and #288

bsuh added a commit to bsuh/mithril.js that referenced this issue Oct 3, 2014

added regression test for MithrilJS#214

ddfc230

bsuh mentioned this issue Oct 3, 2014

update regression tests to introduce delays between keys #297

Merged

lhorie added a commit that referenced this issue Nov 13, 2014

fix #336 (related: #151, #214, #288)

753b1ab

lhorie mentioned this issue Nov 13, 2014

Active input can get out of sync when editing it's value oninput #336

Closed

lhorie mentioned this issue Feb 6, 2015

Enhance Redraw Strategy to be Granular #442

Closed

pygy mentioned this issue May 11, 2015

e2e tests failures #616

Closed

brandonPurvis added a commit to brandonPurvis/osf.io that referenced this issue Feb 26, 2016

Use "oninput" to capture input events.

3a3d8ee

"onkeypress" would cause some keypresses to be missed. Potentially related to a mithril issue. MithrilJS/mithril.js#214

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Losing input events #214

Losing input events #214

purcell commented Aug 19, 2014

lhorie commented Aug 19, 2014

purcell commented Aug 19, 2014

lhorie commented Aug 19, 2014

purcell commented Aug 19, 2014

lhorie commented Aug 19, 2014

purcell commented Aug 19, 2014

purcell commented Aug 20, 2014

taylorhakes commented Sep 15, 2014

lhorie commented Sep 15, 2014

lhorie commented Sep 15, 2014

taylorhakes commented Sep 15, 2014

lhorie commented Sep 17, 2014

eddyystop commented Sep 19, 2014

lhorie commented Sep 25, 2014

purcell commented Sep 25, 2014

lhorie commented Sep 25, 2014

bsuh commented Oct 3, 2014

bsuh commented Oct 3, 2014

bsuh commented Oct 3, 2014

bsuh commented Oct 3, 2014

kvanbere commented Oct 3, 2014

naveennagan commented Jun 12, 2017 •

edited by dead-claudia

dead-claudia commented Jun 12, 2017

naveennagan commented Jun 12, 2017 •

edited

dead-claudia commented Jun 12, 2017

Losing input events #214

Losing input events #214

Comments

purcell commented Aug 19, 2014

lhorie commented Aug 19, 2014

purcell commented Aug 19, 2014

lhorie commented Aug 19, 2014

purcell commented Aug 19, 2014

lhorie commented Aug 19, 2014

purcell commented Aug 19, 2014

purcell commented Aug 20, 2014

taylorhakes commented Sep 15, 2014

lhorie commented Sep 15, 2014

lhorie commented Sep 15, 2014

taylorhakes commented Sep 15, 2014

lhorie commented Sep 17, 2014

eddyystop commented Sep 19, 2014

lhorie commented Sep 25, 2014

purcell commented Sep 25, 2014

lhorie commented Sep 25, 2014

bsuh commented Oct 3, 2014

bsuh commented Oct 3, 2014

bsuh commented Oct 3, 2014

bsuh commented Oct 3, 2014

kvanbere commented Oct 3, 2014

naveennagan commented Jun 12, 2017 • edited by dead-claudia

dead-claudia commented Jun 12, 2017

naveennagan commented Jun 12, 2017 • edited

dead-claudia commented Jun 12, 2017

naveennagan commented Jun 12, 2017 •

edited by dead-claudia

naveennagan commented Jun 12, 2017 •

edited