Skip to content
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

Show spinner when opening/creating a project, take #2 #6827

Merged
merged 10 commits into from
May 26, 2023

Conversation

Procrat
Copy link
Contributor

@Procrat Procrat commented May 24, 2023

Pull Request Description

This is almost the same as #6321, but it got reverted since it created an impactful regression.

To recap, this fixes #5505: A spinner with the Enso logo is now shown for most of the duration when opening and creating projects.\

The only new code is the last commit which ensure that it works when there is no project manager available.

Note that the value of the spinner is somewhat lessened with the new dashboard since some of the startup time is covered after clicking the play button in the dashboard but before opening the project.

With the old dashboard:

2023-05-24.13-18-16.online-video-cutter.com.mp4

With the new dashboard:

2023-05-24.13-19-26.online-video-cutter.com.mp4

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • The documentation has been updated, if necessary.
  • Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
  • All code follows the Scala, Java, and Rust style guides. In case you are using a language not listed above, follow the Rust style guide.
  • All code has been tested:
    • Unit tests have been written where possible.
    • If GUI codebase was changed, the GUI was tested when built using ./run ide build.

@Procrat Procrat force-pushed the wip/procrat/prolong-project-spinner-5505 branch from fb283d6 to 8216a26 Compare May 24, 2023 11:45
@Procrat Procrat marked this pull request as ready for review May 24, 2023 11:45
@farmaazon
Copy link
Contributor

QA Report: 🟡

When opening project with a new dashboard, there is a brief moment when I see "Initializing project..." what suggests the loading spinner is hidden a bit too early.

loading-screen-2023-05-25_14.20.56.mp4

We could live with it, but it should be checked + I'm not sure if it isn't more prominent on weaker hardware.

@wdanilo
Copy link
Member

wdanilo commented May 26, 2023

  1. Could you link the regression ticket, please? I want to better understand what changed between this and previous implementation take.
  2. Could you link the commit with the changes between this and the previous implementation? In the PR description you've written "The only new code is the last commit which ensure that it works when there is no project manager available.", which is probably not the case anymore.

@Procrat
Copy link
Contributor Author

Procrat commented May 26, 2023

  1. Could you link the regression ticket, please? I want to better understand what changed between this and previous implementation take.

This is the regression issue: #6708

  1. Could you link the commit with the changes between this and the previous implementation? In the PR description you've written "The only new code is the last commit which ensure that it works when there is no project manager available.", which is probably not the case anymore.

It's this one: 8216a26

* develop:
  Allow casting a Mixed column into a concrete type (#6777)
  Stop graph editing when in full-screen visualization mode (#6844)
  Handle `show-dashboard` event (#6837)
  Fix some dashboard issues (#6668)
  Fix JWT leak (#6815)
  Fix "set username" screen (#6824)
  Fallback to opened date when ordering projects (#6814)
  Various test improvements to increase coverage and speed things up (#6820)
  do not activate nested dropdowns together (#6830)
  Clearly select single specialization with enum dispatch pattern (#6819)
  Prevent incorrect application of list widget on incompatible expressions (#6771)
@Procrat
Copy link
Contributor Author

Procrat commented May 26, 2023

After discussion with @farmaazon, we decided to merge this as is since it contains useful changes, but not close the original issue since it's not resolved yet.

@Procrat Procrat merged commit 0ed78f9 into develop May 26, 2023
26 checks passed
@Procrat Procrat deleted the wip/procrat/prolong-project-spinner-5505 branch May 26, 2023 16:20
@wdanilo
Copy link
Member

wdanilo commented May 28, 2023

@Procrat I've left a comment in the code but it's not displayed here:

image

Procrat added a commit that referenced this pull request May 30, 2023
…le-6756-6804

* develop: (22 commits)
  Coalesce graph editor view invalidations (#6786)
  Append warnings extracted before tail call execution (#6849)
  Detect and override hooks of the same kind (#6842)
  Dynamic app resampling and better performance measurements. (#6595)
  Show spinner when opening/creating a project, take #2 (#6827)
  Infrastructure for testing inter project imports and exports (#6840)
  Only initialise visualisation chooser if it is used. (#6758)
  Allow casting a Mixed column into a concrete type (#6777)
  Stop graph editing when in full-screen visualization mode (#6844)
  Handle `show-dashboard` event (#6837)
  Fix some dashboard issues (#6668)
  Fix JWT leak (#6815)
  Fix "set username" screen (#6824)
  Fallback to opened date when ordering projects (#6814)
  Various test improvements to increase coverage and speed things up (#6820)
  do not activate nested dropdowns together (#6830)
  Clearly select single specialization with enum dispatch pattern (#6819)
  Prevent incorrect application of list widget on incompatible expressions (#6771)
  Update GraalVM to 22.3.1 JDK17 (#6750)
  Import/export syntax error have more specific messages (#6808)
  ...
farmaazon added a commit that referenced this pull request Jul 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Users should see a spinner / loading indicator when opening a project
3 participants