-
Notifications
You must be signed in to change notification settings - Fork 221
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
Make issue/PR body visible while the rest of the conversation is still loading #1128
Conversation
This sounds like a good idea. From looking at your screencast, a few things don't yet seem ideal to me:
Indeed, I'd also say to only show it when loading is done. |
Sometimes yes, and in these cases the pop-in effect can be seen in the current version of the app too, if the conversation loads fast enough. |
Done, give it a try :)
After experimenting a bit, I don't really agree on this. In case you have a long issue/PR body so that the comments loading indicator can't be seen without scrolling down, the top progress bar makes it clear why the comment editor is still kept hidden. |
I've pushed a fix and one more tweak, I think this can be considered ready. |
Reduces pop-in of elements when the PR conversation is loading and the PR body is visible
caa98c0
to
e25374e
Compare
Fixed conflicts and rebased. |
This PR mitigates the sometimes very slow loading of issue/PR conversations (due to GH REST API limitations we all know) by showing the issue/PR body as soon as possible, instead of waiting for the rest of the conversation to load.
This can be best demonstrated by the screencast here: https://streamable.com/aj4p28.
This way, the user can start reading the content of the issue/PR without waiting, drastically reducing the perception of the loading times.
The only thing I'm not too sure about is whether we should keep the editor bottom sheet hidden while loading is in progress. There is technically nothing that can prevent us not to show it, but at the same time it doesn't make much sense to use it before the conversation is loaded.
Note: I've tried to workaround some issues with the editor bottom sheet that are also present in the current version of the app. Some glitches are still present when rotating the screen, but I haven't had enough time to understand how bottom sheet positioning/resizing works in order to do a proper fix and I'm not sure whether I want to. 🙂
A sidenote: I personally think that we shouldn't expand automatically the list of check runs when there are some failed checks. As you can see in the screencast above, for repositories that have something like 20+ checks (PowerShell/PowerShell and dotnet/runtime are great examples) it becomes a bit annoying having to scroll down the list to reach the first comment (yes, you could collapse it but often just happens to go brainlessly straight to the conversation 😅).