feat: splash screen support for Electron

[sdirix]

What it does

Enhances the ElectronMainApplication to optionally render a splash screen until the frontend is ready.

The splash screen can be configured via the application config object theia.frontend.config.electron.splashScreenOptions. Mandatory is the option content which specifies a relative path from the frontend location to the content of the splash screen. Optionally width, height, minDuration and maxDuration can be handed over too.

By default the splash screen is off.

Implements #13410

Contributed on behalf of Pragmatiqu IT GmbH

TheiaSplashScreen.webm

How to test

1. Place a png in examples/electron/resources/ directory

2. Modify the Electron example application package.json to configure the splash screen, e.g. like this

   "theia": {
     "target": "electron",
     "frontend": {
       "config": {
         "applicationName": "Theia Electron Example",
         "reloadOnReconnect": true,
         "electron": {
           "splashScreenOptions": {
             "content": "resources/my-logo.png"
           }
         }
       }
     },
     "backend": {
       "config": {
         "frontendConnectionTimeout": -1
       }
     }
   }
   

3. Build and start the Electron example app

Play around with the various options, e.g. hand over a html file as the content and try the configuration options width, height, minDuration and maxDuration.

The PR now also configured the Electron example app already

Review checklist

• As an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

• As a reviewer, I agree to behave in accordance with the review guidelines

[tsmaeder]

The lib/frontend directory of an application is generated. What's the suggested process for putting something in there for adopters? Can we have our electron example implement this way?

[sdirix]

The lib/frontend directory of an application is generated. What's the suggested process for putting something in there for adopters?

I chose lib/frontend as this is often chosen for resolving files. I imagined users placing files in there like we do already: Via webpack. However it might not be the best place as it's then also served vie Express which we might not want.

So, I agree, it's probably better to simply resolve from the root of the electron application instead.

Can we have our electron example implement this way?

Locally I adapted the Electron example app for testing. I was unsure whether we wanted to make the splash screen the "default", so I did not modify it in the PR. Would you like to see it configured in the example?

[tsmaeder]

Would you like to see it configured in the example?

Yes, I think that would neatly document how it's done.

[sdirix]

Would you like to see it configured in the example?

Yes, I think that would neatly document how it's done.

Done 👍

[tsmaeder]

One last comment.

[sdirix]

Rebased to solve merge conflicts in changelog

[sdirix]

What don't we just make the type of showWindowEarly to be boolean | SplashScreenOptions? This would make the state space of the application simpler.

I thought about this: Maybe it would actually be better to split them. And showWindowEarly then controls whether we show the splash screen / main window before or after the window is ready to show content. I think that makes it more explicit and is actually a sensible option for both use cases.

[tsmaeder]

showWindowEarly then controls whether we show the splash screen / main window before or after the window is ready to show content.

Why configure a splash screen if you don't show it? So splashScreeen/false is a combination that never makes sense.

[sdirix]

showWindowEarly then controls whether we show the splash screen / main window before or after the window is ready to show content.

Why configure a splash screen if you don't show it? So splashScreeen/false is a combination that never makes sense.

What I meant is:

• showWindowEarly currently controls whether we show the main window early (i.e. immediately) or after the window sends the ready-to-show message
• So we can keep this behavior also for the splash screen, i.e. in case we show a splash screen, the showWindowEarly should be interpreted for the splash screen, not for the main window. So then depending on the showWindowEarly we show the splash screen before or after the ready-to-show message.

I think that makes sense as some users might dislike the blank window which is we why allow to configure showWindowEarly in the first case.

[tsmaeder]

@sdirix I would be fine to merge this one with the behavior as it is in order to have it in the release and do whatever fiddling we still want later. However, I would rather not have adopters change the configuration twice in short succession, so if you want to change the config syntax, I would rather have it in this PR. The pre-release window is closing 🤷 .

[sdirix]

I added the discussed changes and split the options again. I slightly adjusted the splash screen location determination to consider the center point of the Theia application. Before when Theia was closed with only 5 pixels on the left of two screens, the splash screen would be shown left instead of the right.

[sdirix]

I noticed a regression during testing and refactored the code. The issue was that I previously added the preventAutomaticShow to the default window options. That's fine for the first window, but not for all other windows which are created later, e.g. via the "New Window" action.

I now only add the preventAutomaticShow when we actually are currently showing (or intending to show) a splash screen. This prevents the issue.
To avoid breaking changes I also reverted the change to getLastWindowOptions which I had refactored from async to sync. However if someone actually relies on that being async for their customizations, their code would break.

I think the code is now cleaner than before. It works well for me in testing.

[tsmaeder]

@sdirix btw.: usually, we leave updates to the PR in separate commits until merge time so that reviewers don't have to review everything again when comments are being addressed.

[tsmaeder]

By accessing the splashScreenState, we're making this order-dependent. Why not use isShowSplashScreen()?

[sdirix]

isShowSplashScreen() will return true for the lifetime of the application as it's just about the configuration of a splash screen. this.splashScreenState only exist during the time we actually want to show a splash screen

[tsmaeder]

Do we really need to prevent the main window from showing as soon as it's ready? I don't see the use case and it makes the window state graph more complicated.

[tsmaeder]

Why not add an "open" call to TheiaElectronWindow and inoke it from the ElectronMainApplication class?

[sdirix]

Do we really need to prevent the main window from showing as soon as it's ready? I don't see the use case and it makes the window state graph more complicated.

The splash screen was intended as an alternative to the loading spinner of the main window. If we don't prevent the automatic show of the main window we will see the splash screen as well as the full main window at the same time.

[sdirix]

Why not add an "open" call to TheiaElectronWindow and inoke it from the ElectronMainApplication class?

Sadly, I don't understand what you mean. What is the purpose of that open and when shall it be called?

[tsmaeder]

The idea would be that the ElectronMainApplication controls visibility of the windows instead of passing flags to the window and letting the window manage it's own visiblity.

[tsmaeder]

I'm not sure this is correct: before, the main window would show when the ready-to-show even on the window would fire. applicationState === 'ready' may at a different time.

[sdirix]

This is the same behavior as in the previous state of the PR. The preventAutomaticShow was always part of the PR.

The idea of the splash screen is that the splash screen closes and the main window is shown as soon as the loading spinner is gone, i.e. when the frontend state is set to "ready". You can see the attached video in the PR that no loading spinner is visible.

[tsmaeder]

If the window is not ready and we do not have showWindowEarly we should not show the window yet, even if we're hiding the splash screen.

[sdirix]

In my opinion we should always show something. So if the splash screen is force closed via maxTime, we show the current state of the main window.

[tsmaeder]

Sorry about this being a bit late, but doing another pass got me thinking:

1. Taking showWindowEarly and splashScreenOptions apart has made things more complicated, IMO.
2. Does it even make sense to show an empty Window if we have splash screen support?
3. If we always created the initial Window early, we could immediately set up the splash-screen handling in showSplashScreen, instead of potentially adding it to every new window in configureShowOnSplashScreenClose()

[tsmaeder]

For me, the determination of the screen to show the splash screen on does not work: if I move the window to my second screen, and restart, the splash still shows on the main screen.

[sdirix]

Sorry about this being a bit late, but doing another pass got me thinking:

No worries. We should get this right.

1. Taking showWindowEarly and splashScreenOptions apart has made things more complicated, IMO.

The code is more logical in my opinion. These are now just two separate options. showWindowEarly is about whether the first rendered window (no matter whether that's the main window or the splash screen) shall be shown as early as possible, even when this means looking at a blank window.

splashScreenOptions is just for configuring optional splash screen content.

2. Does it even make sense to show an empty Window if we have splash screen support?

Yes. There are proponents for and against splash screens. We can't force everyone to use a splash screen for earliest possible visual feedback

3. If we always created the initial Window early, we could immediately set up the splash-screen handling in showSplashScreen, instead of potentially adding it to every new window in configureShowOnSplashScreenClose()

That's a good point. We could create the window early but just not show it.

The reason I added it to createWindow thinking about use cases where multiple windows might be opened while the splash screen is shown, however that might be too much of an edge case to be worth it.

[sdirix]

@tsmaeder Can you review again? Thanks!

[tsmaeder]

Is this the right event or should we just wait until the browser window has finished loading it's content?

[sdirix]

The idea is to wait until the frontend is ready to show real content. Otherwise the splash screen will just be shown less than 500ms before being replaced by the main window showing a loading spinner.

You can see this in the video: The splash screen is shown and the main window is shown without any loading spinner (as it's already gone)

[tsmaeder]

The idea would be that the ElectronMainApplication controls visibility of the windows instead of passing flags to the window and letting the window manage it's own visiblity.

[sdirix]

The idea would be that the ElectronMainApplication controls visibility of the windows instead of passing flags to the window and letting the window manage it's own visiblity.

Fine with me, I can refactor this

[sdirix]

The idea would be that the ElectronMainApplication controls visibility of the windows instead of passing flags to the window and letting the window manage it's own visiblity.

Fine with me, I can refactor this

I had a look at this and I'm not sure we're gaining much. The issue is that we can't handle the "automatic" opening within the createWindow function by itself as we're lacking context there whether we want to open the window or not. So we would either need to a new parameter to createWindow to indicate whether the window shall be automatically opened (essentially exchanging one new parameter with another) or adapt all createWindow callers. There are three callers, so that would be manageable, but all three need to be adapted then with this additional logic. Additionally createWindow is public API, so if any consumer is using that already for their use cases, we will break them.

If you still prefer that we adapt the logic of the BrowserWindow then let me know.

[sdirix]

Note that the Playwright tests are currently failing in all PRs, so this is unrelated to this PR.