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

feat(ios): improve webview background experience #2933

Merged
merged 12 commits into from
Jul 13, 2020

Conversation

mhartington
Copy link
Contributor

Based on a few different posts from the forum, the configuration for how the webview handles backgrounds can lead to a flash of white when starting up, especially in dark mode.

This PR attempts to improve this by first

  • Setting the webview to not be isOpaque
  • if you're on iOS 13, use the system background color for the webview
  • if you're not on iOS 13, use the color declared in the capacitor config.

There could be room for further improvements, but I think this is a good first step.

@imhoffd
Copy link
Contributor

imhoffd commented Jun 30, 2020

@mhartington

@mhartington mhartington force-pushed the webview-background-enhancments branch from 3bcb502 to 50a5040 Compare July 2, 2020 17:15
@ikeith
Copy link
Contributor

ikeith commented Jul 2, 2020

This looks good to me but I will leave it for @jcesarmobile to review.

This also updates the default launch storyboard template to set a background color on the image view. That was not set before which means that any transparency in a splash image would fall through to an undefined color and render as black.

Copy link
Member

@jcesarmobile jcesarmobile left a comment

Choose a reason for hiding this comment

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

I've added a minor recommended change since we use 2 spaces instead of 4.

I think changing the opaque value should be behind a preference, since it can mess with a few plugins that set opaque to false on their initialization and this change will set it to true when the page finish loading, breaking their functionality.
And also because this change is not needed for everybody, the flash is noticeable when the splash plugin is not used (if using "launchShowDuration": 0), if the splash plugin is used and hidden when the app content is loaded, there is no flash.

And for the storyboard changes, they are not needed neither, it works fine if using transparent background images, but I think we should encourage users to use dedicated light/dark splash images without transparent backgrounds. But since it's a template change and will only affect new apps and only if they use transparent background images, I don't mind if we add it.

ios/Capacitor/Capacitor/CAPBridgeViewController.swift Outdated Show resolved Hide resolved
@ikeith
Copy link
Contributor

ikeith commented Jul 10, 2020

@jcesarmobile I'm not sure a setting is the right solution. Would the setting be to make the web view non-opaque on initial load (and then set it back), or would the default behavior be to make it non-opaque and the setting controls whether or not it gets set back? If it's the first, then you wouldn't be able to set it AND use one of the plugins that expects the change to persist. If it's the second, it's potentially a breaking change for anyone using one of those plugins now.

So, I don't think making the opacity flag a setting is what's needed and there's an alternate approach. Here's a timeline of the app starting up:

  1. Native app initialization
  2. Webview creation
  3. Bridge initialization
  4. Initial web content load starts
  5. Initial web content load finishes

This change to isOpaque happens in steps 4 and 5. But if a plugin changes that value in its load method, that happens in step 3 as it is registered.

Instead of just overwriting isOpaque, we can save the state of isOpaque in loadWebView, set it be false, and then set it to the saved value after the load. With an additional guard to only do that once for the initial load.

That should prevent any conflicts with plugins and any apps that keep the splash screen up until step 5 won't notice.

I would argue that the real issue is that plugins should not be modifying a global property like the webview's isOpaque flag (that should be an application level change). But that's a totally separate discussion and it's probably too late to enforce it as a policy.

ikeith and others added 5 commits July 10, 2020 15:23
…al load and preserve the value in case it was modified by a plugin.
Co-authored-by: jcesarmobile <jcesarmobile@gmail.com>
…m/capacitor into webview-background-enhancments
@ikeith ikeith merged commit 49720a5 into master Jul 13, 2020
@jcesarmobile jcesarmobile deleted the webview-background-enhancments branch July 13, 2020 18:33
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.

4 participants