Create Shared Elements Transition from All/RoomSessionsFragment to SessionDetailFragment #564
Conversation
| if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) { | ||
| setEnterSharedElementCallback(sharedElementCallback) | ||
| } | ||
|
|
There was a problem hiding this comment.
| Unexpected blank line(s) before “}” |
| @@ -83,9 +121,11 @@ class SessionDetailActivity : | |||
| } | |||
| } | |||
|
|
|||
|
|
|||
| fun findFragmentByPosition(viewPager: ViewPager, position: Int): SessionDetailFragment { | ||
| return instantiateItem(viewPager, position) as SessionDetailFragment | ||
| } | ||
|
|
There was a problem hiding this comment.
| Unexpected blank line(s) before “}” |
| fun start(context: Context, session: Session) { | ||
| context.startActivity(createIntent(context, session.id)) | ||
| } | ||
|
|
||
| fun start(activity: Activity, session: Session, sharedElement: Pair<View, String>) { | ||
| val options = ActivityOptionsCompat.makeSceneTransitionAnimation(activity, sharedElement) |
There was a problem hiding this comment.
| Exceeded max line length (100) |
| putExtra(EXTRA_TRANSITION_NAME, sharedElement.second) | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
| Unexpected blank line(s) before “}” |
|
👀 |
| @@ -32,6 +40,8 @@ class SessionDetailActivity : | |||
| @Inject lateinit var viewModelFactory: ViewModelProvider.Factory | |||
| @Inject lateinit var drawerMenu: DrawerMenu | |||
|
|
|||
| private var backPressed = false | |||
There was a problem hiding this comment.
Please comment why this need 🙏
There was a problem hiding this comment.
I think it will not be needed..., I will remove this on next commit. Thanks.
| @@ -75,6 +83,26 @@ class SessionDetailFragment : Fragment(), Injectable { | |||
| } | |||
|
|
|||
| binding.toolbar.setNavigationOnClickListener { activity?.finish() } | |||
|
|
|||
| binding.speakerSummary.viewTreeObserver.addOnPreDrawListener( | |||
There was a problem hiding this comment.
Can you add an extension method to ViewExt?
fun View.addOnetimeOnPreDrawListener(listener:()->Unit)
| } | ||
|
|
||
| @TargetApi(Build.VERSION_CODES.LOLLIPOP) | ||
| private val sharedElementCallback = object : SharedElementCallback() { |
There was a problem hiding this comment.
I have a question 🙋
SharedElementCallback is not contained API Level 19.
Is this not crash on API Level 19?
| @@ -75,6 +83,26 @@ class SessionDetailFragment : Fragment(), Injectable { | |||
| } | |||
|
|
|||
| binding.toolbar.setNavigationOnClickListener { activity?.finish() } | |||
|
|
|||
| binding.speakerSummary.viewTreeObserver.addOnPreDrawListener( | |||
There was a problem hiding this comment.
| @@ -34,6 +39,9 @@ class SessionDetailFragment : Fragment(), Injectable { | |||
| ViewModelProviders.of(activity!!, viewModelFactory).get(SessionDetailViewModel::class.java) | |||
| } | |||
|
|
|||
| val speakerSummary: SpeakersSummaryLayout | |||
There was a problem hiding this comment.
[nits] Do you use this property?
There was a problem hiding this comment.
Thanks! I forgot to remove this property..., I will remove on next commit.
e10dokup
force-pushed
the
feature/450_session_transition
branch
from
ceddbbe
to
f0b84f5
Compare
e10dokup
force-pushed
the
feature/450_session_transition
branch
from
f0b84f5
to
1d73ffe
Compare
|
@takahirom @rkowase Thanks for your reviews! I added fix commits to respond your comments. |
e10dokup
force-pushed
the
feature/450_session_transition
branch
from
e73e923
to
dd9b57c
Compare
e10dokup
force-pushed
the
feature/450_session_transition
branch
from
dd9b57c
to
17d0220
Compare
| && Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) { | ||
| initViewTransitions(view) | ||
| } | ||
|
|
There was a problem hiding this comment.
| Unexpected blank line(s) before “}” |
|
I'm sorry. |
|
@takahirom Thanks. I'm sorry I create poor PR... |
Issue
Overview (Required)
（以下日本語で失礼します）
SessionDetailFrgamentからAll/RoomSessionsFragmentへの戻りのTransactionを実装したかったのですが、SessionDetailActivityのViewPagerに入るSessionDetailFragmentのSessionが全セッションとなっているため、RoomSessionsFragmentとリストの整合性がうまく取れず綺麗に遷移できなかったため、現状は戻りに関してはアニメーションをしないようにしています…。
Links
Screenshot