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(splashscreen): Added backgroundColor to Splash Screen plugin. #1649

Merged
merged 11 commits into from
Jun 20, 2019

Conversation

trancee
Copy link
Contributor

@trancee trancee commented Jun 8, 2019

In cases where you use a different scale type, e.g. CENTER_INSIDE, it is possible that the splash image is smaller than the view itself. The missing parts around the splash image will then be transparent and showing the background of the app. You can now override this by specifying a background color (similar to the cordova splash screen plugin). The value is a simple #RGB or #ARGB value.

@mlynch Thank you for your feedback. I am not sure if this config key makes sense on iOS, as it handles splash screens differently. It is necessary on Android because you can change the scale type and it may happen that not the full screen is covered by the splash screen. In those cases, it requires the background color to cover those areas or they will be transparent and showing off the web app which looks ugly.

Basically, I am just following the practice from the Cordova splash screen plugin.

@trancee
Copy link
Contributor Author

trancee commented Jun 8, 2019

@mlynch I have managed to add backgroundColor support for iOS. I renamed the configuration key to backgroundColor instead and it accepts either Hex RGB or Hex ARGB value.

@trancee trancee changed the title Added androidBackgroundColor to Splash Screen plugin. Added backgroundColor to Splash Screen plugin. Jun 8, 2019
showSplash(showDuration: showDuration, fadeInDuration: fadeInDuration, fadeOutDuration: fadeOutDuration, autoHide: autoHide, completion: {
call.success()
}, isLaunchSplash: false)
let backgroundColor = call.get("backgroundColor", String.self)
Copy link
Member

Choose a reason for hiding this comment

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

It should be like this:

Suggested change
let backgroundColor = call.get("backgroundColor", String.self)
let backgroundColor = getConfigValue("backgroundColor") as? String ?? nil

call.get is to get the value from the plugin call, not from the capacitor.config.json

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I have changed it according to your suggestion.

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.

Added a comment.
Also you should rebase with master as your other PR for the spinner created a lot of conflicts.

@jcesarmobile
Copy link
Member

Something went wrong with the rebase, looks like you didn't pull latest changes.

@trancee
Copy link
Contributor Author

trancee commented Jun 20, 2019

Something went wrong with the rebase, looks like you didn't pull latest changes.

Maybe I forgot to include all of the files. I am doing that now. Hopefully that works.

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.

Looks good to me

@jcesarmobile jcesarmobile changed the title Added backgroundColor to Splash Screen plugin. feat(splashscreen): Added backgroundColor to Splash Screen plugin. Jun 20, 2019
@jcesarmobile jcesarmobile merged commit 45db73d into ionic-team:master Jun 20, 2019
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.

2 participants