Add spinner when loading results

[koesie10]

This will add a spinner to each repo row when the results for a particular repo are loading. It will also disable the row to make clear that it is loading and not clickable.

Screen.Recording.2022-12-02.at.16.22.42.mov

Checklist

• CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
• Issues have been created for any UI or other user-facing changes made by this pull request.
• [Maintainers only] If this pull request makes user-facing changes that require documentation changes, open a corresponding docs pull request in the github/codeql repo and add the ready-for-doc-review label there.

[robertbrignull]

Really like the idea. It makes it much nicer to use because you know it's doing somehting.

The only problem I see if that it only triggers the loading icon the first time you expand the results. We don't set resultsLoading on subsequent expandings. It is quicker to display the second time but it still takes almost as long, and the spinner doesn't show up so you're back to the experience where you think it's frozen.

The UI freezes quite badly for me when opening a large set of results, so it's not clear whether other clicks on things are doing anything. That's not a problem of this PR though, and this PR is only making things better.

Could we show the spinner every time? Would there be a problem knowing when to stop showing it? As in, is there an event that fires once rendering is complete?

[koesie10]

Unfortunately, React doesn't fire any event when it has completed rendering, so we can't really do that. We are also not really able to render the spinner icon before rendering the expanded results since React will try to batch these updates automatically. We could artificially try to add some delay between these two renders, but I think that will result in code that is really hard to follow.

[charisk]

Can we gradually render results, rather than a big bang?

[koesie10]

I'm not sure that's possible, since we would still be blocking the main rendering thread. If we stream in results and render e.g. 100 results every second, it might freeze every second and the layout might jump around since new results can potentially be rendered while scrolling.

The real solution here would probably be react-window or react-virtualized, but as far as I can tell they require us to know the height of every rendered result. Both do support differing heights for different items, but I think it would be quite hard to tell the exact height of every element without rendering them. We would need to take into account the height of every code snippet, as well as knowing padding, wrapping etc. This might be possible, but should probably be investigated as part of a larger issue.

[charisk]

Yeah that sounds crazy complicated :D This might be something that would be worth involving Design in too btw.

[robertbrignull]

I agree the virtualized rendering is probably the best solution, but it's hard to implement.

What I understood from @charisk's comment was the classic "load more" button (e.g. we render 10 results, but you can click a button to load the rest or load a larger chunk like 1000). That would be a lot easier to implement, but we'd want to run it past design before implementing.

[koesie10]

I've created a simple proof of concept for virtualization, which skips implementing some of the harder parts: https://github.com/github/vscode-codeql/tree/koesie10/results-virtualization. It is now heavily based on heuristics, but does render the results instantly.

I agree with you that adding a simple button to load more results or some other form of pagination would be a lot easier to implement, but I think that can be a separate PR. For this PR, just implementing the spinner will still improve the experience of loading results.

[elenatanasoiu]

We can continue the virtualization discussion (which Koen has already kinda sorta implemented?! ⚡) but I think this is a good enough change on its own. Any indication of progress is an improvement. 👍