Migration to ViewPager 2 #159

vbuberen · 2019-11-17T13:30:50Z

📄 Context

Google encourages developers to try the new ViewPager 2, since it offers quite a lot optimizations.
More info on changes: https://medium.com/google-developer-experts/exploring-the-view-pager-2-86dbce06ff71
The library is in release candidate state, so I am sure that no changes should appear there and we can safely update Chucker to use latest components.
Additionally, this migration allowed to make the code cleaner and more concise.

📝 Changes

Added dependency to corresponding library.
Updated ViewPager adapters in MainActivity and TransactionActivity to be able to work with ViewPager2 as well as changed layouts to use ViewPager2.

…fecycle to FragmentActivity based adapter constructor

cortinico

Generally the code looks good and cleaner, thanks for doing so :)
Other than the comments I left, I noticed two things:

There are way more leaks reported by leakcanary for MainActivity (at least on the emulator). Are you experiencing the same?
There is a flickering of the Option menu when changing tab from Network to Errors for the first time. I can notice that 3 bin icons are shown (rather than 1). Here a Gif to explain this issue:

cortinico · 2020-01-02T01:06:08Z

library/src/main/java/com/chuckerteam/chucker/internal/ui/MainActivity.kt

+            }
+        })
+
+        val tabLayout = findViewById<TabLayout>(R.id.tabLayoutHome)


Suggested change

val tabLayout = findViewById<TabLayout>(R.id.tabLayoutHome)

val tabLayout : TabLayout = findViewById(R.id.tabLayoutHome)

cortinico · 2020-01-02T01:06:29Z

library/src/main/java/com/chuckerteam/chucker/internal/ui/transaction/TransactionActivity.kt

-                selectedTabPosition = position
-                populateUI(transaction)
+    private fun setupViewPager() {
+        val viewPager = findViewById<ViewPager2>(R.id.viewPagerTransaction)


Suggested change

val viewPager = findViewById<ViewPager2>(R.id.viewPagerTransaction)

val viewPager : ViewPager2 = findViewById(R.id.viewPagerTransaction)

cortinico · 2020-01-02T01:06:45Z

library/src/main/java/com/chuckerteam/chucker/internal/ui/transaction/TransactionActivity.kt

+            currentItem = selectedTabPosition
+        }
+
+        val tabLayout = findViewById<TabLayout>(R.id.tabLayoutTransaction)


Suggested change

val tabLayout = findViewById<TabLayout>(R.id.tabLayoutTransaction)

val tabLayout : TabLayout = findViewById(R.id.tabLayoutTransaction)

I think that we should drop all these findViewById usages. But in another PR later.

Yup, feel free to drop it for now.

vbuberen · 2020-01-03T21:29:08Z

Don't see leaks. After merging latest develop changes with ViewModel see no leaks too.
Yeah, strange order of menu creations in fragments. Interesting.

cortinico · 2020-01-04T01:02:38Z

Don't see leaks. After merging latest develop changes with ViewModel see no leaks too.

Same here, so you can drop this now.

Yeah, strange order of menu creations in fragments. Interesting.

This is definitely a regression. Are you able to have a look at it before we merge this?

vbuberen · 2020-01-04T08:55:46Z

Yes, I definitely want to investigate it. Tried a few things yesterday, but wasn't able to resolve just yet.
It is something with transactions list menu invalidation before errors list menu starts its initialization.
Also, it happens only during first switch between views.

vbuberen · 2020-01-12T21:46:21Z

Tried to debug and find the problem, but had no luck so far.
Somehow onCreateOptionsMenu() is called in TransactionListFragment again before calling onCreateOptionsMenu() in ErrorListFragment
Opened an issue on Google's issue tracker: https://issuetracker.google.com/issues/147550812

cortinico · 2020-01-16T16:54:47Z

Tried to debug and find the problem, but had no luck so far.

What do we do here? Do we want to move it to 3.2.0 or we include it as it is? @vbuberen

vbuberen · 2020-01-16T17:43:30Z

I would postpone to 3.2.0 to not compromise on quality. Ideally, we should strive to have stable versions as bugless as possible (even without such minor issues), so users could be confident in library reliability.

vbuberen · 2020-03-30T12:56:19Z

Since no updates to ViewPager2 were presented moving further to versions after 3.2.0

vbuberen · 2020-04-02T07:45:43Z

Yesterday Google released 1.1.0-alpha01 and it has fix for the issue with options menu. Checked it and everything works fine. Going to wait till it gets stable and we will be able to finally merge this PR.

cortinico · 2020-04-03T00:04:45Z

Yesterday Google released 1.1.0-alpha01 and it has fix for the issue with options menu

Great news, let's wait for the stable and we can proceed them. Should we have a label like on hold for PRs like this one?

vbuberen · 2020-04-03T08:29:47Z

I think do not merge works fine here. Don't want to make things too complicated.

vbuberen · 2020-05-22T17:36:38Z

Another test
/rebase

cortinico · 2020-05-22T17:44:12Z

Another test
/rebase

I'm afraid you can't rebase this branch as it has a non linear git history (there are a lot of Merge latest develop changes). It works fine if there are no merge commits in the branch to rebase.

vbuberen · 2020-05-22T17:48:54Z

Yeah, I see.

…fragment

vbuberen · 2020-11-15T13:43:02Z

It's been a year since I opened this PR and it looks like we still can't merge it.
Today I tried to update the PR with latest changes from develop branch, which I believed would unblock us from merging.

Since we have no ViewPager in the MainActivity anymore, we should have no issues with options menu items there, which were one of the reasons why the PR is still unmerged. However, while testing things again I noticed another issue:

Sometimes wrong set of option menu items appears in the toolbar. For example here is a screenshot of Overview tab, where search and save should be hidden:

Due to this issue and the fact that Google hasn't released any updates to ViewPager2 1.0.0 except some alpha (which we can't use to not create potential issues like #469) I am convinced that we are fine with older ViewPager, which has no issues for our usage and needs no workarounds.

Closing this PR. Hope that in future we could return to this when ViewPager2 gets more love from Google.

Vova Buberenko added 6 commits November 17, 2019 14:07

Add dependency for ViewPager2

b7459d5

Migrate HomeScreen to ViewPager2

dc79272

Migrate TransactionPagerAdapter and minor updates for HomePageAdapter

199ee3f

Remove magic constant in TransactionActivity

1c67a08

Workaround for lint error in ViewPager2

28a3e0e

Update to stable release version, switch from fragment manager and li…

4863fc5

…fecycle to FragmentActivity based adapter constructor

cortinico self-requested a review January 2, 2020 00:29

cortinico added the enhancement New feature or improvement to the library label Jan 2, 2020

cortinico requested changes Jan 2, 2020

View reviewed changes

cortinico added this to the 3.1.0 milestone Jan 2, 2020

Merge latest develop changes

c9c952e

vbuberen modified the milestones: 3.1.0, 3.2.0 Jan 18, 2020

vbuberen mentioned this pull request Jan 23, 2020

Better RTL support #117

Closed

vbuberen added 7 commits January 23, 2020 21:39

Merge latest develop changes

0f7edb3

Merge latest develop changes, address PR feedback

1b28512

Fix ktlint issue

70859f0

Merge latest develop changes

bde7724

Merge latest develop changes

cfadd4d

Merge latest develop changes

089b856

Merge latest develop changes

3a64f7c

vbuberen removed this from the 3.2.0 milestone Mar 30, 2020

vbuberen added the ⛔️ do not merge A PR that should not be merged label Apr 3, 2020

Merge latest develop changes

08b2657

cortinico mentioned this pull request May 22, 2020

Fix the CI setup for build on forks #347

Merged

vbuberen added 3 commits May 23, 2020 17:30

Merge branch 'develop' into migration/view_pager2

f370f7c

Merge latest develop changes

f3df0ed

Merge latest develop changes, drop animateLayoutChanges from payload …

18a21c1

…fragment

vbuberen closed this Nov 15, 2020

vbuberen mentioned this pull request Feb 17, 2022

WrongFragmentContainerViolation with TransactionOverviewFragment #774

Open

vbuberen mentioned this pull request Apr 30, 2023

Automatic Scroll on Search added #988

Merged

vbuberen mentioned this pull request Sep 23, 2023

Adding viewpager 2 for TransactionPagerAdapter #1099

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migration to ViewPager 2 #159

Migration to ViewPager 2 #159

vbuberen commented Nov 17, 2019

cortinico left a comment

cortinico Jan 2, 2020

cortinico Jan 2, 2020

cortinico Jan 2, 2020

vbuberen Jan 3, 2020

cortinico Jan 4, 2020

vbuberen commented Jan 3, 2020

cortinico commented Jan 4, 2020

vbuberen commented Jan 4, 2020

vbuberen commented Jan 12, 2020

cortinico commented Jan 16, 2020

vbuberen commented Jan 16, 2020 •

edited

Loading

vbuberen commented Mar 30, 2020

vbuberen commented Apr 2, 2020

cortinico commented Apr 3, 2020

vbuberen commented Apr 3, 2020

vbuberen commented May 22, 2020

cortinico commented May 22, 2020

vbuberen commented May 22, 2020

vbuberen commented Nov 15, 2020 •

edited

Loading

	val tabLayout = findViewById<TabLayout>(R.id.tabLayoutHome)
	val tabLayout : TabLayout = findViewById(R.id.tabLayoutHome)

	val viewPager = findViewById<ViewPager2>(R.id.viewPagerTransaction)
	val viewPager : ViewPager2 = findViewById(R.id.viewPagerTransaction)

Migration to ViewPager 2 #159

Migration to ViewPager 2 #159

Conversation

vbuberen commented Nov 17, 2019

📄 Context

📝 Changes

cortinico left a comment

Choose a reason for hiding this comment

cortinico Jan 2, 2020

Choose a reason for hiding this comment

cortinico Jan 2, 2020

Choose a reason for hiding this comment

cortinico Jan 2, 2020

Choose a reason for hiding this comment

vbuberen Jan 3, 2020

Choose a reason for hiding this comment

cortinico Jan 4, 2020

Choose a reason for hiding this comment

vbuberen commented Jan 3, 2020

cortinico commented Jan 4, 2020

vbuberen commented Jan 4, 2020

vbuberen commented Jan 12, 2020

cortinico commented Jan 16, 2020

vbuberen commented Jan 16, 2020 • edited Loading

vbuberen commented Mar 30, 2020

vbuberen commented Apr 2, 2020

cortinico commented Apr 3, 2020

vbuberen commented Apr 3, 2020

vbuberen commented May 22, 2020

cortinico commented May 22, 2020

vbuberen commented May 22, 2020

vbuberen commented Nov 15, 2020 • edited Loading

vbuberen commented Jan 16, 2020 •

edited

Loading

vbuberen commented Nov 15, 2020 •

edited

Loading