Show loading screen in urwid. #667

kaustubh-nair · 2020-05-20T09:33:17Z

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 · 2020-05-20T09:33:44Z

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

Adding tests.

sumanthvrao · 2020-06-09T02:33:13Z

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

neiljp · 2020-06-09T03:50:31Z

@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!

This is done so that it can be later run asynchronously by introducing an intermediate view for loading.

Display LoadingView during startup. This blocks the settings shown in stdout while loading which will be added in the subsequent commits.

Stop showing loading settings for theme and autohide to stdout and use the LoadingView instead. Note that in case of invalid settings, output would still be displayed to stdout. Tests amended.

sumanthvrao

@kaustubh-nair Thanks for rebasing 👍 .

Additionally, I think this may avoid the UI distort race issue which we have seen in the past. I left a couple of commits but looks good overall.

sumanthvrao · 2020-08-12T14:00:13Z

zulipterminal/core.py

+                for cursor in '|/-\\':
+                    yield cursor
+
+        self.capture_stdout()


Do we need to capture stdout here again? We already do this now in the previous commit in the __init__ stage itself.

sumanthvrao · 2020-08-12T14:08:42Z

zulipterminal/ui_tools/views.py


    def set_spinner(self, spinner: str) -> None:
-        self.base_widget.set_text(self.text + [spinner])
+        self.body.base_widget.set_text(self.text + [spinner])


Is this something necessary for this commit? Couldn't we move this to the previous commit where you introduce the set_spinner class?

Ezio-Sarthak · 2021-01-13T17:41:38Z

zulipterminal/core.py

    def deregister_client(self) -> None:
-        queue_id = self.model.queue_id
-        self.client.deregister(queue_id, 1.0)
+        # Deregister before model has loaded.


Is this necessary in this commit?

zulipbot · 2021-01-30T20:34:44Z

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 added the size: L [Automatic label added by zulipbot] label May 20, 2020

kaustubh-nair marked this pull request as draft May 20, 2020 09:33

kaustubh-nair changed the title ~~Show loading screen in urwid.~~ WIP: Show loading screen in urwid. May 20, 2020

kaustubh-nair force-pushed the loading_view branch from 0165cc0 to 89df998 Compare June 2, 2020 14:22

zulipbot added size: XL [Automatic label added by zulipbot] and removed size: L [Automatic label added by zulipbot] labels Jun 2, 2020

kaustubh-nair changed the title ~~WIP: Show loading screen in urwid.~~ Show loading screen in urwid. Jun 2, 2020

kaustubh-nair marked this pull request as ready for review June 2, 2020 14:23

kaustubh-nair force-pushed the loading_view branch from 89df998 to bc62995 Compare June 2, 2020 14:26

sumanthvrao added feedback wanted PR needs review PR requires feedback to proceed and removed feedback wanted labels Jun 5, 2020

neiljp removed the PR needs review PR requires feedback to proceed label Jun 9, 2020

kaustubh-nair force-pushed the loading_view branch 2 times, most recently from 38b0209 to 3a5ce6a Compare June 13, 2020 15:47

kaustubh-nair added the PR needs review PR requires feedback to proceed label Jun 15, 2020

kaustubh-nair force-pushed the loading_view branch from 3a5ce6a to 4ea702d Compare June 18, 2020 13:37

kaustubh-nair added 4 commits August 2, 2020 19:40

refactor: core: Move Model and View initialization to new a method.

20666f3

This is done so that it can be later run asynchronously by introducing an intermediate view for loading.

core: Show loading screen in urwid.

589ea84

Display LoadingView during startup. This blocks the settings shown in stdout while loading which will be added in the subsequent commits.

core: views: Add spinning cursor to LoadingView.

4d8009e

core: Display settings in LoadingView.

4ae1f2a

Stop showing loading settings for theme and autohide to stdout and use the LoadingView instead. Note that in case of invalid settings, output would still be displayed to stdout. Tests amended.

kaustubh-nair force-pushed the loading_view branch from 4ea702d to 4ae1f2a Compare August 2, 2020 14:32

neiljp mentioned this pull request Aug 2, 2020

bugfix: helper: Check view is rendered before displaying errors. #761

Merged

sumanthvrao reviewed Aug 12, 2020

View reviewed changes

neiljp added this to the Next Release milestone Aug 16, 2020

Ezio-Sarthak reviewed Jan 13, 2021

View reviewed changes

neiljp removed this from the Next Release milestone Jan 22, 2021

neiljp added this to the Release after next milestone Jan 22, 2021

Base automatically changed from master to main January 30, 2021 20:30

zulipbot added the has conflicts label Jan 30, 2021

neiljp force-pushed the main branch from f9f483a to 793e73d Compare December 15, 2021 21:30

neiljp modified the milestones: Next Release, Release after next Mar 5, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Show loading screen in urwid. #667

Show loading screen in urwid. #667

Uh oh!

kaustubh-nair commented May 20, 2020 •

edited

Loading

Uh oh!

kaustubh-nair commented May 20, 2020

Uh oh!

sumanthvrao commented Jun 9, 2020 •

edited

Loading

Uh oh!

neiljp commented Jun 9, 2020

Uh oh!

sumanthvrao left a comment

Uh oh!

sumanthvrao Aug 12, 2020

Uh oh!

sumanthvrao Aug 12, 2020

Uh oh!

Ezio-Sarthak Jan 13, 2021

Uh oh!

zulipbot commented Jan 30, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Show loading screen in urwid. #667

Are you sure you want to change the base?

Show loading screen in urwid. #667

Uh oh!

Conversation

kaustubh-nair commented May 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kaustubh-nair commented May 20, 2020

Uh oh!

sumanthvrao commented Jun 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

neiljp commented Jun 9, 2020

Uh oh!

sumanthvrao left a comment

Choose a reason for hiding this comment

Uh oh!

sumanthvrao Aug 12, 2020

Choose a reason for hiding this comment

Uh oh!

sumanthvrao Aug 12, 2020

Choose a reason for hiding this comment

Uh oh!

Ezio-Sarthak Jan 13, 2021

Choose a reason for hiding this comment

Uh oh!

zulipbot commented Jan 30, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

kaustubh-nair commented May 20, 2020 •

edited

Loading

sumanthvrao commented Jun 9, 2020 •

edited

Loading