maxence-charriere/go-app

I think there is serious bug with `ctx.PreventUpdate()` now

oderwat opened this issue · 5 comments

When I use ctx.PreventUpdate() in an app.EventHandler (OnClick / OnScoll this problem is confirmed) I do not get any update till I "manually" call "c.Update()" on that component at a later stage. As a work around I need to add c.Update() to all other event handlers on the same component, which works in the current proof of concept (infinite scroller) but not for our real applications with many event handlers that would need to be changed to work around this problem.

Sadly I can't easily test with other versions because of the "js" problem when using older versions with the current go compiler.

I have a feeling as if the ignored component is not reset. I am pretty sure it worked with older versions in the way I expect it to work: A one time prevention of an update, that would otherwise be triggered through this handler.

This behavior is pretty important because we can't have rendering of entries in large lists for example in a OnScroll() (or Intersection) handler when nothing needs to be done (similar to key presses in input fields).

One way to fix this is:

image

Another is to clear the map (or create a new one. I am not sure about the memory allocations):

image

Last variant is what I will try out till it is fixed or otherwise handled.

I am pretty sure it worked with older versions in the way I expect it to work

Hey @oderwat, I did not touch this since it was implemented unless you are on the v10 feature branch. Can you confirm if you are using a stable branch?

Any chances you can test this on https://github.com/maxence-charriere/go-app/tree/v10 too?
I'm almost done with this.

We were using the latest release (v9.8.0). It may be that the effect we see now, was mitigated through something else in the past and that this behavior did not show up. I think that it is wrong to not reset the update behavior after the frame was rendered.

The thing is, that this kind of preventing updates was "invented" and is used for preventing updates while debouncing. There are delayed backend calls which used c.Update() at some place and this was the only action that happens in that component. I am not sure why this problem did not came up earlier. I would need to check old code.

I think the implementation I had made the PR was different and a "one shot" variant. We also were a long time on our own branch of go-app at that time. Our last app did not need any update prevents and the one we used it with was just a tech demonstration that was not updated / used for a long time. We are currently in Beta with a large app that we developed over the last year and that one did not use the update prevents till now. It came up while implementing an virtual endless scroller that helps making updates for a large list faster on slow mobile devices. I ran into this problem while optimizing the code to save on "Render()" calls.

I will check with v10 later today and report back. Right now I have everything working with that change I posted earlier.

I close this as it is fixed with v10, and we moved our codebase over already.