Skip to content
This repository has been archived by the owner on Feb 19, 2020. It is now read-only.

Fix additional UINavigationControllers #404

Merged

Conversation

dtweston
Copy link
Contributor

We recently wanted to use the feedback features of the iOS SDK, but noticed a couple of visual bugs in how it was working. We need the status bar to use the .LightContent style, which is configured by the UINavigationBar.barStyle

I'm concerned that the second commit in this series may cause regressions for customers who are setting the modal presentation style. Do you have any sense of how much this gets used?

Also, for the third commit, more could be done to set up the UIImagePickerController, but this is all we needed so I didn't do the additional work to test other scenarios.

Call into BITHockeyBaseManager for a UINavigationController to
match other places in the code base.
This parameter was never used, instead using the property defined on
self. However, everywhere it was passed in as .FormSheet, which is
what the local property defaulted to.
UIImagePickerController inherits from UINavigationController, so
we should set the barStyle with the same default we use when creating
a UINavigationController.
@msftclas
Copy link

@dtweston,
Thanks for your contribution as a Microsoft full-time employee or intern. You do not need to sign a CLA.
Thanks,
Microsoft Pull Request Bot

@ElektrojungeAtWork
Copy link
Contributor

Hey Dave,

thanks for the contribution. I'm swamped with work this week, I'll look at this and test next week and let you know what we want to do with this.

🖖
Benny

@dtweston
Copy link
Contributor Author

Thanks for the quick response, and sorry to just drop the PR in your lap like that without a corresponding issue. Let me know if you need/want anything more from me.

@ElektrojungeAtWork
Copy link
Contributor

Hey Dave,
I finally had a chance to look and do some testing.
Commits 1 and 3 look good.
About the 2nd one.
I don't have any numbers on how much this get's used and couldn't repro any issue here with your change here, but might not I have tested all cases.

@dtweston
Copy link
Contributor Author

Yeah, if people are setting the BITHockeyBaseManager.modalPresentationStyle parameter to something other than UIModalPresentationStyleFormSheet, they're going to see a change in behavior. It might be worth calling it out in the release notes, or something like that?

I don't know the history of how this property came about, but if you can figure that out from your end, it may point you to companies who are actually using this parameter.

@ElektrojungeAtWork
Copy link
Contributor

ElektrojungeAtWork commented Mar 31, 2017

Merging this now as I couldn't repro any issue with this.

@ElektrojungeAtWork ElektrojungeAtWork merged commit f234ec1 into bitstadium:develop Mar 31, 2017
@dtweston dtweston deleted the fix_additional_nav_controllers branch April 1, 2017 00:36
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 this pull request may close these issues.

3 participants