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

[iOS] Tab children should be cast to Page #398

Merged
merged 4 commits into from Oct 3, 2016

Conversation

Projects
None yet
4 participants
@adrianknight89
Contributor

adrianknight89 commented Sep 30, 2016

Description of Change

This fixes an internal bug and is also a discussion.

I was looking at https://bugzilla.xamarin.com/show_bug.cgi?id=34553 and https://bugzilla.xamarin.com/show_bug.cgi?id=26228.

Just wondering, why did you decide to hide Edit button by default? Since the bugs were reported for 1.3.1, the button was enabled back then. Disabling it should have been a breaking change. If we enable it now, it's another breaking change. Why hide the default behavior?

This PR should still be merged because I fixed a casting error. If you comment

//disable edit/reorder of tabs
CustomizableViewControllers = null;

or reinitialize the array in a custom renderer, then Edit should appear and reordering elements and navigating to them should work without an overflow exception.

@dnfclas

This comment has been minimized.

Show comment
Hide comment
@dnfclas

dnfclas Sep 30, 2016

Hi @adrianknight89, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by .NET Foundation and real humans are currently evaluating your PR.

TTYL, DNFBOT;

dnfclas commented Sep 30, 2016

Hi @adrianknight89, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by .NET Foundation and real humans are currently evaluating your PR.

TTYL, DNFBOT;

@rmarinho

This comment has been minimized.

Show comment
Hide comment
@rmarinho

rmarinho Oct 3, 2016

Member

If i m not mistaken we have some issues when reorder and using NavigationPages, so it's unsupported scenario, we can move this to a virtual method, to be easier to override, something like ConfigureTabVie, i m doing something similar in MacOS, and since we introduce a virtual for CreateNativeControl we could do one for ConfigureNativeControl for these kind of settings.. This could also be a platform specific feature .

about this failure this seems to be a issue in refactoring, good catch, this code wasn't being called but it would crash.

Thanks.

Member

rmarinho commented Oct 3, 2016

If i m not mistaken we have some issues when reorder and using NavigationPages, so it's unsupported scenario, we can move this to a virtual method, to be easier to override, something like ConfigureTabVie, i m doing something similar in MacOS, and since we introduce a virtual for CreateNativeControl we could do one for ConfigureNativeControl for these kind of settings.. This could also be a platform specific feature .

about this failure this seems to be a issue in refactoring, good catch, this code wasn't being called but it would crash.

Thanks.

@rmarinho rmarinho merged commit 5372d21 into xamarin:master Oct 3, 2016

0 of 3 checks passed

Android-UITests Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle8 :: UI Tests :: OSX Test Cloud Package - Run Android 6.0.1 : Tests failed: 1, pass…
Details
iOS10-UITests Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle8 :: UI Tests :: OSX Test Cloud Package - Run iOS Unified iOS10 : Tests failed: 5, …
Details
iOS9-UITests Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle8 :: UI Tests :: OSX Test Cloud Package - Run iOS Unified iOS9 : Tests failed: 1, p…
Details
@adrianknight89

This comment has been minimized.

Show comment
Hide comment
@adrianknight89

adrianknight89 Oct 10, 2016

Contributor

@rmarinho How would ConfigureNativeControl be any different from a platform specific feature? Aren't they the same things?

Contributor

adrianknight89 commented Oct 10, 2016

@rmarinho How would ConfigureNativeControl be any different from a platform specific feature? Aren't they the same things?

@adrianknight89 adrianknight89 referenced this pull request Oct 10, 2016

Closed

[iOS] More button on TabbedPage should work #440

2 of 4 tasks complete
@rmarinho

This comment has been minimized.

Show comment
Hide comment
@rmarinho

rmarinho Oct 10, 2016

Member

If you want to just inherit from our renderers, just more options.

Member

rmarinho commented Oct 10, 2016

If you want to just inherit from our renderers, just more options.

@samhouts samhouts added this to the 2.3.4 milestone Jun 27, 2018

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