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

More information for UIViewController breadcrumbs #1727

Closed
philipphofmann opened this issue Mar 28, 2022 · 7 comments · Fixed by #1945
Closed

More information for UIViewController breadcrumbs #1727

philipphofmann opened this issue Mar 28, 2022 · 7 comments · Fixed by #1945

Comments

@philipphofmann
Copy link
Member

Problem Statement

Right now a breadcrumb for an UIViewController.viewDidAppear only contains the name of the UIViewController. We could add more information.
Screen Shot 2022-03-28 at 17 33 19

Solution Brainstorm

Add more information like maybe accessabilityIdentifier as added with #1724. @brustolin can you maybe add some ideas?

@brustolin
Copy link
Contributor

More ideas:

  • Title

  • Parent ViewController

  • Presentation mode

@armcknight
Copy link
Member

armcknight commented Jul 7, 2022

More ideas:

  • presentingViewController (class name and instance address)
  • navigationController/splitViewController/tabBarController (class name and instance address)
  • view.window (class name and instance address)
    • whether it is this window's rootViewController
    • whether this window is the key window
    • the window's screen and/or windowScene
    • the windowLevel
  • storyboard
  • nib name/bundle
  • transitioningDelegate (class name and instance address)

Also, it's possible that a view controller's title contains PII.

@kevinrenskers
Copy link
Contributor

I think navigationController / splitViewController / tabBarController are not super useful as normally it'll just be UINavigationController / UISplitViewController / UITabbarController (not a custom subclass), so you just get an instance address which doesn't seem super useful?

Same goes for the storyboard: I can't get the storyboard name, so you only end up with the instance address.

@kevinrenskers
Copy link
Contributor

@armcknight can you add some context as to why you think things like the window and the storyboard or nib name are useful? For the storyboard we can't get the name for example, and in any case as a developer you'd know from which storyboard or nib your view controller instance would come from, right?

And as for the window, we're wondering in which scenarios this info would be useful. Can you explain your ideas? Thanks!

About the title containing PII: we have the beforeBreadcrumb callback so that developers can clean this up if they need to. We think that's enough, and better than not including the titles which in 99% of cases wouldn't include PII. What do you think?

@armcknight
Copy link
Member

Nice, didn't know about the beforeBreadcrumb callback. That should be fine I think.

As for all the navigation related stuff, I was just thinking it could be helpful to have more context around how a user arrived at a particular VC, but I agree that just showing hex addresses of nav controllers won't be very helpful; I was thinking about a larger feature there, which I'll write a separate proposal for.

As for the window context, like which window a vc belongs to, if that's the key window, or if a VC is the root of a window, that could help provide info on if this is the first VC that was displayed after launch, or if it's visible on screen at all. That can give some context as to how important a VC is; the first one displayed after launch is much more important than one that's rendering something off screen in an invisible window; and something rendering off-screen can potentially be kicked further down the QoS ladder to a background thread if it's adversely affecting performance.

I edited my last comment to add is screen the UIScreen.mainScreen and/or windowScene (both of those have headerdocs mentioning performance intensivity) and windowLevel to the UIWindow list.

I agree that's some niche stuff but could potentially be helpful, I'd be interested to hear feedback on that from someone with a complex app architecture.

@philipphofmann
Copy link
Member Author

Thanks for the update, @armcknight. I would like to point out that we can also iterate on this feature. We can add some simple information first and open issue(s) for information that requires more work.

@armcknight
Copy link
Member

Totally, I was just dumping some brainstorm here 👍🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants