Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some bugfixes #6

Open
wants to merge 37 commits into
base: master
Choose a base branch
from
Open

Some bugfixes #6

wants to merge 37 commits into from

Conversation

brstgt
Copy link

@brstgt brstgt commented Sep 23, 2013

Please review my changes and pull them if possible.

I don't know exactly what the intention was in AdapterDataSetObserver.onChanged if lazyload is set. It did not work for me as it realigned the first visible position after a load, which IMHO should not happen.
With my small changes, the list works great for me.

  • No crashes on retained fragments after orientation change
  • Proper lazy loading, not on every drag movement
  • Gradle support for Android Studio

@jordijansen
Copy link

Please pull this in!

Fixed a bug for me with multiple views. Thanks @brstgt

@bm-i
Copy link

bm-i commented Dec 2, 2013

This is great, thanks a lot! This pull request fixed a lot of issues we had: cells not being reused, reload when long-pressing items, etc. Please merge this request. 👍

@brstgt
Copy link
Author

brstgt commented Dec 2, 2013

Sorry, I am a bit confused. What should I Merge? I don't see any commits concerning this in your repo?

@bm-i
Copy link

bm-i commented Dec 2, 2013

This comment was directed to the maintainer, i.e., he should merge your pull request, as it fixes a lot of bugs. You don't need to do anything.

@bm-i
Copy link

bm-i commented Dec 2, 2013

However, now I see the view jumping to the top or somehow reloading all items when scrolling fast down. After this initial jump scrolling works fine. What could cause this?

@brstgt
Copy link
Author

brstgt commented Dec 2, 2013

... i see ... thanks :)

@scrolling:
I had the issue that the view jumped to top when the adapter called
"notifyDatasetChanged". This happens if the adapter returns false on
"hasStableIds".
If hasStableIds returns true, everything is okay. This is maybe due to
recalculating positions was not possible, if you expect to have a
'unreliable' dataset. I didn't want to dig that deep so i just worked with
'stable ids'.

But I never had problems with jumping when flinging. We have it in
production. See "Jaumo" app in Google Play.

2013/12/2 bm-i [email protected]

However, now I see the view jumping to the top or somehow reloading all
items when scrolling fast down. After this initial jump scrolling works
fine. What could cause this?


Reply to this email directly or view it on GitHubhttps://github.com//pull/6#issuecomment-29613545
.

@brstgt
Copy link
Author

brstgt commented Dec 2, 2013

P.S.: You can simply fork my version.

I am sorry for some confusing commit messages. The reason is, that this is
a submodule of another project and I accidentally used the same commit
message when commiting for the parent project.

2013/12/2 benjamin roth [email protected]

... i see ... thanks :)

@scrolling:
I had the issue that the view jumped to top when the adapter called
"notifyDatasetChanged". This happens if the adapter returns false on
"hasStableIds".
If hasStableIds returns true, everything is okay. This is maybe due to
recalculating positions was not possible, if you expect to have a
'unreliable' dataset. I didn't want to dig that deep so i just worked with
'stable ids'.

But I never had problems with jumping when flinging. We have it in
production. See "Jaumo" app in Google Play.

2013/12/2 bm-i [email protected]

However, now I see the view jumping to the top or somehow reloading all
items when scrolling fast down. After this initial jump scrolling works
fine. What could cause this?


Reply to this email directly or view it on GitHubhttps://github.com//pull/6#issuecomment-29613545
.

@bm-i
Copy link

bm-i commented Dec 2, 2013

I just switched over to your fork and made the adapter return true in hasStableIds and set a button as the header. When scrolling down it doesn't jump to the top anymore, but now when scrolling back up to the top manually, suddenly the position of the right column is wrong and there's a gap at the start. The data is stable and notifyDataSetChanged is only called once, after the initial data has been loaded. The items themselves also have a constant height. Tapping on one of the elements fixes the layout. Do you have any idea what could cause this?
I also checked out the app and it seems to work just fine in your case, with the list as the header.

@brstgt
Copy link
Author

brstgt commented Dec 2, 2013

Maybe the height of your views "changed"? Do you return the right value for
getViewTypeCount in your adapter?
I don't do any magic, the adapter follows common best practices for android.

This initialized my grid view:
protected void setUpListView() {
int margin = AQUtility.dip2pixel(getActivity(), 8);
gridView.setItemMargin(margin);
gridView.setPadding(margin, 0, margin, 0);

    gridView.setColumnCount(getColumnCount());
    gridView.setAdapter(this.adapter);
    gridView.setOnLoadmoreListener(new

StaggeredGridView.OnLoadmoreListener() {
@OverRide
public void onLoadmore() {
loadMore();
}
});
loaderView =
LayoutInflater.from(getActivity()).inflate(R.layout.gridview_loader,
gridView, false);
gridView.setFooterView(loaderView);

    aq.id(R.id.jumpToTop).clicked(new View.OnClickListener() {
        @Override
        public void onClick(View v) {
            aq.id(R.id.jumpToTop).gone();
            gridView.setSelectionToTop();
        }
    });

    gridView.setJumpToTopListener(new

StaggeredGridView.JumpToTopListener() {
@OverRide
public void onJumpToTopStateChanged(boolean canJump) {
if (canJump) {
aq.id(R.id.jumpToTop).visible();
}
else {
aq.id(R.id.jumpToTop).invisible();
}
}
});
gridView.setHeaderView(spotlightView);

}

2013/12/2 bm-i [email protected]

I just switched over to your fork and made the adapter return true in
hasStableIds and set a button as the header. When scrolling down it
doesn't jump to the top anymore, but now when scrolling back up to the top
manually, suddenly the position of the right column is wrong and there's a
gap at the start. The data is stable and notifyDataSetChanged is only
called once, after the initial data has been loaded. The items themselves
also have a constant height. Tapping on one of the elements fixes the
layout. Do you have any idea what could cause this?
I also checked out the app and it seems to work just fine in your case,
with the list as the header.


Reply to this email directly or view it on GitHubhttps://github.com//pull/6#issuecomment-29618414
.

@bm-i
Copy link

bm-i commented Dec 2, 2013

Thanks for the help! I just created a small demo adapter which returns fixed size views, and getCount returning a fixed number. An initial notifyDataSetChanged loads the data and everything looks good. For testing purposes, I then start a timer calling notifyDataSetChanged periodically every 2 seconds. Even if the grid is in it's initial top position, the layout reorders views in a very strange way and also while scrolling the layout is completely broken and contains overlaps and gaps. I'm not sure what is causing it and it seemed to have worked when I used this original fork, but I could also be wrong. Quite frustrating to find a fully working solution ...

@brstgt
Copy link
Author

brstgt commented Dec 2, 2013

If you have a completely new dataset, you should re-set the adapter using
setAdapter. This repopulates alle completely from scratch see clearAllState
and populate(true)
I only use notifyDataSetChanged when appending data. If i reload then i
re-set the adapter. That worked for me.

Yes - it is frustrating - was also for me. But actually i also did not find
a better working solution.

2013/12/2 bm-i [email protected]

Thanks for the help! I just created a small demo adapter which returns
fixed size views, and getCount returning a fixed number. An initial
notifyDataSetChanged loads the data and everything looks good. For
testing purposes, I then start a timer calling notifyDataSetChangedperiodically every 2 seconds. Even if the grid is in it's initial top
position, the layout reorders views in a very strange way and also why
scrolling the layout is completely broken and contains overlaps and gaps.
I'm not sure what is causing it and it seemed to have worked when I used
this original fork, but I could also be wrong. Quite frustrating to find a
fully working solution ...


Reply to this email directly or view it on GitHubhttps://github.com//pull/6#issuecomment-29623916
.

@bm-i
Copy link

bm-i commented Dec 2, 2013

Well, I actually want to add new data when reaching the end of the view,
but it should also work in that case. Thanks anyway!

On 2 Dec 2013, at 16:05, Benjamin Roth [email protected] wrote:

If you have a completely new dataset, you should re-set the adapter using
setAdapter. This repopulates alle completely from scratch see clearAllState
and populate(true)
I only use notifyDataSetChanged when appending data. If i reload then i
re-set the adapter. That worked for me.

Yes - it is frustrating - was also for me. But actually i also did not find
a better working solution.

2013/12/2 bm-i [email protected]

Thanks for the help! I just created a small demo adapter which returns
fixed size views, and getCount returning a fixed number. An initial
notifyDataSetChanged loads the data and everything looks good. For
testing purposes, I then start a timer calling notifyDataSetChangedperiodically every 2 seconds. Even if the grid is in it's initial top
position, the layout reorders views in a very strange way and also why
scrolling the layout is completely broken and contains overlaps and gaps.
I'm not sure what is causing it and it seemed to have worked when I used
this original fork, but I could also be wrong. Quite frustrating to find a
fully working solution ...


Reply to this email directly or view it on GitHubhttps://github.com//pull/6#issuecomment-29623916
.


Reply to this email directly or view it on GitHub.

@brstgt
Copy link
Author

brstgt commented Dec 2, 2013

Stupid question:

You are really sure you have my master branch and it is really compiled in?
The effects you describe sound very familiar to me but i never had it with
my latest fixes.

Maybe your submodule did not update or recompile correctly?

At least... implementing an infinite scroller worked very well for me.

2013/12/2 bm-i [email protected]

Well, I actually want to add new data when reaching the end of the view,
but it should also work in that case. Thanks anyway!

On 2 Dec 2013, at 16:05, Benjamin Roth [email protected] wrote:

If you have a completely new dataset, you should re-set the adapter
using
setAdapter. This repopulates alle completely from scratch see
clearAllState
and populate(true)
I only use notifyDataSetChanged when appending data. If i reload then i
re-set the adapter. That worked for me.

Yes - it is frustrating - was also for me. But actually i also did not
find
a better working solution.

2013/12/2 bm-i [email protected]

Thanks for the help! I just created a small demo adapter which returns
fixed size views, and getCount returning a fixed number. An initial
notifyDataSetChanged loads the data and everything looks good. For
testing purposes, I then start a timer calling
notifyDataSetChangedperiodically every 2 seconds. Even if the grid is in
it's initial top
position, the layout reorders views in a very strange way and also why
scrolling the layout is completely broken and contains overlaps and
gaps.
I'm not sure what is causing it and it seemed to have worked when I
used
this original fork, but I could also be wrong. Quite frustrating to
find a
fully working solution ...


Reply to this email directly or view it on GitHub<
https://github.com/bulletnoid/StaggeredGridView/pull/6#issuecomment-29623916>

.


Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHubhttps://github.com//pull/6#issuecomment-29624468
.

…gin) changed so that column mapping is correct when scrolling up
…a, scrolling more down and back up. This wrecked the column offsets by falsely incrementing mFirstPosition when removing footer view
@Swimburger
Copy link

Great updates, works a lot better now in one of the earlier commit!

Though in the last version I am getting a nullpointer.
When configuration changes, my fragment rebuilds the data and calls notifyDataChanged on my adapter.
Then in the following code mItemBottoms and mItemTops is null.

for (int i = 0; i < colCount; i++) {
mItemBottoms[i] = mItemTops[i];
}

I am not using any header or bottom view.

I'll keep using one of you're previous commits.
Thanks!

@Swimburger
Copy link

Thanks for you reply.

I have tried it but mItemTops is null so it throws a nullpointer exception
on notifyDataSetChanged call on the adapter after a configuration change.

Kind regards,
Niels Swimberghe

2014-12-10 20:05 GMT+01:00 Fluxinated [email protected]:

@Sniels https://github.com/SNiels try this...
final int top = getPaddingTop();
for (int i = 0; i < colCount; i++) {
//final int offset = top + ((mRestoreOffsets != null) ?
Math.min(mRestoreOffsets[i], 0) : 0);
int offset = top;
if ((mRestoreOffsets != null) && mRestoreOffsets.length == colCount)
offset += Math.min(mRestoreOffsets[i], 0);
mItemTops[i] = (offset == 0) ? mItemTops[i] : offset;
mItemBottoms[i] = (offset == 0) ? mItemBottoms[i] : offset;
}


Reply to this email directly or view it on GitHub
#6 (comment)
.

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.

4 participants