You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cordova.apache.org by GitBox <gi...@apache.org> on 2020/03/10 02:57:29 UTC
[GitHub] [cordova-ios] terreng opened a new pull request #808: (iOS) Dark
mode splashscreen storyboard images
terreng opened a new pull request #808: (iOS) Dark mode splashscreen storyboard images
URL: https://github.com/apache/cordova-ios/pull/808
This is my first pr so please don't burn me at the stake.
<!--
Please make sure the checklist boxes are all checked before submitting the PR. The checklist is intended as a quick reference, for complete details please see our Contributor Guidelines:
http://cordova.apache.org/contribute/contribute_guidelines.html
Thanks!
-->
### Platforms affected
iOS
### Motivation and Context
<!-- Why is this change required? What problem does it solve? -->
<!-- If it fixes an open issue, please link to the issue here. -->
I wanted to include a dark mode variant of my app's splash screen, but found that Cordova did not yet support this feature. It turns out that it only took a few lines of code, so I figured I'd share it here.
I found this related issue: https://github.com/apache/cordova-plugin-splashscreen/issues/246. It turns out that no changes to the splashscreen plugin are necessary to make it happen.
### Description
<!-- Describe your changes in detail -->
This enables dark mode storyboard images by allowing you to specify splash images based on appearance (dark or light). It checks for ~dark or ~light suffixes in splash images. If neither is found, they're used as the default. It then adds the variants to Contents.json, and the rest is handled by iOS. Example config.xml:
`<splash src="res/screen/ios/Default@3x~iphone~comany.png" />`
`<splash src="res/screen/ios/Default@3x~iphone~comany~dark.png" />`
This pr only adds support for iOS.
### Testing
<!-- Please describe in detail how you tested your changes. -->
It works on my iPhone. I haven't run any tests, but chances are I probably didn't break anything.. right?
### Checklist
- [ ] I've run the tests to see all new and existing tests pass
- [ ] I added automated test coverage as appropriate for this change
- [x] Commit is prefixed with `(platform)` if this change only applies to one platform (e.g. `(android)`)
- [ ] If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct [keyword to close issues using keywords](https://help.github.com/articles/closing-issues-using-keywords/))
- [ ] I've updated the documentation if necessary
Documentation would need to change over at https://github.com/apache/cordova-plugin-splashscreen/blob/master/README.md.
Thank you.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For additional commands, e-mail: commits-help@cordova.apache.org
[GitHub] [cordova-ios] codecov-io commented on issue #808: (iOS) Dark mode
splashscreen storyboard images
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #808: (iOS) Dark mode splashscreen storyboard images
URL: https://github.com/apache/cordova-ios/pull/808#issuecomment-597425170
# [Codecov](https://codecov.io/gh/apache/cordova-ios/pull/808?src=pr&el=h1) Report
> Merging [#808](https://codecov.io/gh/apache/cordova-ios/pull/808?src=pr&el=desc) into [master](https://codecov.io/gh/apache/cordova-ios/commit/20248c196d35c9b8390dbbd2953f9bc1191b6c6d?src=pr&el=desc) will **increase** coverage by `0.08%`.
> The diff coverage is `100%`.
[](https://codecov.io/gh/apache/cordova-ios/pull/808?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #808 +/- ##
==========================================
+ Coverage 74.2% 74.28% +0.08%
==========================================
Files 13 13
Lines 1849 1855 +6
==========================================
+ Hits 1372 1378 +6
Misses 477 477
```
| [Impacted Files](https://codecov.io/gh/apache/cordova-ios/pull/808?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [bin/templates/scripts/cordova/lib/prepare.js](https://codecov.io/gh/apache/cordova-ios/pull/808/diff?src=pr&el=tree#diff-YmluL3RlbXBsYXRlcy9zY3JpcHRzL2NvcmRvdmEvbGliL3ByZXBhcmUuanM=) | `82.28% <100%> (+0.23%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/cordova-ios/pull/808?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/cordova-ios/pull/808?src=pr&el=footer). Last update [20248c1...c7c2b6e](https://codecov.io/gh/apache/cordova-ios/pull/808?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For additional commands, e-mail: commits-help@cordova.apache.org
[GitHub] [cordova-ios] NiklasMerz commented on issue #808: (iOS) Dark mode
splashscreen storyboard images
Posted by GitBox <gi...@apache.org>.
NiklasMerz commented on issue #808: (iOS) Dark mode splashscreen storyboard images
URL: https://github.com/apache/cordova-ios/pull/808#issuecomment-599045325
I think documentation how to use this feature should be part of this PR, too.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For additional commands, e-mail: commits-help@cordova.apache.org
[GitHub] [cordova-ios] breautek commented on issue #808: (iOS) Dark mode
splashscreen storyboard images
Posted by GitBox <gi...@apache.org>.
breautek commented on issue #808: (iOS) Dark mode splashscreen storyboard images
URL: https://github.com/apache/cordova-ios/pull/808#issuecomment-597158598
Thank you for your PR. Just a couple notes, not exactly related to this PR, but when making PRs in general.
> I haven't run any tests
You can run local tests on your machine by running `npm test`. This will help catch some problems (such as your eslint error :stuck_out_tongue_winking_eye: )
I also noticed that you made changes in your fork's master branch for your PR. I would recommend always creating a new branch for your PRs to keep your master branch clean. This isn't a problem right now, but could be later. If you add more commits to your master (maybe for a different PR) those commits will end up in this PR as well and that would not be very desirable.
Regarding this PR specifically, I think everything looks good and I'm happy you considered making specifying the appearance optional, which also helps maintain backwards compatibility. As soon as the checks turn green I'll give my :+1:
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For additional commands, e-mail: commits-help@cordova.apache.org
[GitHub] [cordova-ios] terreng commented on issue #808: (iOS) Dark mode
splashscreen storyboard images
Posted by GitBox <gi...@apache.org>.
terreng commented on issue #808: (iOS) Dark mode splashscreen storyboard images
URL: https://github.com/apache/cordova-ios/pull/808#issuecomment-597414858
Alright, I've updated the failed tests and added three new ones.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For additional commands, e-mail: commits-help@cordova.apache.org
[GitHub] [cordova-ios] NiklasMerz commented on issue #808: (iOS) Dark mode
splashscreen storyboard images
Posted by GitBox <gi...@apache.org>.
NiklasMerz commented on issue #808: (iOS) Dark mode splashscreen storyboard images
URL: https://github.com/apache/cordova-ios/pull/808#issuecomment-599120440
Good point. I got confused that the code is part of cordova-ios and not the plugin.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For additional commands, e-mail: commits-help@cordova.apache.org
[GitHub] [cordova-ios] terreng commented on issue #808: (iOS) Dark mode
splashscreen storyboard images
Posted by GitBox <gi...@apache.org>.
terreng commented on issue #808: (iOS) Dark mode splashscreen storyboard images
URL: https://github.com/apache/cordova-ios/pull/808#issuecomment-599075277
Documentation would have to change for cordova-plugin-splashscreen which isn't a part of this repo.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For additional commands, e-mail: commits-help@cordova.apache.org
[GitHub] [cordova-ios] NiklasMerz commented on issue #808: (iOS) Dark mode
splashscreen storyboard images
Posted by GitBox <gi...@apache.org>.
NiklasMerz commented on issue #808: (iOS) Dark mode splashscreen storyboard images
URL: https://github.com/apache/cordova-ios/pull/808#issuecomment-596955263
@terreng Thank you for your first PR 🥇 This is a great feature. Please check the Travis log, if you can turn the tests green, review is much easier.
Looks like some tests need to be changed and/or new tests are needed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For additional commands, e-mail: commits-help@cordova.apache.org