Skip to content

Migrate activities to ViewPager2#152

Merged
samfundev merged 1 commit intotwireapp:masterfrom
TacoTheDank:viewpager2
Dec 19, 2020
Merged

Migrate activities to ViewPager2#152
samfundev merged 1 commit intotwireapp:masterfrom
TacoTheDank:viewpager2

Conversation

@TacoTheDank
Copy link
Contributor

@TacoTheDank TacoTheDank commented Oct 24, 2020

Yes, the holy viewpager2 migration. But first, some problems.

Before we begin, the migrated channel and search activities work perfectly fine. Those have no issues.

However, the chat fragment has two bugs that need to be fixed. Both bugs have to do with the emotes.

The first came with the migration (likely due to a coding mistake of some sort). The second one has already existed but became much more obvious after the migration due to a behavior change that probably comes with viewpager2. I don't really know how to fix either of these bugs.

Bug 1 (new bug, and you will see it plain as day): OH GOD THE UNICODE EMOJI TAB IS MISSING. Well, yes but no. Swipe to it (or tap on where the icon would be), and the tab icon will load. I can't seem to find any reason why the other tabs color just fine, yet the unicode emoji tab won't. If you turn on the dark modes, you will find that the unicode emoji tab icon is plain white while the others are correctly tinted. Apparently this specific tab just doesn't tint correctly until you swipe to or tap on it, and then it's fine afterward (unless you exit the stream and go back in again, then you have to do it over again).

(Funny thing about bug 1: it was actually much worse, with none showing until swiping or tapping on them, but I managed to fix that by moving the for loop into the TabLayoutMediator.)

Bug 2 (existing bug): First, test your existing 2.9.0 stable F-Droid release version, and go to a random current stream. You should be able to reproduce this behavior (watch the whole thing). Now test the APK I've attached below and try the same thing. You will quickly see that the already unwanted behavior has changed for the worse: now you won't be able to scroll to the bottom because it jumps. One should be able to scroll completely up and down without any sort of stopping before the end of the list.

Here's an APK so you can see this migration in action for yourself (remember to check out the channel and search activity tabs, as those have been migrated to viewpager2): app-debug.zip

There are several roads to take:
1.) This can be merged just as-is, and bug fixes can be made in following commits.
2.) I always allow edits by maintainers, so bug fixes can be committed to this PR.

Either way, these bugs should probably be fixed before the next release.

@samfundev
Copy link
Collaborator

samfundev commented Dec 8, 2020

Sorry, I wanted to make this clear because I forgot to say this when this was originally posted. I plan on going with road 2 and I'll try to fix the two bugs when I can.

@TacoTheDank
Copy link
Contributor Author

@samfundev Alright, thanks!

@TacoTheDank
Copy link
Contributor Author

TacoTheDank commented Dec 8, 2020

@samfundev Just a heads-up: I've rebased the commits onto the latest upstream, and separated the activity and fragment migrations into different commits.

@TacoTheDank TacoTheDank changed the title Migrate to viewpager2 Migrate to ViewPager2 Dec 8, 2020
@TacoTheDank TacoTheDank changed the title Migrate to ViewPager2 Migrate activities to ViewPager2 Dec 19, 2020
@TacoTheDank
Copy link
Contributor Author

TacoTheDank commented Dec 19, 2020

@samfundev I have created a separate pull request for the ChatFragment so that the activities aren't being held up. For all intents and purposes, this is now good to merge.

@samfundev
Copy link
Collaborator

@TacoTheDank Great idea! I'll merge this in. Much appreciated.

@samfundev samfundev merged commit 5ebe5e5 into twireapp:master Dec 19, 2020
@TacoTheDank TacoTheDank deleted the viewpager2 branch December 19, 2020 23:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants