Create Shared Elements Transition from All/RoomSessionsFragment to SessionDetailFragment #564

e10dokup · 2018-02-03T03:09:58Z

Issue

close [Assigned] Add animating speaker icon in ActivityTransition from a session list screen to a session detail screen #450

Overview (Required)

Create Shared Elements Transition
- All/RoomSessionsFragment to SessionDetailFragment
- SpeakersSummaryLayout will be set to Shared Element

（以下日本語で失礼します）

SessionDetailFrgamentからAll/RoomSessionsFragmentへの戻りのTransactionを実装したかったのですが、SessionDetailActivityのViewPagerに入るSessionDetailFragmentのSessionが全セッションとなっているため、RoomSessionsFragmentとリストの整合性がうまく取れず綺麗に遷移できなかったため、現状は戻りに関してはアニメーションをしないようにしています…。

Links

Screenshot

Before	After

droidkaigibot · 2018-02-03T03:15:26Z

...rc/main/java/io/github/droidkaigi/confsched2018/presentation/detail/SessionDetailActivity.kt

+                    if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) {
+                        setEnterSharedElementCallback(sharedElementCallback)
+                    }
+


⚠️ Unexpected blank line(s) before “}”

droidkaigibot · 2018-02-03T03:15:26Z

...rc/main/java/io/github/droidkaigi/confsched2018/presentation/detail/SessionDetailActivity.kt

@@ -83,9 +121,11 @@ class SessionDetailActivity :
        }
    }

+


⚠️ Needless blank line(s)

droidkaigibot · 2018-02-03T03:15:27Z

...rc/main/java/io/github/droidkaigi/confsched2018/presentation/detail/SessionDetailActivity.kt

+        fun findFragmentByPosition(viewPager: ViewPager, position: Int): SessionDetailFragment {
+            return instantiateItem(viewPager, position) as SessionDetailFragment
+        }
+


⚠️ Unexpected blank line(s) before “}”

droidkaigibot · 2018-02-03T03:15:27Z

...rc/main/java/io/github/droidkaigi/confsched2018/presentation/detail/SessionDetailActivity.kt

        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)


⚠️ Exceeded max line length (100)

droidkaigibot · 2018-02-03T03:15:28Z

...rc/main/java/io/github/droidkaigi/confsched2018/presentation/detail/SessionDetailActivity.kt

+                putExtra(EXTRA_TRANSITION_NAME, sharedElement.second)
+            }
+        }
+


⚠️ Unexpected blank line(s) before “}”

takahirom · 2018-02-03T04:25:22Z

👀

takahirom · 2018-02-03T04:30:19Z

...rc/main/java/io/github/droidkaigi/confsched2018/presentation/detail/SessionDetailActivity.kt

@@ -32,6 +40,8 @@ class SessionDetailActivity :
    @Inject lateinit var viewModelFactory: ViewModelProvider.Factory
    @Inject lateinit var drawerMenu: DrawerMenu

+    private var backPressed = false


Please comment why this need 🙏

I think it will not be needed..., I will remove this on next commit. Thanks.

takahirom · 2018-02-03T04:37:25Z

...rc/main/java/io/github/droidkaigi/confsched2018/presentation/detail/SessionDetailFragment.kt

@@ -75,6 +83,26 @@ class SessionDetailFragment : Fragment(), Injectable {
        }

        binding.toolbar.setNavigationOnClickListener { activity?.finish() }
+
+        binding.speakerSummary.viewTreeObserver.addOnPreDrawListener(


Can you add an extension method to ViewExt?

fun View.addOnetimeOnPreDrawListener(listener:()->Unit)

https://github.com/takahirom/material-element/blob/490090906716d129c26a5329930b5c41b9e84724/app/src/main/java/com/github/takahirom/materialelement/animation/OnetimeViewTreeObserver.java#L7

takahirom · 2018-02-03T04:48:54Z

...rc/main/java/io/github/droidkaigi/confsched2018/presentation/detail/SessionDetailActivity.kt

+    }
+
+    @TargetApi(Build.VERSION_CODES.LOLLIPOP)
+    private val sharedElementCallback = object : SharedElementCallback() {


I have a question 🙋
SharedElementCallback is not contained API Level 19.
Is this not crash on API Level 19?

takahirom · 2018-02-03T04:50:50Z

...rc/main/java/io/github/droidkaigi/confsched2018/presentation/detail/SessionDetailFragment.kt

@@ -75,6 +83,26 @@ class SessionDetailFragment : Fragment(), Injectable {
        }

        binding.toolbar.setNavigationOnClickListener { activity?.finish() }
+
+        binding.speakerSummary.viewTreeObserver.addOnPreDrawListener(


Can you separate method like this https://github.com/DroidKaigi/conference-app-2018/blob/master/app/src/main/java/io/github/droidkaigi/confsched2018/presentation/speaker/SpeakerDetailFragment.kt#L109 ? 👀

rkowase · 2018-02-03T06:59:14Z

...rc/main/java/io/github/droidkaigi/confsched2018/presentation/detail/SessionDetailFragment.kt

@@ -34,6 +39,9 @@ class SessionDetailFragment : Fragment(), Injectable {
        ViewModelProviders.of(activity!!, viewModelFactory).get(SessionDetailViewModel::class.java)
    }

+    val speakerSummary: SpeakersSummaryLayout


[nits] Do you use this property?

Thanks! I forgot to remove this property..., I will remove on next commit.

e10dokup · 2018-02-04T04:31:33Z

@takahirom @rkowase Thanks for your reviews! I added fix commits to respond your comments.

…us object

…crash

droidkaigibot · 2018-02-04T04:53:41Z

...rc/main/java/io/github/droidkaigi/confsched2018/presentation/detail/SessionDetailFragment.kt

+                && Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) {
+            initViewTransitions(view)
+        }
+


⚠️ Unexpected blank line(s) before “}”

takahirom · 2018-02-04T13:09:28Z

I'm sorry.
I am thinking about this PR. 🤔

e10dokup · 2018-02-05T00:11:32Z

@takahirom Thanks. I'm sorry I create poor PR...

droidkaigibot reviewed Feb 3, 2018

View reviewed changes

takahirom reviewed Feb 3, 2018

View reviewed changes

rkowase reviewed Feb 3, 2018

View reviewed changes

e10dokup force-pushed the feature/450_session_transition branch from ceddbbe to f0b84f5 Compare February 4, 2018 04:23

e10dokup added 3 commits February 4, 2018 13:25

Create transition

2b78515

Fix missing transition

ed971ba

Set Shared element on other sessions fragments

dfe03a6

e10dokup force-pushed the feature/450_session_transition branch from f0b84f5 to 1d73ffe Compare February 4, 2018 04:28

e10dokup force-pushed the feature/450_session_transition branch from e73e923 to dd9b57c Compare February 4, 2018 04:34

Remove unnecessary properties / move SharedElementCallback to anonymo…

17d0220

…us object

e10dokup force-pushed the feature/450_session_transition branch from dd9b57c to 17d0220 Compare February 4, 2018 04:35

Add id to SpeakersSummaryLayout on item_horizontal_session for avoid …

3109dca

…crash

droidkaigibot reviewed Feb 4, 2018

View reviewed changes

Fix danger issue

0d2d648

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create Shared Elements Transition from All/RoomSessionsFragment to SessionDetailFragment #564

Create Shared Elements Transition from All/RoomSessionsFragment to SessionDetailFragment #564

e10dokup commented Feb 3, 2018

droidkaigibot Feb 3, 2018

droidkaigibot Feb 3, 2018

droidkaigibot Feb 3, 2018

droidkaigibot Feb 3, 2018

droidkaigibot Feb 3, 2018

takahirom commented Feb 3, 2018

takahirom Feb 3, 2018

e10dokup Feb 3, 2018

takahirom Feb 3, 2018 •

edited

Loading

takahirom Feb 3, 2018 •

edited

Loading

takahirom Feb 3, 2018

rkowase Feb 3, 2018

e10dokup Feb 3, 2018

e10dokup commented Feb 4, 2018

droidkaigibot Feb 4, 2018

takahirom commented Feb 4, 2018

e10dokup commented Feb 5, 2018

Create Shared Elements Transition from All/RoomSessionsFragment to SessionDetailFragment #564

Are you sure you want to change the base?

Create Shared Elements Transition from All/RoomSessionsFragment to SessionDetailFragment #564

Conversation

e10dokup commented Feb 3, 2018

Issue

Overview (Required)

Links

Screenshot

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

takahirom commented Feb 3, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

takahirom Feb 3, 2018 • edited Loading

Choose a reason for hiding this comment

takahirom Feb 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

e10dokup commented Feb 4, 2018

Choose a reason for hiding this comment

takahirom commented Feb 4, 2018

e10dokup commented Feb 5, 2018

takahirom Feb 3, 2018 •

edited

Loading

takahirom Feb 3, 2018 •

edited

Loading