Correct scroll sync issues

[longrunningprocess]

Description

There are a number of scenarios where the user can be left disoriented by the fact that the entry that is selected is not visible in the full list of entries on the left. Some of the use cases are outlined in the comments of #1133

Fixes #1133 Fixes #1130 Fixes #1014

Type of Change

Only keep lines below that describe this change, then delete the rest.

• Bug fix (non-breaking change which fixes an issue)

Screenshots

Video's demoing the fixes are already captured in the issue comments.

Verified / Tested on Staging

• list scrolls to entry when PgUp / PgDn keyboard shortcuts are used
• list scrolls to entry when Prev / Next buttons are used
• list scrolls to entry when refreshing page on an entry that is down the list (keeping editor state)
• When clearing the filter via "show all" button, list scrolls to selected entry
• When clearing text in the search box, list scrolls to selected entry
• List scrolls to entry when first starting from list view, e.g. entry near bottom of list

Checklist:

• I have performed a self-review of my own code
• I have reviewed the title/description of this PR which will be used as the squashed PR commit message
• I have commented my code, particularly in hard-to-understand areas
• I have added tests that prove my fix is effective or that my feature works

[megahirt]

This is a fantastic PR that squashes some bugs that have been out there for years. Many thanks for also finding some methods that were async but not explicitly set as such. This should help with weird behavior as well. Great job!

[megahirt]

merging per conversation with @longrunningprocess

[longrunningprocess]

This is a fantastic PR that squashes some bugs that have been out there for years. Many thanks for also finding some methods that were async but not explicitly set as such. This should help with weird behavior as well. Great job!

Thanks @megahirt , it definitely gets the job done. I would've preferred not to compromise on the solution but the reload is reliable.

[megahirt]

@longrunningprocess This is such a great improvement that I wanted to get this in with the current deployment that we are testing on QA, so I did a bit of extra git to move it into staging. I'm very excited about this.

In my testing, I found another use case that isn't current working that I can reproduce.

I've added it to the "tested in staging checkbox above"

• List scrolls to entry when first starting from list view, e.g. entry near bottom of list

Here are the repro steps:

1. go to my projects and click on the project name
2. in the list view, click on an entry that is at least 15 down in the list (so it must scroll)
3. see that it doesn't scroll and there is a console log error
   

This only appears to happen when the app hasn't loaded before, so if you go to the list a second time without leaving the project, the scroll to entry works as designed.

I suspect this is a case where an async method is called synchronously, as you discovered in our code (which is all over)

[longrunningprocess]

good find, thank you! I'll look at that today and plan on creating a PR against staging for your review.

[longrunningprocess]

bummer, I'm having a hard time reproducing it locally at least...mine is with a non-S/R type project. I'll add that same project you used and see if the problem has something to do with S/R projects.

Screen.Recording.2021-12-01.at.11.24.41.AM.mov

[longrunningprocess]

having some trouble getting S/R to work locally but I was not able to reproduce in QA either...maybe I'm missing something in the use case set up?

Screen.Recording.2021-12-01.at.2.12.30.PM.mov

[longrunningprocess]

ok, I'm getting it now. I had to throttle the network down to a fast 3G and that does it.

[longrunningprocess]

made an attempt at this in here: staging...bugfix/broken-scroll-during-poor-connections

This resolves the issue even at "slow 3G" throttling...it's still a gross fix but it might work for a while.

All other scenarios still test out fine with this impl. I couldn't find a situation where that is(':visible') check was ever true so I just removed it. If you want to give this some more rigorous testing we can do that. Just let me know how you want to proceed @megahirt .