Skip to content

Conversation

@loic-sharma
Copy link
Member

@loic-sharma loic-sharma commented Aug 18, 2022

Currently the Windows app is shown immediately with no content and a white background until the first frame is drawn. This change hides the app and makes it visible after that first frame has been drawn.

Part of #41980

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@flutter-dashboard flutter-dashboard bot added c: contributor-productivity Team-specific productivity, code health, technical debt. tool Affects the "flutter" command-line tool. See also t: labels. labels Aug 18, 2022
@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

Copy link
Member

@cbracken cbracken left a comment

Choose a reason for hiding this comment

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

LGTM stamp from a Japanese personal seal

@cbracken
Copy link
Member

You'll need to request a test exemption from @Hixie.

This code is integration-tested by the devicelab testing. You'll want to sync to tip-of tree and apply this change to the hello_world and platform_view samples that @yaakovschectman recently added Windows devicelab testing for in #109618.

Copy link
Contributor

@yaakovschectman yaakovschectman left a comment

Choose a reason for hiding this comment

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

LGTM

@loic-sharma
Copy link
Member Author

I migrated the new Windows app entry points!

@flutter-dashboard flutter-dashboard bot added the d: examples Sample code and demos label Aug 19, 2022
@Hixie
Copy link
Contributor

Hixie commented Aug 19, 2022

Can you elaborate on how this is tested?

@jonbhanson
Copy link

@loic-sharma @cbracken in iOS, Android, and Web, my package flutter_native_splash allows devs to display a splash screen before the first frame has been drawn. This is preferable to hiding the app because it gives the user immediate feedback that the app is launching and gives a better impression of device responsiveness.

In your development of the Windows Flutter app architecture, can you include the ability for adding a native splash screen that can be displayed before the first frame is drawn?

@cbracken
Copy link
Member

cbracken commented Aug 22, 2022

@jonbhanson thanks for your comment.

In your development of the Windows Flutter app architecture, can you include the ability for adding a native splash screen that can be displayed before the first frame is drawn?

In the default app template, we use this to hide the main window for a few additional (and almost certainly unnoticeable) milliseconds until the first frame is ready in order to prevent the user seeing a very short flash of an unrendered view. A splash screen implementor would be able to use this callback to determine when the app is ready to display, which they can use as one input when determining when they'd like to take down the splash screen.

A splash screen isn't particularly useful for covering up delays measured in milliseconds, hence why it's not default, but:

  • The frame callback is something that is potentially useful for splashscreen implementors (e.g. for apps that have long load times for one reason or another).
  • Users of splash screens typically want the main window hidden while the splash screen is up, regardless
  • For add-to-app scenarios, or other scenarios where Flutter owns the view, but doesn't own the window, the callback gives the app author a handy way to know when to add the flutter view to their app (or display some other useful signal to the user).

Please shout if any of these patches causes problems for your package. There's a separate bug (#41980) open for splash screen support.

@loic-sharma loic-sharma requested a review from keyonghan as a code owner August 22, 2022 22:56
@loic-sharma loic-sharma marked this pull request as draft August 22, 2022 22:57
loic-sharma added a commit that referenced this pull request Aug 26, 2022
Add a basic integration test for the Windows's app startup. In a subsequent change (#109816), this test will be updated to verify the app's window isn't visible until after the first frame has been drawn.

Part of #41980
@loic-sharma loic-sharma marked this pull request as ready for review August 26, 2022 16:58
@loic-sharma
Copy link
Member Author

loic-sharma commented Aug 26, 2022

I rebased this off of #110114 and added an integration test to verify the app starts with a hidden window that becomes visible after the first frame.

@cbracken and @yaakovschectman, I've dismissed your previous approvals given the changes were substantial. Please take another look when available! 😄

Copy link
Contributor

@yaakovschectman yaakovschectman left a comment

Choose a reason for hiding this comment

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

Still looks okay

@loic-sharma loic-sharma added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 30, 2022
@auto-submit auto-submit bot merged commit c60cf97 into flutter:master Aug 30, 2022
@loic-sharma loic-sharma deleted the windows_hide_launch branch August 30, 2022 14:26
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 30, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Aug 30, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 30, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Sep 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App c: contributor-productivity Team-specific productivity, code health, technical debt. d: examples Sample code and demos tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants