Skip to content
This repository has been archived by the owner on Mar 5, 2023. It is now read-only.

viewWillAppear called twice #32

Closed
swt2c opened this issue Dec 15, 2014 · 9 comments · Fixed by #34
Closed

viewWillAppear called twice #32

swt2c opened this issue Dec 15, 2014 · 9 comments · Fixed by #34

Comments

@swt2c
Copy link
Contributor

swt2c commented Dec 15, 2014

In setSelectedViewController, it seems that viewWillAppear ends up getting called twice. It is called explicitly once, but calling setContentView on the window also seems to cause it to be called.

if ([controller respondsToSelector:@selector(viewWillAppear)])
    [controller viewWillAppear];

[self.window setContentView:controllerView];

Is the explicit call needed?

@sorbits
Copy link
Collaborator

sorbits commented Dec 15, 2014

The explicit call is required since viewWillAppear was not a system
method prior to 10.10.

It could be updated to skip the call when on Yosemite.

On 15 Dec 2014, at 22:17, Scott Talbert wrote:

In setSelectedViewController, it seems that viewWillAppear ends up
getting called twice. It is called explicitly once, but calling
setContentView on the window also seems to cause it to be called.

if ([controller respondsToSelector:@selector(viewWillAppear)])
[controller viewWillAppear];

[self.window setContentView:controllerView];

Is the explicit call needed?


Reply to this email directly or view it on GitHub:
#32

@swt2c
Copy link
Contributor Author

swt2c commented Dec 16, 2014

Ah, so setContentView only causes viewWillAppear on Yosemite?

What is the preferred way to check the OSX version? NSAppKitVersionNumber?

@shpakovski
Copy link
Collaborator

Hi guys, how about [UIViewController instancesRespondToSelector:@selector(viewWillAppear)]?

@sorbits
Copy link
Collaborator

sorbits commented Dec 16, 2014

Yes, only on Yosemite, the documentation says it’s called when:

  • The view is about to be added to the view hierarchy of the view
    controller
  • The view controller’s window is about to become visible, such as
    coming to the front or becoming unhidden

I would go with @shpakovski suggestion of instancesRespondsToSelector:
(though sent to NSViewController) — in theory this method could exist
prior to 10.10 with different semantics, but testing against
NSAppKitVersionNumber requires optionally defining version number
constants so that it works when building with the 10.9 SDK or earlier.

On 16 Dec 2014, at 22:51, Scott Talbert wrote:

Ah, so setContentView only causes viewWillAppear on Yosemite?

What is the preferred way to check the OSX version?
NSAppKitVersionNumber?


Reply to this email directly or view it on GitHub:
#32 (comment)

@swt2c
Copy link
Contributor Author

swt2c commented Dec 22, 2014

So we'd want something like this?

    if (![NSViewController instancesRespondToSelector:@selector(viewWillAppear)])
        if ([controller respondsToSelector:@selector(viewWillAppear)])
            [controller viewWillAppear];

@shpakovski
Copy link
Collaborator

Yes, but it would be great to test on earlier OS X versions before shipping :) Thanks in advance for the pull request!

@sorbits
Copy link
Collaborator

sorbits commented Dec 22, 2014

Would also be good to add a comment about the code, as I’m sure
someone will wonder why we call a method on an object only if that
object’s superclass does not respond to said method.

On 22 Dec 2014, at 13:48, Vadim Shpakovski wrote:

Yes, but it would be great to test on earlier OS X versions before
shipping :) Thanks in advance for the pull request!


Reply to this email directly or view it on GitHub:
#32 (comment)

@fingolfin
Copy link
Contributor

I just tested on 10.8.5, and everything seems to work fine with the change proposed by @swt2c -- but I also agree with @sorbits that this is the kind of change that really would benefit from a short source comment explaining what it is about (and perhaps also pointing to this issue).

@shpakovski
Copy link
Collaborator

I believe the comment with URL to the current page would work best :)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants