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

Fixed tab not updating when page title is changed #327

Merged
merged 3 commits into from Nov 2, 2016

Conversation

Projects
None yet
7 participants
@jimmgarrido
Collaborator

jimmgarrido commented Aug 28, 2016

Description of Change

With AppCompat, changing the Title of a TabbedPage child Page would no longer update its tab text. This fixes that by wiring up to each child page's PropertyChanged event using the same mechanism as in the iOS TabbedRenderer in order to respond to the change. This also sets up the ability to respond to other child page property changes later if needed.

Bugs Fixed

API Changes

None

Behavioral Changes

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Rebased on top of master at time of PR
  • Changes adhere to coding standard
  • Consolidate commits as makes sense
@dnfclas

This comment has been minimized.

Show comment
Hide comment
@dnfclas

dnfclas Aug 28, 2016

@jimmgarrido, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, DNFBOT;

dnfclas commented Aug 28, 2016

@jimmgarrido, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, DNFBOT;

@dnfclas dnfclas added cla-signed and removed cla-required labels Aug 28, 2016

if (e.PropertyName == Page.TitleProperty.PropertyName)
{
var page = (Page)sender;
var index = Element.Children.IndexOf(page);

This comment has been minimized.

@lirkki

lirkki Aug 30, 2016

Contributor

@jimmgarrido While verifying bug https://bugzilla.xamarin.com/show_bug.cgi?id=43734 using this commit, I met a regression crash when disposing my TabbedPage. The crash occured because OnPagePropertyChanged was called and Element was null. In my work area I made a quick fix with null-checking Element like this:

            if (e.PropertyName == Page.TitleProperty.PropertyName)
            {
                if (Element?.Children != null)
                {
                    var page = (Page)sender;
                    var index = Element.Children.IndexOf(page);
                    TabLayout.Tab tab = _tabLayout.GetTabAt(index);
                    tab.SetText(page.Title);
                }
            }
@lirkki

lirkki Aug 30, 2016

Contributor

@jimmgarrido While verifying bug https://bugzilla.xamarin.com/show_bug.cgi?id=43734 using this commit, I met a regression crash when disposing my TabbedPage. The crash occured because OnPagePropertyChanged was called and Element was null. In my work area I made a quick fix with null-checking Element like this:

            if (e.PropertyName == Page.TitleProperty.PropertyName)
            {
                if (Element?.Children != null)
                {
                    var page = (Page)sender;
                    var index = Element.Children.IndexOf(page);
                    TabLayout.Tab tab = _tabLayout.GetTabAt(index);
                    tab.SetText(page.Title);
                }
            }

This comment has been minimized.

@rmarinho

rmarinho Sep 16, 2016

Member

On dispose you need to remove the events of the pages @jimmgarrido , can you try with that.

@rmarinho

rmarinho Sep 16, 2016

Member

On dispose you need to remove the events of the pages @jimmgarrido , can you try with that.

This comment has been minimized.

@rmarinho

rmarinho Oct 1, 2016

Member

@lirkki please try again if you can @jimmgarrido updated the pr

@rmarinho

rmarinho Oct 1, 2016

Member

@lirkki please try again if you can @jimmgarrido updated the pr

@rmarinho

You need to clear all subscribes to pages made on setup when you dispose, not just the one to remove.

@jimmgarrido

This comment has been minimized.

Show comment
Hide comment
@jimmgarrido

jimmgarrido Oct 1, 2016

Collaborator

@rmarinho I'm doing the unsubscribe while inside the foreach loop that goes through `Element.Children' during disposing so this should clear it from all the pages, no?

Collaborator

jimmgarrido commented Oct 1, 2016

@rmarinho I'm doing the unsubscribe while inside the foreach loop that goes through `Element.Children' during disposing so this should clear it from all the pages, no?

@rmarinho

This comment has been minimized.

Show comment
Hide comment
@rmarinho

rmarinho Oct 1, 2016

Member

Sorry my bad! 👍

Member

rmarinho commented Oct 1, 2016

Sorry my bad! 👍

@adrianknight89

This comment has been minimized.

Show comment
Hide comment
@adrianknight89

adrianknight89 Oct 1, 2016

Contributor

Referencing #410 again.

Feel free to use my sample code here

Contributor

adrianknight89 commented Oct 1, 2016

Referencing #410 again.

Feel free to use my sample code here

@hartez hartez self-assigned this Oct 20, 2016

@hartez

A couple of minor changes and a rebase, and this should be good to go.

@jimmgarrido

This comment has been minimized.

Show comment
Hide comment
@jimmgarrido

jimmgarrido Oct 24, 2016

Collaborator

@hartez Done!

Collaborator

jimmgarrido commented Oct 24, 2016

@hartez Done!

@hartez

hartez approved these changes Oct 24, 2016

@hartez hartez merged commit 7afe69e into xamarin:master Nov 2, 2016

2 of 6 checks passed

Android-UITests Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle8 (EZ Test) :: UI Tests :: OSX Test Cloud Package - Run Android 6.0.1 : Canceled (E…
Details
iOS10-UITests Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle8 (EZ Test) :: UI Tests :: OSX Test Cloud Package - Run iOS Unified iOS10 : Tests f…
Details
iOS8-UITests Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle8 (EZ Test) :: UI Tests :: OSX Test Cloud Package - Run iOS Unified IOS8 : Tests fa…
Details
iOS9-UITests Started TeamCity Build Xamarin.Forms :: Debug :: Cycle9 (EZ Test) :: UI Tests :: OSX Test Cloud Package - Run iOS Unified iOS10 (old style)
Details
OSX-Debug-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle9 (EZ Test) :: OSX Debug : Running
Details
Windows-Debug-C8 Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle9 (EZ Test) :: Windows Debug : Tests passed: 3458, ignored: 8
Details

@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