Skip to content

Conversation

@eduardb
Copy link

@eduardb eduardb commented Feb 13, 2014

I implemented setSingleChoiceItems (ListAdapter adapter, int checkedItem, final DialogInterface.OnClickListener listener), and added usage example to test activity. Also, I incremented gradle version.

@dentex
Copy link
Owner

dentex commented Feb 13, 2014

Hello!
It's a great addition.
I was wondering if you could do a couple of fixes before merging:

  • the single-choice-items dialog misses the "cancel / OK" buttons;
  • the title it's the same as the one from the 3rd button.

Thanks!

@eduardb
Copy link
Author

eduardb commented Feb 14, 2014

Hi,

  • I didn't need them for my project, so I totally forgot about those. I will add them soon.
  • The last button's text should be right, I just checked...

Regards,
Edy

@dentex
Copy link
Owner

dentex commented Feb 14, 2014

  • Ok, thanks.

(How does it work for your app? you check the item and then? How do you confirm? Just going back? Maybe also "multiple choice" dialog would be cool ;) )

  • The button text is correct. The subsequent dialog text (hard-coded into TestDialogActivity.java at line 145) it's again setTitle("Dialog with disabled Items").

@eduardb
Copy link
Author

eduardb commented Feb 15, 2014

Hi again,

  • For my project I used android.R.layout.simple_list_item_1 as view for the adapter, and use OnClickListener to get selection :) Maybe in the future I will implement multi choice also, but to be honest I was thinking to some refactoring, and follow more closely the implementation from the Android SDK.
  • You are right about the dialog title, I just copied and pasted and didn't change the title. My bad.

Regards,
Edy

@eduardb
Copy link
Author

eduardb commented Feb 15, 2014

I made the required updates (dialog title, positive and negative button, which works out of the box), plus I added implementation for the function overloads, although not exemplified in test activity.
Multi choice probably coming in the next 2 weeks :)

@dentex
Copy link
Owner

dentex commented Feb 15, 2014

Hi.
Thanks for the changes. I'll review everything ASAP! (almost nighttime here) :)

dentex added a commit that referenced this pull request Feb 16, 2014
@dentex dentex merged commit 4ad8888 into dentex:master Feb 16, 2014
@dentex
Copy link
Owner

dentex commented Feb 16, 2014

Thank you very much!
:)

@eduardb
Copy link
Author

eduardb commented Feb 17, 2014

You're welcome!

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