Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't save state to localStorage on every input field change #391

Merged
merged 2 commits into from
Jan 2, 2023

Conversation

josephfrazier
Copy link
Owner

I suspect this may be causing latency when editing the Description
field, see https://reportedcab.slack.com/archives/C9VNM3DL4/p1672679099120099:

2 minor bugs I see on iPhone chrome browser.

  1. The keyboard responds very slowly when typing in the description box.
  2. when scrolling sometimes the bottom 2/3rds of the screen is white

#1 (latency) is something
@joseph Frazier
and I have talked about — when I’m typing descriptions I type them into another window (sometimes even the address bar), then copy and paste, otherwise I could die waiting

looking into the description box keyboard latency issue again, I recall
that there's some code that tries to save the state of the app to
localStorage each time the value changes. The intent of this was to make
it so that if the app is closed or crashes, then you can open it again
and pick up where you left off. However, it's possible that re-saving on
every keypress is overkill and is taxing the browser. Let me see if I
can push an update that debounces the number of saves to at most like
once/twice per second, rather than upon every key press

I do like that saved state thing, it works, but if it saved less often (or even not at all) it would be fine

I did some digging, and found that I had introduced this
save-per-keypress behavior to handle a bug where the usual saving wasn't
happening when the app was swiped away to close it:
4b22f2d

However, I'm thinking that if removing this behavior fixes the latency
issues, then it's worth putting up with edge case, assuming the edge
case even exists anymore, now that 4.5 years have passed

I suspect this may be causing latency when editing the Description
field, see https://reportedcab.slack.com/archives/C9VNM3DL4/p1672679099120099:

> 2 minor bugs I see on iPhone chrome browser.
>   1. The keyboard responds very slowly when typing in the description box.
>   2. when scrolling sometimes the bottom 2/3rds of the screen is white

> #1 (latency) is something
> @joseph Frazier
>  and I have talked about — when I’m typing descriptions I type them into another window (sometimes even the address bar), then copy and paste, otherwise I could die waiting

> looking into the description box keyboard latency issue again, I recall
> that there's some code that tries to save the state of the app to
> localStorage each time the value changes. The intent of this was to make
> it so that if the app is closed or crashes, then you can open it again
> and pick up where you left off. However, it's possible that re-saving on
> every keypress is overkill and is taxing the browser. Let me see if I
> can push an update that debounces the number of saves to at most like
> once/twice per second, rather than upon every key press

> I do like that saved state thing, it works, but if it saved less often (or even not at all) it would be fine

> I did some digging, and found that I had introduced this
> save-per-keypress behavior to handle a bug where the usual saving wasn't
> happening when the app was swiped away to close it:
> 4b22f2d
>
> However, I'm thinking that if removing this behavior fixes the latency
> issues, then it's worth putting up with edge case, assuming the edge
> case even exists anymore, now that 4.5 years have passed
@josephfrazier josephfrazier marked this pull request as ready for review January 2, 2023 22:38
@josephfrazier josephfrazier enabled auto-merge (squash) January 2, 2023 22:38
@josephfrazier josephfrazier merged commit eff308e into main Jan 2, 2023
@josephfrazier josephfrazier deleted the joseph/dont.save.localstorage.on.inputchange branch January 2, 2023 23:07
josephfrazier added a commit that referenced this pull request Jan 21, 2023
josephfrazier added a commit that referenced this pull request Jan 21, 2023
…o at most twice a second (#394)

Reverts #391 but tweaks it a bit

That change seems to have resurrected the data loss issue I had originally fixed by adding the laggy auto-save functionality, so I'm revisiting it to see if I can find a way to auto-save without lag.

See also https://reportedcab.slack.com/archives/C9VNM3DL4/p1673184759964829:

> Not sure if this is an app problem or a me and my phone problem, but when I am starting a report, then leave the tab or browser before I finish, it makes me start all over from the login screen when I come back

> I'm seeing a similar issue myself, and I suspect it's a side effect of the change I made trying to fix the latency issues previously reported: https://reportedcab.slack.com/archives/C9VNM3DL4/p1672701191657709?thread_ts=1672697862.350319&cid=C9VNM3DL4
> 
> I'm away from a computer for this upcoming week, but afterwards, I might see if I can strike a balance between latency and data persistence, rather than having one or the other
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant