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

[improv]: electron side changes to window behavior #3300

Merged
merged 3 commits into from
Dec 9, 2023

Conversation

imLinguin
Copy link
Member

window now has a background which should prevent flashing of white,
window is shown after its contents are loaded - removed our custom loadingScreenReady event

additionally I replaced deprecated protocol.registerStringProtocol in favor of protocol.handle

Use the following Checklist if you have changed something on the Backend or Frontend:

  • Tested the feature and it's working on a current and clean install.
  • Tested the main App features and they are still working on a current and clean install. (Login, Install, Play, Uninstall, Move games, etc.)
  • Created / Updated Tests (If necessary)
  • Created / Updated documentation (If necessary)

window now has a background which should prevent flashing of white, window is shown after its contents are loaded
@imLinguin imLinguin added the pr:ready-for-review Feature-complete, ready for the grind! :P label Dec 7, 2023
@imLinguin imLinguin requested review from a team, arielj, flavioislima, CommandMC, Etaash-mathamsetty and Nocccer and removed request for a team December 7, 2023 13:01
@arielj
Copy link
Collaborator

arielj commented Dec 8, 2023

I think we need to find a different solution because the problem seems to be that the show: false option we use when we initialize the window is not really working for some reason, because the our code is saying "start hidden, and only show the window once the frontend tells you the loading screen of heroic is displayed" but then the window is displayed way earlier than that.

This pr is kinda working around that issue but not fully solving it, that hardcoded color is not great (cause now the page wont blink, but it will start black and then change to another color).

I played around and seems like the issue is this call to maximize() https://github.com/Heroic-Games-Launcher/HeroicGamesLauncher/blob/main/src/backend/main_window.ts#L89

We should wait until the application is ready and the window is displayed to decide if it should be maximized instead. Because I also noticed the blinking doesn't seem to happen if the app is not maximized when closed.

@arielj
Copy link
Collaborator

arielj commented Dec 8, 2023

related to the removal of the loadingScreenReady event, maybe it can be replaced with https://www.electronjs.org/docs/latest/api/browser-window#event-ready-to-show instead of did-finish-load?

I'm not sure about that change though, the different between the custom event we have and the electron events is that ours should wait until the loading screen is ready, and the electron one doesn't know about the loading screen and it can show the app before that (without a blink, but also without the loading screen animation)

@imLinguin
Copy link
Member Author

Indeed, maximize causes the window to show. Will fix that...

I'm not sure about that change though

I think it should be fine, the loading indicator is the first thing that displays on the page after being "loaded".

Copy link
Collaborator

@arielj arielj left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@imLinguin imLinguin merged commit f2911ee into main Dec 9, 2023
14 checks passed
@imLinguin imLinguin deleted the improv/mainwindow-blink branch December 9, 2023 14:57
@Heroic-Games-Launcher Heroic-Games-Launcher locked and limited conversation to collaborators Dec 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pr:ready-for-review Feature-complete, ready for the grind! :P
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants