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

core: update default loading animation #10761

Merged
merged 1 commit into from
Feb 18, 2022
Merged

core: update default loading animation #10761

merged 1 commit into from
Feb 18, 2022

Conversation

vince-fugnitto
Copy link
Member

@vince-fugnitto vince-fugnitto commented Feb 16, 2022

What it does

Fixes: #6463.
Closes: #7513.

The pull-request updates the default loading animation on startup to instead use the existing codicon icon for consistency with the rest of the framework.

The pull-request contains the minimal changes from #7513 without the need to replace the existing icon, or preload template not to break downstream applications.

preload-2.mov

How to test

  1. build the example-browser or example-electron application
  2. start the application - the new codicon icon should be used
  3. verify the changes with different themes - both light and dark

Review checklist

Reminder for reviewers

Signed-off-by: vince-fugnitto vincent.fugnitto@ericsson.com
Co-authored-by: Dmitry Sharshakov d3dx12.xx@gmail.com

@vince-fugnitto vince-fugnitto added theming issues related to theming ui/ux issues related to user interface / user experience labels Feb 16, 2022

.theia-preload::after {
animation: 1s theia-preload-rotate infinite;
color: #777; /* color works on both light and dark themes */
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately we cannot re-use a variable here like var(theia-foreground since it is not ready and would cause a flicker. Instead we can use a color which should work for most themes.

Copy link
Contributor

@JonasHelming JonasHelming left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks nice, thanks for taking care of this!

Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe using the load codicon (\eb19) would be a better fit here perhaps? At least to me, it looks more visually pleasing for this use case.

The commit updates the default loading animation on startup.
The changes make use of an existing `codicon` icon (for application
consistency).

Signed-off-by: vince-fugnitto <vincent.fugnitto@ericsson.com>
Co-authored-by: Dmitry Sharshakov <d3dx12.xx@gmail.com>
@vince-fugnitto
Copy link
Member Author

I believe using the load codicon (\eb19) would be a better fit here perhaps? At least to me, it looks more visually pleasing for this use case.

Sounds good! I updated it and agree it looks better in this case:

preload-2.mov

Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks great!

@vince-fugnitto vince-fugnitto merged commit 55bd2c5 into master Feb 18, 2022
@vince-fugnitto vince-fugnitto deleted the vf/loading branch February 18, 2022 22:46
@github-actions github-actions bot added this to the 1.23.0 milestone Feb 18, 2022
@kittaakos
Copy link
Contributor

@vince-fugnitto, thanks for the change. It looks excellent 😍 However, I want to keep the old spinner for a while but use the latest Theia. How can I customize it? In my extension, I can override the .theia-preload selector, but this is hardcoded here:

<div class="theia-preload">${this.compileIndexPreload(frontendModules)}</div>

So at app startup, I can see the new Theia spinner for a second or two if I add these selectors to my styles:

/* restore the old Theia spinner */
.theia-preload {
  background-image: var(--theia-preloader) !important;
  display: unset;
  justify-content: unset;
  align-items: unset;
}

.theia-preload::after {
  animation: none;
  color: unset;
  content: none;
  font: unset;
}
custom_spinner.mp4

I also tried to customize it on the code level here add replaced the CSS selectors on the DOM, but it's too late. How does one change the app spinner? Thank you!

I also noticed that the old spinner is still in the app and used by the mini-browser? Why is this difference in the Theia UI?

I appreciate your support.

@vince-fugnitto
Copy link
Member Author

@kittaakos you can provide your own custom preload element similar to eclipse-theia/theia-ide#73 (the actual splash screen can be whatever you'd like, and could be the old spinner if you require it).

I also noticed that the old spinner is still in the app and used by the mini-browser? Why is this difference in the Theia UI?

At the time I wanted to keep the changes minimal to only the splash screen and also make that loading animation different than regular loading within the app. We can of course update so that both are consistent if we want.

@kittaakos
Copy link
Contributor

Thank you so much for your time and help, @vince-fugnitto. 🙏 I was unaware of the custom preload option.

Finally, I ended up patching the generated index.html after the app build.


I wanted to keep the changes minima

I see. It makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theming issues related to theming ui/ux issues related to user interface / user experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve default loading animation
4 participants