zulip-terminal icon indicating copy to clipboard operation
zulip-terminal copied to clipboard

Show loading screen in urwid.

Open kaustubh-nair opened this issue 5 years ago • 4 comments

These commits are not perfectly isolated, because there are a lot of things to be done here:

  • Show LoadingView first and run Model and View initialization asynchronously.
  • Move settings from run.py to LoadingView
  • Move spinning cursor from Controller to LoadingView

kaustubh-nair avatar May 20 '20 09:05 kaustubh-nair

Making a new PR for this. I'll keep the current tutorial PR #583 only for tutorial related enhancements.

Adding tests.

kaustubh-nair avatar May 20 '20 09:05 kaustubh-nair

@kaustubh-nair I tried this out manually and it looks great! 👌

sumanthvrao avatar Jun 09 '20 02:06 sumanthvrao

@kaustubh-nair I replied on the stream about this - functionally this does what we want, but the structure is a little confusing and smaller commits may help explain which changes you're making and why. It's great that you're picking up others' older PRs and moving them forward, but it can be challenging precisely since they're not your code

Some aspects to consider:

  • what (current) aspects of the model and view can be started in parallel or not?
  • we have various cases in the code where we check whether the view is set up (hasattr); if we can ensure the ordering in the initial setup then can we remove these?
  • while Controller() and its .main(...) are run consecutively, if we change the order we should understand/explain why
  • when commits simply "move" code around, make sure that the code hasn't been adjusted by other commits between this - this is particularly true with rebasing older PRs!

neiljp avatar Jun 09 '20 03:06 neiljp

Heads up @kaustubh-nair, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/main branch and resolve your pull request's merge conflicts accordingly.

zulipbot avatar Jan 30 '21 20:01 zulipbot