Add in capacity for a SortableListPreference #1

kd7uiy · 2013-12-29T22:32:48Z

I've added a new demo which I think will be of some interest. I know I'm planning on using it in my current project a couple of times. It's a SortableListPreference, which I've added to the demo activity. I think it will be of use to others, so I'm handing it back to you with the hopes that you can add it to your project, making it available for others to use as well.

now, the items are displayed correctly, and are sorted, but not saved or loaded correctly.

more descriptive.

preference if not checked.

JayH5 · 2013-12-30T17:36:46Z

Thanks for this.

There is something wrong with the diff, however, possibly due to the edit that you are using doing strange things to whitespace/newlines. See here: http://stackoverflow.com/questions/6134066

Also, there are a few small style fixes I'd like to make.

My branch is just one of many unofficially maintained copies of the original project so I understand if you don't have much reason to put a lot of time and effort into this pull request but I'm willing to work through these changes with you if you want. Let me know what suits you.

kd7uiy · 2013-12-30T18:34:52Z

Jamie,

I'm certainly willing to put the effort into it. I put a fair bit of effort
to add the sortable preference list, as I'm planning on using it for my
project, I feel it's a way to give back to the open source community which
I owe so much to. It seems like yours is one of the more active ones
around. I'm using Windows, I suspect that the original is Unix. I'm going
to try to submit it again and give it a shot. Feel free to many any style
changes as well.

Ben

On Mon, Dec 30, 2013 at 12:36 PM, Jamie Hewland [email protected]:

Thanks for this.

There is something wrong with the diff, however, possibly due to the edit
that you are using doing strange things to whitespace/newlines. See here:
http://stackoverflow.com/questions/6134066

Also, there are a few small style fixes I'd like to make.

My branch is just one of many unofficially maintained copies of the
original project so I understand if you don't have much reason to put a lot
of time and effort into this pull request but I'm willing to work through
these changes with you if you want. Let me know what suits you.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/1#issuecomment-31357387
.

Ben Pearson
http://www.kd7uiy.com

JayH5 · 2013-12-30T19:35:21Z

Mine? More active? Cool.

OK, so a few style notes on the changes overall:

The rest of DSLV uses 4-space tabs. From the new files you've added it looks like you've used 2-space tabs
Be consistent with code block formatting. Put the opening brace on the same line as the starting (if/for) statement, e.g.:

if (true)
{
    do.x();
}

Should be:

if (true) {
    do.x();
}

...also you should avoid if/for statements with no braces as per the Android code style guidelines.

Use consistent spacing around symbols. There should be spacing on either side of all curly braces as well as colons in for loops.

Style tweaks are obviously easier/quicker things for me to spot...actual code fixes take more time...but I will add a few in the diff.

Thanks again.

JayH5 · 2013-12-30T19:35:42Z

library/src/com/mobeta/android/dslv/DragSortListView.java

@@ -640,7 +640,7 @@ public ListAdapter getInputAdapter() {
        }
    }

-    private class AdapterWrapper implements WrapperListAdapter {
+    private class AdapterWrapper extends BaseAdapter implements WrapperListAdapter {


Is there a reason for this change?

This change will prevent a crash in the demo app. Warp just crashes without
it. Looking around, this seemed like the best solution, and it works well,
but if you have another one, I'm opened to it.

Ben.

On Mon, Dec 30, 2013 at 2:35 PM, Jamie Hewland [email protected]:

In library/src/com/mobeta/android/dslv/DragSortListView.java:

@@ -640,7 +640,7 @@ public ListAdapter getInputAdapter() {
}
}

private class AdapterWrapper implements WrapperListAdapter {

private class AdapterWrapper extends BaseAdapter implements WrapperListAdapter {

Is there a reason for this change?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/1/files#r8590130
.

Ben Pearson
http://www.kd7uiy.com

I'm just going to combine the rest of the comments into a single email.

Yes, that blurb of code is required on KitKit, or else it will crash. If
you've seen a rash of releases recently "To fix a crash on KitKat", I bet
that's what it is.

I'm going to keep the exceptions in place, instead of returning -1. I'd
rather thrown an exception in the case that something isn't right, because
it's easier to debug exceptions. And the exception thrown has actually
saved me a few times when debugging... I did change it to
IllegalStateException, per your suggestion, however.

I'm not entirely sure what you meant about the space/tabs. I've
"corrected indentation" on Eclipse, hopefully that clears it up.

No problem being a stickler. Part of the reason I wanted to do this was
to get someone else to look at a major block of code that combines things
that I've never done before, and it helps to have a second set of eyes.
I've rarely had someone who actually was a stickler about code look at
mine, so...

The main setting blurb, was that about the comments there? Essentially,
they came from Android when you create a new project, but I can get rid of
them, if you want. I'm guessing it was the last line, the inherited
document, so I removed that.

As far as your project being one of the more active, well, I admittedly
looked at recent updates to pick one. Still, I'm planning on sticking with
your code base, should I update the code, so...

In addition to your changes, I made a few other changes as well, and tested
it out. Primarily, I ran Lint and cleaned up a few things, including
outside of the code I touched, so that's why a whole bunch of files were
touched. I also found a bug in another class, which caused a crash in
Background Handle. I'm not 100% confident of the fix, but it should do for
now...

Ben

On Mon, Dec 30, 2013 at 4:55 PM, Ben Pearson [email protected] wrote:

This change will prevent a crash in the demo app. Warp just crashes
without it. Looking around, this seemed like the best solution, and it
works well, but if you have another one, I'm opened to it.

Ben.

On Mon, Dec 30, 2013 at 2:35 PM, Jamie Hewland [email protected]:

In library/src/com/mobeta/android/dslv/DragSortListView.java:

@@ -640,7 +640,7 @@ public ListAdapter getInputAdapter() {
}
}

private class AdapterWrapper implements WrapperListAdapter {

private class AdapterWrapper extends BaseAdapter implements WrapperListAdapter {

Is there a reason for this change?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/1/files#r8590130
.

Ben Pearson
http://www.kd7uiy.com

Ben Pearson
http://www.kd7uiy.com

JayH5 · 2013-12-30T20:14:13Z

Apologies for being kind of a stickler...

Handle, and probably other classes that extend DSLVFragment.

kd7uiy · 2013-12-30T23:13:05Z

The change to the core library prevents a crash. As far as other changes:

Yes, that blurb of code is required on KitKit, or else it will crash. If you've seen a rash of releases recently "To fix a crash on KitKat", I bet that's what it is.
I'm going to keep the exceptions in place, instead of returning -1. I'd rather thrown an exception in the case that something isn't right, because it's easier to debug exceptions. And the exception thrown has actually saved me a few times when debugging... I did change it to IllegalStateException, per your suggestion, however.
I'm not entirely sure what you meant about the space/tabs. I've "corrected indentation" on Eclipse, hopefully that clears it up.
No problem being a stickler. Part of the reason I wanted to do this was to get someone else to look at a major block of code that combines things that I've never done before, and it helps to have a second set of eyes. I've rarely had someone who actually was a stickler about code look at mine, so...
The main setting blurb, was that about the comments there? Essentially, they came from Android when you create a new project, but I can get rid of them, if you want. I'm guessing it was the last line, the inherited document, so I removed that.
As far as your project being one of the more active, well, I admittedly looked at recent updates to pick one. Still, I'm planning on sticking with your code base, should I update the code, so...

In addition to your changes, I made a few other changes as well, and tested it out. Primarily, I ran Lint and cleaned up a few things, including outside of the code I touched, so that's why a whole bunch of files were touched. I also found a bug in another class, which caused a crash in Background Handle. I'm not 100% confident of the fix, but it should do for now...

JayH5 · 2013-12-31T05:56:34Z

Cool, thanks this is looking good. I won't have time to look at this properly until probably late in the day tomorrow.
Have a good New Year's 🎉

kd7uiy · 2014-01-10T20:26:22Z

Just curious if you've gotten to this anytime recently. Hope you had a good New Years!

JayH5 · 2014-01-11T15:39:26Z

demo/src/com/mobeta/android/demodslv/SortableListPreference.java

@@ -0,0 +1,271 @@
+/* 


Surely this should be in the library and not the demo app?

kd7uiy · 2014-01-13T20:06:11Z

I can go ahead and make the move, I think you are right that it fits well as a library application. I'll do that shortly when I get a chance, hopefully this week.

kd7uiy · 2014-01-26T23:35:48Z

To put this in the main library will require more work that I'm going to be able to do for a while. Specifically, it needs to get references for the list item, and text handles, and while I'm confident I could do it, it'd take a while.

Bottom line is, it works now as a reference, and if I find the time sometime soon I'll try and get it ported to the main library. I leave it up to you if you want to include it in your list of demo apps.

Finished the process to convert DragSortListPreference to a part of the main library. It now doesn't depend on external resources, and can be controlled via styleables.

kd7uiy · 2014-03-06T22:35:05Z

Had the time to finish it now, I now have DragSortListPreference, and an example of how to use it. The documentation should probably still be updated, but overall, it looks like it'll work.

JayH5 · 2014-03-07T11:01:06Z

Thanks, looking good 👍

I'll review this properly over the weekend. Just two things before that:

Can you make sure you use unix-style line endings (or get your git to convert them for you).
This needs a bit of git work. Can you squash your 12 commits into 1? I think this also needs to be rebased against my fork as I've made some changes since this pull request was first opened.

…ception

Fix bug where an empty preference set would lead to an IllegalStateException

- Add check to onScroll in DragSortController in response to instances seen when these events were null.

Update to gradle 1.12 and update build tools [build]

kd7uiy added 7 commits December 28, 2013 13:30

This fixes a bug so the Warp demonstration works (Fast Scrolling)

dcbe9bb

This is the start of a SortableListPreference class. As it stands right

565bfc1

now, the items are displayed correctly, and are sorted, but not saved or loaded correctly.

Preferences now are being preserved correctly. Changes the names to be

3d3a9f8

more descriptive.

Removed debug code.

bd70b3a

Now supports checkable preference lists. The values are not put into the

51f6b66

preference if not checked.

Cleaned up

c214e3b

Removing setPersistant as it causes undesired behavior.

cbdbbc5

Converted to Unix formatting

dc84525

Converted to Unix format

8167405

JayH5 reviewed Dec 30, 2013
View reviewed changes

General code cleanup, Lint fixes, prevented a crash in Background

49fa775

Handle, and probably other classes that extend DSLVFragment.

Removed debug code

21ea943

JayH5 reviewed Jan 11, 2014
View reviewed changes

DragSortListPreference now available

8ebe125

Finished the process to convert DragSortListPreference to a part of the main library. It now doesn't depend on external resources, and can be controlled via styleables.

woudt and others added 8 commits August 9, 2014 22:16

Fix bug where an empty preference set would lead to an IllegalStateEx…

445b538

…ception

Merge pull request #1 from woudt/master

16a1a02

Fix bug where an empty preference set would lead to an IllegalStateException

Update to gradle 1.12 and update build tools [build]

3e24147

Null protect onScroll logic

609b5de

- Add check to onScroll in DragSortController in response to instances seen when these events were null.

Remove unnecessary conditional check.

36a85b9

Update README.md

8cf4619

Add name/version and missing dependency [build]

b20f957

Merge pull request #2 from loeschg/master

d1a14f4

Update to gradle 1.12 and update build tools [build]

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add in capacity for a SortableListPreference #1

Add in capacity for a SortableListPreference #1

kd7uiy commented Dec 29, 2013

JayH5 commented Dec 30, 2013

kd7uiy commented Dec 30, 2013

JayH5 commented Dec 30, 2013

JayH5 Dec 30, 2013

kd7uiy Dec 30, 2013

kd7uiy Dec 30, 2013

JayH5 commented Dec 30, 2013

kd7uiy commented Dec 30, 2013

JayH5 commented Dec 31, 2013

kd7uiy commented Jan 10, 2014

JayH5 Jan 11, 2014

kd7uiy commented Jan 13, 2014

kd7uiy commented Jan 26, 2014

kd7uiy commented Mar 6, 2014

JayH5 commented Mar 7, 2014

Add in capacity for a SortableListPreference #1

Are you sure you want to change the base?

Add in capacity for a SortableListPreference #1

Conversation

kd7uiy commented Dec 29, 2013

JayH5 commented Dec 30, 2013

kd7uiy commented Dec 30, 2013

JayH5 commented Dec 30, 2013

JayH5 Dec 30, 2013

Choose a reason for hiding this comment

kd7uiy Dec 30, 2013

Choose a reason for hiding this comment

kd7uiy Dec 30, 2013

Choose a reason for hiding this comment

JayH5 commented Dec 30, 2013

kd7uiy commented Dec 30, 2013

JayH5 commented Dec 31, 2013

kd7uiy commented Jan 10, 2014

JayH5 Jan 11, 2014

Choose a reason for hiding this comment

kd7uiy commented Jan 13, 2014

kd7uiy commented Jan 26, 2014

kd7uiy commented Mar 6, 2014

JayH5 commented Mar 7, 2014