hammady / Islamic-Streamer-Android

Stream Islamic audio content (Quran, lessons, videos, ...) on your Android phone
20 stars 8 forks source link

Checkboxes are out of order #1

Closed hammady closed 13 years ago

hammady commented 13 years ago

When selecting any of the checkboxes, duplicate checkboxes are selected automatically

moamenam commented 13 years ago

i'm stucked in this issue

hammady commented 13 years ago

This is the first thing to work on guys

wnafee commented 13 years ago

salam I found the problem with this and i fixed it alhamdolillah.

In ListAdapter.java in under line 71 place the following 2 lines of code:

cb.setOnCheckedChangeListener(null); cb.setChecked(getItem(position).isChecked);

Explanation:

The problem is that the checkboxes are reused in the convert view and nothing sets their state to begin with. In Android, the UI tries to save memory by reusing old list items that have gone out of the screen. So if you have a long list and one of the rows scrolls out of the screen at the bottom while you are scrolling, it will be recycled and reused to create the new row that just came into the screen at the top. Android passes it on to you in the variable called "convertView" in your function getView(). If it is null, that means you should be creating a new row entry. If it is non-null, then this means it is giving you a recycled row that you are supposed to fill in your new row data for.

In your case, you did not fill in the checkbox state for this row, you just left it the same way it was before. So if one of the rows you happened to get recycled was a row that he its checkbox checked, then you will see it for your row as well.

Hope that makes sense.

That said, there are a lot of things that I saw that were strange in this class.... for example:

"((ProjectObject)((ListView) ((View)v.getParent()).getParent()).getAdapter().getItem(position)).isChecked=false;"

Yanhar abyad :)

This should never happen this way. You can simply do something like this:

ListAdapter.this.getItem(position).isChecked = false;

Also I saw this: View v2=((View)((View)((View)((View)((View)((View)v.getParent()).getParent()).getParent()).getParent()).getParent().getParent().getParent()).findViewById(R.id.alaMenuLayout));

This is extreeeeeeemely dangerous because if you touch your layout files in any way, then you will get null pointers. You should totally change the way that you access UI elements from your adapters to follow the common 3urf standards.

(by the way, it is because of these types of problems that I had to give you the 2 lines to add to fix the problem, ideally it should only have been the 1 line for "cb.setChecked(getItem(position).isChecked);" but because of the long chained getParent() calls, it causes a null pointer exception so i had to set the check listener to null first before i do this!)

wnafee commented 13 years ago

sorry, clicked close by mistake... so i reopened

moamenam commented 13 years ago

ما شاء الله really very amazing notice, i knew what you said about the list view items as i saw that it has only 12 children but i never thought that it'll be very easy to be solved like this. thanks you very much

about the code you mention which is strange, i had to do exactly what you said, each time i change any thing in the layout, I've to change this part of code, but do you know better way to get the listview object from the checkbox item in the listview item??

thank you very much for help i'll do this fix in the version i have locally

moamenam commented 13 years ago

i think you can close the issue now hossam

hammady commented 13 years ago

Excellent job @wnafee, did you push your changes? I will close the issue now, @moamenam please pull the changes by issuing git pull master (am I correct here?).

wnafee commented 13 years ago

pushed

wnafee commented 13 years ago

@moamenam. regarding how to fix the issue with nesting calls to getParent(), the way to deal with it is to leave all UI changes to be handled in your activity class (in your case QuranStreaming.java) and let the ListAdapter focus on just feeding entries on the list.

So basically 3 steps you should do:

1) Changed ALL views in your listview.xml and listview_ar.xml file to be android:focusable = "false"

2) Remove ALL the onCheckChangedListeners, onClickListeners etc from you getView() function in your ListAdapter (the reason why you have to do this is because if you have an item in XML set to be non-focusable, then in runtime you add an onclicklistener to it, then the Android UI framework will change it to focusable at runtime on order to allow it to be clickable. so it ends up cancelling what you did in your xml)

3) Implement lvSurah's onItemClickListener() in QuranStreaming.java, and in there you can set your checkbox to checked and then you can do whatever UI changes you need there with your "alaMenuLayout" etc...

hope that helps inshaAllah

moamenam commented 13 years ago

thank you very much, i'll do these changes

On Wed, Jun 22, 2011 at 3:50 PM, wnafee < reply@reply.github.com>wrote:

@moamenam. regarding how to fix the issue with nesting calls to getParent(), the way to deal with it is to leave all UI changes to be handled in your activity class (in your case QuranStreaming.java) and let the ListAdapter focus on just feeding entries on the list.

So basically 3 steps you should do:

1) Changed ALL views in your listview.xml and listview_ar.xml file to be android:focusable = "false"

2) Remove ALL the onCheckChangedListeners, onClickListeners etc from you getView() function in your ListAdapter (the reason why you have to do this is because if you have an item in XML set to be non-focusable, then in runtime you add an onclicklistener to it, then the Android UI framework will change it to focusable at runtime on order to allow it to be clickable. so it ends up cancelling what you did in your xml)

3) Implement lvSurah's onItemClickListener() in QuranStreaming.java, and in there you can set your checkbox to checked and then you can do whatever UI changes you need there with your "alaMenuLayout" etc...

hope that helps inshaAllah

Reply to this email directly or view it on GitHub:

https://github.com/hammady/Islamic-Streamer-Android/issues/1#issuecomment-1418063

Moamen Ahmed Morsy Senior Software Engineer Ejada-KSA Tel:+966-59-8522038