fix: Move all componentWillUnMount functionality to respective events

[harshithpabbati]

Closes #1515
Move all functionality from componentWillUnMount lifecycle

@acao can you review this PR

[acao]

awesome work @harshithpabbati !

we just need to make sure to debounce the more high-throughput events to avoid performance/render-blocking issues, as invoking lots of storage.set() events can sometimes lead to issues like that

as for the delay, 500-1000ms should be fine. these values only need to be persisted in case the user navigates away or refreshes. this will essentially delay the write until the user has finished, or if there's lots of dragging or variables editing going on, it will only write to ls every 500ms-1000ms, rather than writing to localstorage 60 times a second or more

[harshithpabbati]

@acao should I debounce the whole function ?

[acao]

@harshithpabbati no, only the storage write events

[harshithpabbati]

@harshithpabbati no, only the storage write events

ok, I will update that

[acao]

if you debounce the setState() events, we get weird issues with resizing panes as you can see

[harshithpabbati]

if you debounce the setState() events, we get weird issues with resizing panes as you can see

oh, yeah true 👍

[acao]

@harshithpabbati why did you close this PR?

[harshithpabbati]

@harshithpabbati why did you close this PR?

That's because we found a better solution than this.

[acao]

@harshithpabbati i'm not sure that's the case, we just found a better way to reproduce the error. is there a conversation that i did't see where you and Mateo decided his solution was better?

[harshithpabbati]

Oh is it ? Then cool!

[harshithpabbati]

@harshithpabbati i'm not sure that's the case, we just found a better way to reproduce the error. is there a conversation that i did't see where you and Mateo decided his solution was better?

No, we didn't have any discussion

[acao]

@harshithpabbati yes, as i mentioned in discord, we will wait until both solutions are ready, and make sure we can reproduce the error with both and see if one doesn't solve it better than the other. both proposals are welcome.

[harshithpabbati]

Done updated @acao :)

[acao]

looking good @harshithpabbati !

as a rule of thumb, any event that has its events triggered many many times a second, such as resize or keyboard stroke, should debounce all the localStorage.setItem() calls (via storage.set())

[harshithpabbati]

looking good @harshithpabbati !

as a rule of thumb, any event that has its events triggered many many times a second, such as resize or keyboard stroke, should debounce all the localStorage.setItem() calls (via storage.set())

Sure, I will update it soon

[harshithpabbati]

@acao is this PR helpful ?

[acao]

@harshithpabbati it might still be?

[acao]

lol oops! tried merging via UI, fixing it now. gonna try and see if this resolves our user's issue

[acao]

@harshithpabbati fixed it! merging soon

[harshithpabbati]

lol oops! tried merging via UI, fixing it now. gonna try and see if this resolves our user's issue

Hope this solves the issue. It will and it should ofcourse

[acao]

awesome work! everything looks to be operating as it should be