You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by spiritax <gi...@git.apache.org> on 2015/08/07 15:36:10 UTC

[GitHub] cordova-plugin-splashscreen pull request: iOS : Fix for screen not...

GitHub user spiritax opened a pull request:

    https://github.com/apache/cordova-plugin-splashscreen/pull/56

    iOS : Fix for screen not fading

    Fix for screen not fading in iOS.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/spiritax/cordova-plugin-splashscreen ios-FixFade

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/cordova-plugin-splashscreen/pull/56.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #56
    
----
commit f321ca63b72c19b808688c3f727611cd938b277e
Author: Glibert Axel <ax...@neopixl.com>
Date:   2015-08-07T10:29:16Z

    Fix for screen not fading

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-splashscreen pull request: iOS : Fix for screen not...

Posted by purplecabbage <gi...@git.apache.org>.
Github user purplecabbage commented on the pull request:

    https://github.com/apache/cordova-plugin-splashscreen/pull/56#issuecomment-131936309
  
    Ignoring whitespace changes, this 'fix' is simply just removing the `dispatch_async(dispatch_get_main_queue(), ^{` blocks.
    Can you elaborate on why this is required?



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-splashscreen pull request: iOS : Fix for screen not...

Posted by chaffeqa <gi...@git.apache.org>.
Github user chaffeqa commented on the pull request:

    https://github.com/apache/cordova-plugin-splashscreen/pull/56#issuecomment-145501931
  
    any decision on this? would be a real plus to have back in the plugin!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-splashscreen pull request: iOS : Fix for screen not...

Posted by MTShare <gi...@git.apache.org>.
Github user MTShare commented on the pull request:

    https://github.com/apache/cordova-plugin-splashscreen/pull/56#issuecomment-144942116
  
    +1


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-splashscreen pull request: iOS : Fix for screen not...

Posted by idpaterson <gi...@git.apache.org>.
Github user idpaterson commented on the pull request:

    https://github.com/apache/cordova-plugin-splashscreen/pull/56#issuecomment-145523163
  
    [The above code](#issuecomment-132235333) has been working for us in production, I can submit a pull request but I'm not quite clear enough on the scope of the original UI-off-the-main-thread issue in order to properly justify in JIRA that the change does not introduce regression issues. I'll try my best.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-splashscreen pull request: iOS : Fix for screen not...

Posted by idpaterson <gi...@git.apache.org>.
Github user idpaterson commented on the pull request:

    https://github.com/apache/cordova-plugin-splashscreen/pull/56#issuecomment-132235333
  
    Since the animation block must perform its changes synchronously in order to animate, adding `__block` as follows does not resolve the issue; the splash screen still does not fade.
    ```obj-c
          [UIView transitionWithView:self.viewController.view
                            duration:fadeDuration
                             options:UIViewAnimationOptionTransitionNone
                          animations:^(void) {
                              __block __typeof(self) strongSelf = weakSelf;
                              if (strongSelf != nil) {
                                  dispatch_async(dispatch_get_main_queue(), ^{
                                          [strongSelf->_activityView setAlpha:0];
                                          [strongSelf->_imageView setAlpha:0];
                                  });
                              }
                          }
                          completion:^(BOOL finished) {
                              if (finished) {
                                  dispatch_async(dispatch_get_main_queue(), ^{
                                          [weakSelf destroyViews];
                                  });
                              }
                          }
            ];
    ```
    
    I believe that the most comprehensive solution to ensure that fading works and avoid threading issues is something like the following. I am not sure how or why `setVisible:` is being called on a background thread, but this would ensure that any resulting UI modifications are performed on the main thread.
    ```obj-c
    - (void)setVisible:(BOOL)visible
    {
        if (visible == _visible) {
            return;
        }
        _visible = visible;
    
        id fadeSplashScreenValue = [self.commandDelegate.settings objectForKey:[@"FadeSplashScreen" lowercaseString]];
        id fadeSplashScreenDuration = [self.commandDelegate.settings objectForKey:[@"FadeSplashScreenDuration" lowercaseString]];
    
        float fadeDuration = fadeSplashScreenDuration == nil ? kSplashScreenDurationDefault : [fadeSplashScreenDuration floatValue];
    
        if ((fadeSplashScreenValue == nil) || ![fadeSplashScreenValue boolValue]) {
            fadeDuration = 0;
        }
    
        [self setVisible:visible fadeDuration:fadeDuration];
    }
    
    - (void)setVisible:(BOOL)visible fadeDuration:(float)fadeDuration 
    {
        if ([NSThread isMainThread]) {
            // Never animate the showing of the splash screen.
            if (visible) {
                if (_imageView == nil) {
                    [self createViews];
                }
            } else if (fadeDuration == 0) {
                [self destroyViews];
            } else {
              __weak __typeof(self) weakSelf = self;
    
              [UIView transitionWithView:self.viewController.view
                                duration:fadeDuration
                                 options:UIViewAnimationOptionTransitionNone
                              animations:^(void) {
                                  __block __typeof(self) strongSelf = weakSelf;
                                  if (strongSelf != nil) {
                                      [strongSelf->_activityView setAlpha:0];
                                      [strongSelf->_imageView setAlpha:0];
                                  }
                              }
                              completion:^(BOOL finished) {
                                  if (finished) {
                                      [weakSelf destroyViews];
                                  }
                              }
                ];
            }
        }
        else {
            dispatch_async(dispatch_get_main_queue(), ^{
                [self setVisible:visible fadeDuration:fadeDuration];
            });
        }
    }
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-splashscreen pull request: iOS : Fix for screen not...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/cordova-plugin-splashscreen/pull/56


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-splashscreen pull request: iOS : Fix for screen not...

Posted by idpaterson <gi...@git.apache.org>.
Github user idpaterson commented on the pull request:

    https://github.com/apache/cordova-plugin-splashscreen/pull/56#issuecomment-131946308
  
    The `dispatch_async` calls were added [here](https://github.com/apache/cordova-plugin-splashscreen/commit/559b300d29ba6eb2f951eb19b9bbf7ba9238a862) and reports on the commit of fading no longer working were disregarded. Further discussion on [the original issue](https://issues.apache.org/jira/browse/CB-8836) noted that the `dispatch_async` calls did not fully resolve whatever crash was being experienced.
    
    Certainly removing these calls outright is not a complete solution since it disregards the earlier problem of crashes due to operating off the main thread, but this pull request thus far identifies that changes in the animation block must be synchronous in order to actually animate.
    
    The related issue in Jira for fading no longer working on iOS is [CB-8875](https://issues.apache.org/jira/browse/CB-8875) for which there is an outstanding pull request #44 by @chrskrchr. This is a better but possibly not a complete solution either since the calls to `destroyViews` and `createViews` should probably also only be called on the main thread. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-splashscreen pull request: iOS : Fix for screen not...

Posted by danieldocki <gi...@git.apache.org>.
Github user danieldocki commented on the pull request:

    https://github.com/apache/cordova-plugin-splashscreen/pull/56#issuecomment-131145262
  
    +1


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-splashscreen pull request: iOS : Fix for screen not...

Posted by idpaterson <gi...@git.apache.org>.
Github user idpaterson commented on the pull request:

    https://github.com/apache/cordova-plugin-splashscreen/pull/56#issuecomment-131889236
  
    Has this been submitted as an issue in [Jira](https://issues.apache.org/jira/secure/Dashboard.jspa) to make sure it is assigned to someone? The fix works for me. 
    
    Combined with the fact that the SplashScreenDelay preference is not used in any way in the iOS code, this plugin currently does absolutely nothing on iOS unless the show and hide methods are called explicitly by the app...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org


[GitHub] cordova-plugin-splashscreen pull request: iOS : Fix for screen not...

Posted by purplecabbage <gi...@git.apache.org>.
Github user purplecabbage commented on the pull request:

    https://github.com/apache/cordova-plugin-splashscreen/pull/56#issuecomment-131980497
  
    Isn't using __block to define __typeof(self) strongSelf = weakSelf; the more correct solution here?
    The issue seems to be that once dispatch_async happens strongSelf may no longer exist.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org