Set My site fragment title to 'My site' instead of using site title #11691
Conversation
|
You can trigger optional UI/connected tests for these changes by visiting CircleCI here. |
| @@ -1009,12 +1006,9 @@ public void onStart() { | |||
|
|
|||
| @Override | |||
| public void setTitle(@NonNull final String title) { | |||
| if (isAdded()) { | |||
There was a problem hiding this comment.
Did you remove the isAdded check because we no longer need context when setting the title?
There was a problem hiding this comment.
I removed the isAdded check because I simplified this implementation using the Me fragment as reference – I have no idea whether it's necessary, so happy to take your lead here.
There was a problem hiding this comment.
I think it isn't necessary :-).
frosty
changed the title
|
You can test the changes on this Pull Request by downloading the APK here. |
| @@ -813,12 +813,8 @@ private void updateTitle() { | |||
| } | |||
|
|
|||
| private void updateTitle(PageType pageType) { | |||
There was a problem hiding this comment.
the updateTitle() method is no longer used so you can remove it
There was a problem hiding this comment.
Isn't it still used from onPageChanged()?
There was a problem hiding this comment.
I meant the one above this one, without parameters. I couldn't comment there :-)
| .setTitle(mBottomNav.getTitleForPageType(pageType).toString()); | ||
| } | ||
| ((MainToolbarFragment) mBottomNav.getActiveFragment()) | ||
| .setTitle(mBottomNav.getTitleForPageType(pageType).toString()); |
There was a problem hiding this comment.
It's out of scope of this PR but this whole logic is strange (and wrong). We don't actually need to update the title anywhere else than in the Bottom nav component since this is a callback triggered by the bottom nav. What we do here is that the bottom nav tells us that a tab was selected and we react to it by asking bottom nav for the active fragment and setting its title from the bottom nav.
| if (mToolbar != null) { | ||
| mToolbar.setTitle(mToolbarTitle); | ||
| } | ||
| mToolbarTitle = title; |
There was a problem hiding this comment.
Out of scope of this PR: I'm actually wondering if we need the body of this method at all since the toolbar title is a constant and we set it in the onCreate method
IANAAD (I Am Not An Android Developer), so please let me know if I did anything wrong! This PR is a small tweak the title of the My Site screen to always read "My site" instead of duplicating the site name, which is shown in the screen header anyway.
To test:
PR submission checklist:
RELEASE-NOTES.txtif necessary.