Detail page has two toolbars

[XinyueZ]

After converting to single-activity, there're two toolbars on detail screen. The app needs a ux/ui decision what should be shown in detail page. @tiembo

[tiembo]

Good catch @XinyueZ - and indeed a good time to think about how the detail page should look. The Session Details page in the Google I/O 2018 Android app can serve as a good baseline of implementing a bottom app bar + FAB, along with title and details.

What are your thoughts?

[XinyueZ]

The IO app can be a benchmark for detail screen. @tiembo that's great. I try some works to make similar effect.

[sagar-viradiya]

@tiembo and @XinyueZ Sounds good!

[XinyueZ]

@tiembo A way to fix duplicated appbar has been in PR(#78).

The the I/O-Like approach of detail screen might be worked with mutli-activities. The problem is if we do that, one of the appbar must be removed and keep one appbar(main), this should change struct of layout of detail screen, any suggestion? @sagar-viradiya

[andhie]

the I//O 2018 app uses a separate activity .ui.sessiondetail.SessionDetailActivity to achieve it. are there plans to revert the commit? i think the proposed solution is by putting the AppBar in individual fragment. this helps to cater different AppBar styles.

[XinyueZ]

@andhie What's your opinion to #78 ?

[andhie]

it feels cheating. and not so elegant solution. it sends out a wrong idea that this is how you're suppose to do it when faced with multiple AppBar situation. at this moment, i am looking for more patterns on how to use Navigation in cases like this. e.g. different style AppBar, no AppBar.

Hopefully @ianhanniballake could shed some light on such cases.

[XinyueZ]

@andhie yes, i c, it's really a new pattern what people should face with Navi-Controller because the pro-suggestion of Google self is using single-activity with different fragments, however, there is no workaround at this moment to deal it with a method that was intended to be resolved with multi-activity. 🤕

[sagar-viradiya]

@XinyueZ Even I have similar thoughts on your changes as @andhie mentioned above. If we are sticking with single activity then we should have appbar for each fragment. Having single appbar in main activity will cause problem if you have new fragment tomorrow and it has different requirement than main activity appbar.

[XinyueZ]

@sagar-viradiya Do you agree to have appbar or toolbar on each fragment? I am not against, actually, I have worked a potential PR which based on this direction. The reason why don't select for the potential PR is that I wouldn't change base struct of all these layouts and keep simple at min changes. @andhie How is your opinion?

[andhie]

i remember the Google I/O Navigation talk mention to put AppBar/Toolbar inside each fragment. I would wait to listen Google's opinionated guide on this.

[sagar-viradiya]

@XinyueZ I would prefer to have separate toolbar for each fragment. We need others opinion here. May be there could be better solution, but right now I could only think of this.

[XinyueZ]

@sagar-viradiya @andhie I have the alternative solution as PR #84, how about this? The approach is using toolbars for each fragment.

[tiembo]

@andhie @sagar-viradiya @XinyueZ I'm going to talk to some folks internally regarding guidance for toolbars / single activity / bottom nav. Will follow up in about a week due to the US July 4th holiday.

[sagar-viradiya]

@tiembo That would be great.

[sagar-viradiya]

@tiembo Any update ?

[XinyueZ]

@sagar-viradiya I prefer to #84

[sagar-viradiya]

@XinyueZ Will review and let you know

[XinyueZ]

@sagar-viradiya thanks, #84 and #78 are two opposite solutions, one is multi appbar, one is single appbar, either of the two.

[tiembo]

Hi everyone - we're working on a solution for toolbars with single Activity / multiple Fragments internally. Will update this issue once we have something to share.

@XinyueZ thanks for opening PRs #78 and #84! It's probably a good idea to hold off on making additional commits to those for now.

[XinyueZ]

@tiembo OK, looking forward to it.

[mcastagnolo]

It looks like the NavigationComponent in alpha03 added functionality to work with implementations where fragments own their toolbars.

https://developer.android.com/jetpack/docs/release-notes#july_12_2018 https://issuetracker.google.com/issues/109868820

[XinyueZ]

I'll try it in a PR, @tiembo @sagar-viradiya it seems great.

[XinyueZ]

@tiembo

I am not sure whether it is really logical to put two different scenarios and businesses together.

official document, at last section mentions that ..... In cases where multiple Activities share the same layout, the navigation graphs can be combined ....

In our case they have actually different layout bases.

[sagar-viradiya]

@XinyueZ @tiembo Any plan to address this ?

[XinyueZ]

There're 2 PRs from my sides which could solve the problem, however, I prefer the #84 which uses the multi toolbar in different fragments for the case. Rather, still waiting for Google's official pattern. At moment there're conflicts, I'm just waiting for the merges of #170 and another associated PR of loading-ui @tiembo

[XinyueZ]

@tiembo Please check #238, the new version of nav-lib might be a chance to fix this issue, I guess only. I will try a private branch with new lib to find way to fix the two-toolbar problem.

[tiembo]

Hi @XinyueZ, sorry this issue and your PRs have been sitting for awhile.

Given how we will eventually add Material design to Sunflower I merged #315 as a fix for the double toolbar. I'll go ahead and close out your PRs #78 and #84 - thank you for exploring those approaches!