Skip to content
This repository has been archived by the owner on Jun 13, 2024. It is now read-only.

Improvement: Maybe we should switch from ListView to RecyclerView? #41

Closed
developerfromjokela opened this issue Aug 12, 2020 · 23 comments · Fixed by #127
Closed

Improvement: Maybe we should switch from ListView to RecyclerView? #41

developerfromjokela opened this issue Aug 12, 2020 · 23 comments · Fixed by #127
Assignees
Labels
enhancement New feature or request help wanted Extra attention is needed someday No hurry

Comments

@developerfromjokela
Copy link
Collaborator

developerfromjokela commented Aug 12, 2020

ListView is now old, Google itself recommends using RecyclerView.

I've seen in the app when I archive something, the courier Icon spacing gets messed up. I suspect that to the ListView.

Also if my list is able to scroll down, when trying to scroll up, swipe-to-refresh always hits me.

@norkator
Copy link
Owner

ListView is now old, Google itself recommends using RecyclerView.

I've seen in the app when I archive something, the courier Icon spacing gets messed up. I suspect that to the ListView.

Also if my list is able to scroll dowb, when trying to scroll up, swipe-to-refresh always hits me.

That is true and I fully approve changing to recyclerview!

@norkator norkator added the enhancement New feature or request label Aug 12, 2020
@norkator
Copy link
Owner

@developerfromjokela you can do this if you have time. I used now Recycler view on new app so have experience too but I will only make this some time in future. I will notify here if I do it but @developerfromjokela feel free to do if you have time before me.

@developerfromjokela
Copy link
Collaborator Author

@norkator in the latest update, swipe refresh no longer works. Is it intended to be like that?

@developerfromjokela
Copy link
Collaborator Author

It works when I launch the app, if I scroll down, and scroll back up to pull it down, it no longer works

@norkator
Copy link
Owner

It works when I launch the app, if I scroll down, and scroll back up to pull it down, it no longer works

I cannot re produce, works on my devices.

@developerfromjokela
Copy link
Collaborator Author

Hmm interesting, I am using Android 10 on Pixel 4 XL.

You enter the app, immediately after you scroll down, I think the Swipe to refresh gets disabled.

When I scroll back up, it never gets enabled.

@norkator
Copy link
Owner

Hmm interesting, I am using Android 10 on Pixel 4 XL.

You enter the app, immediately after you scroll down, I think the Swipe to refresh gets disabled.

When I scroll back up, it never gets enabled.

Okay yes Android 10 has problems with it.

@norkator
Copy link
Owner

Hmm interesting, I am using Android 10 on Pixel 4 XL.

You enter the app, immediately after you scroll down, I think the Swipe to refresh gets disabled.

When I scroll back up, it never gets enabled.

After more testing looks like onScroll with firstVisiblePosition works when looking from logs but
swipeRefreshLayout.setEnabled does not set it's state everytime. Something wrong with swipeRefreshLayout

@norkator
Copy link
Owner

norkator commented Aug 30, 2020

Yep, on api 28 it works first okay and suddenly stops working all together. api 29 it's not working right at all.

@norkator
Copy link
Owner

image

Found causing problem... it's this method here. Need to add position checking here too.

@developerfromjokela
Copy link
Collaborator Author

@norkator Is the update released to fix the Swipe refresh problem?

@norkator
Copy link
Owner

norkator commented Sep 2, 2020

@norkator Is the update released to fix the Swipe refresh problem?

Yes.

@developerfromjokela
Copy link
Collaborator Author

@norkator On my device problem still exists

@norkator
Copy link
Owner

norkator commented Sep 2, 2020

@norkator On my device problem still exists

What device you have? Is it possible that play store cache caused esrlier version as update? What does version code say at about view?

@developerfromjokela
Copy link
Collaborator Author

@norkator Pixel 4 XL, version code 128 version name 2.1.11

@norkator
Copy link
Owner

norkator commented Sep 2, 2020

@norkator Pixel 4 XL, version code 128 version name 2.1.11

I am using Android 10 phone now too but it works for me. What kind of behavior it does for you?

@ristomatti
Copy link
Collaborator

I was able to reproduce it now also. This time it requires first opening package details, scrolling it down, pressing back. After returning to list view pull to refresh doesn't necessarily trigger straight after scrolling up but it triggers before the topmost item is fully visible.

@norkator
Copy link
Owner

norkator commented Sep 2, 2020

I was able to reproduce it now also. This time it requires first opening package details, scrolling it down, pressing back. After returning to list view pull to refresh doesn't necessarily trigger straight after scrolling up but it triggers before the topmost item is fully visible.

Aaa. Onresume, onpause event or any other has been rogotten maybe for checking.

@norkator
Copy link
Owner

norkator commented Sep 2, 2020

I was able to reproduce it now also. This time it requires first opening package details, scrolling it down, pressing back. After returning to list view pull to refresh doesn't necessarily trigger straight after scrolling up but it triggers before the topmost item is fully visible.

Update, tried too.. yes topmost item is not fully visible till refresh triggers. This is now tricky because listview position gives 0 already. 👎

@developerfromjokela
Copy link
Collaborator Author

@norkator Just checked out, issue happens while it's refreshing. When refreshing stops, it works like it should.

@norkator
Copy link
Owner

norkator commented Sep 2, 2020

Okay I found problem.. listview should be inside swipe refresh layout but it was not. All those code change were total "hacks" so removed them.

@norkator
Copy link
Owner

norkator commented Sep 2, 2020

See #68

@norkator norkator self-assigned this Apr 16, 2021
@norkator
Copy link
Owner

I start working on recycler view at least initially for main menu. Targeting to make it more nicer looking.

@norkator norkator linked a pull request Apr 16, 2021 that will close this issue
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request help wanted Extra attention is needed someday No hurry
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants