Review and merge branch Jason has - many fixes/features in it

[GoogleCodeExporter]

See branches, and threads on issues.

Original issue reported on code.google.com by charlie....@gmail.com on 2 Jul 2010 at 3:12

[GoogleCodeExporter]

I took a quick look at this today and borrowed a few things from it when I 
created the "notes" section using a SlidingDrawer. Still, didn't really have 
time to properly review this (quick glance I would change the way the tags 
cursor is used directly in an Activity, and I had a few bugs with 
adding/removing tags when I used it - errors and or stuck in places - still, 
haven't reviewed properly so take this with grain of salt, still need to 
review). (Really appreciate the contribution too, added you to "developers" 
attribute in about->details.)

Original comment by charlie....@gmail.com on 25 Jul 2010 at 6:40

[GoogleCodeExporter]

Yeah the currently delivered version has a serious problem with the adding tags 
from the tag batch selection screen, that I think I've got fixed in my local 
copy. Or were you seeing issues with the tag selector in BookDetail? I haven't 
seen any issues there, but there's always the problem when testing your own 
code, that you don't test it in unexpected ways. Let me know what you saw and 
I'll try to reproduce and fix it.
For the tags cursor, were you thinking of dumping it to a list from the DAO and 
then selecting on the list? I can see how this would improve DB encapsulation 
and it should be a relatively easy refactoring. Or did you have something else 
entirely in mind?

Original comment by JasonLiick@gmail.com on 25 Jul 2010 at 6:58

[GoogleCodeExporter]

I'm making a point to stop being a lazy (or otherwise busy) jackass and doing a 
real review of this today, now. I'll report back here in a while after having 
looked over the branch more closely, and hopefully we can integrate today too. 
(I have some time today.)

Original comment by charlie....@gmail.com on 1 Aug 2010 at 1:04

[GoogleCodeExporter]

First, the new TagDAO stuff is exactly what I was talking about, and more, 
great job there. I am changing the DAO interface to require the onCreate and 
onDelete stuff like TagDAO. I'll refactor the other DAOs in the branch to do 
the same (good pattern).

Original comment by charlie....@gmail.com on 1 Aug 2010 at 1:11

[GoogleCodeExporter]

Ok, all in all I really like the tags stuff after looking more closely at it 
today and reviewing the code. 

1. I like the new TagDAO and the structure of tags and join table, etc, nice 
work. 

2. My understanding of the Tag UI is - from Main menu->tags takes me to "Tag 
Batch Editor" which is nice, clean way to multi assoc books and tags. But, it's 
sort of confusing to know what this page is at first (I had to look at code).

2.5 I don't get the tag templates.

3. From Tag Batch Editor I can use action bar to Edit or Add a tag, or Filter 
or Sort list. Once I add a tag, or edit one, probably should auto take me back 
to the list (now stays on that activity). 

4. From Tag Batch Editor I don't understand Filter vs Sort. I see comments in 
code and get the String.format part, but I don't really see what else it is 
getting me (unless the filter is dynamic, but doesn't seem to be?). Also, if 
filter is better (and I just don't get it, which is probably the case), should 
we use just filter and dump the sort? 

5. Once I have books and tags setup, I don't understand how to use them from 
the main activity (not from Tag Batch Editor, but from Main). At that point we 
need to be able to sort and filter THERE by the tags, right? 

5.5. I like the SlidingDrawer with info. We need to incorporate notes there 
too, maybe we can do it in one screen, or maybe we can have multiple slide outs?

6. Also minor stuff, I would like to put the "tag batch edit" icon on the Main 
task bar, rather than in the menu. And, there are a few other minor UI things 
like spacing on tag dialog on "Select Tag" dialog (different than all other 
dialogs). 

That's what I see for now, but I'd like to get this stuff worked in to the 
trunk. Most of the review is minor, and or things I maybe don't understand. I'm 
creating a new branch now where I can work on this branch (so as not to mess 
with your stuff, and not to muck with trunk, yet). 

Original comment by charlie....@gmail.com on 1 Aug 2010 at 3:36

[GoogleCodeExporter]

One other thing, we need to make sure the CSV import/export works with tags. 

Original comment by charlie....@gmail.com on 1 Aug 2010 at 3:43

[GoogleCodeExporter]

Glad you like it. I'll try to take your comments one at a time:

2. I've been trying to figure out a good way to make the batch editor more 
intuitive and discoverable. One thing I definitately need to get working is the 
default text in the spinner to tell the user what its for. Another possibilty 
is to add more info to the title bar such as "Bookworm - Check books linked to 
selected tag", but more consise. Any suggestions? 
Some of this can be dealt with with documentation such as the details screen or 
wiki, but I'd much prefer it to be obvious how it's meant to be used.

2.5 The intent of the tag templates is to allow the quick, consistent  creation 
of common tags; for now this includes genres and series. The template is 
automatically prepended to the tag text. I could add a short explanation on the 
editor page, or maybe change default text to say "Prepend template text..." 
instead of "No template".

3.I've also been planning to close the tag editor on save as you suggest, 
although I'm currently torn between this and saving on back and converting the 
button to cancel.

4. I'm not sure I understand the confusion on filtering: it's a dynamic filter 
matching whatever is typed on the keyboard, but this part is the same as the 
pre-existing filter on the main screen. The only difference is that you can now 
select the field to filter on and unlike the main screen you can undo the 
filter. I think we'd want to keep both sort and filter since they are 
complementary functions. 
There are also some filter items I haven't added yet for the tags screen just 
to make it easier to reorder tags (you can see some of this in the code, but it 
isn't used used yet).

5. Right now tags have no effect on the main screen,  but once I add the filter 
there as well, I want to replace the "read" and ratings field with whatever the 
sort & filter keys are currently set to. There are issues with this idea with 
things like long or multiple tags. 
The main place to view tags would be the BookDetail screen, where all tags 
linked to a single book are displayed. This is also where the tags for a single 
book can be edited. I'm not sure from your comments, but have you looked at the 
single book tag editor from the BookDetail menu?

5.5 As for combining your notes entry with the details, I like the idea, but 
I'm starting to toy with the idea of going further: a single scrollable book 
form with all book info, perhaps with a smaller, fixed portion similar to the 
current BookDetail activity. Another part of this same idea would be horizontal 
swipes to select next/previous books in the list.

6. I agree about putting the tag button on main task bar: I was thinking about 
swapping it with the import utilities button, but I wanted to get your opinion 
before reworking the main screen. At the same time I wanted to update the main 
sort to use the new reusable mechanism I've got for tags and add filtering to 
the main screen. However to fit this in for portrait mode, I think I need to 
complete the one-button add-book mechanism I described in a previous email.

For the spacing, are you referring to the select tags dialog on the batch form? 
If so, I had some problems with the layout of the spinner that you might be 
able help with: if I use the default settings the dialog looks right, but the 
spinner button is huge; if I adjust some parameters (I think it was textsize) 
the spinner button is fine, but the dialog is compressed.

As for the CSV import and export, you're absolutely right: I've been trying to 
do this as small, discrete changes where every change is self-contained, but at 
the feature level, tags are not yet complete. Basically, I'm trying to avoid 
the big-bang integration problem.

Original comment by JasonLiick@gmail.com on 1 Aug 2010 at 8:31

[GoogleCodeExporter]

General comments, I totally agree about making the detail screen 
larger/scrollable. I've been thinking about that too. 

As for tags, I think the data structures are right on (but we need to deal with 
migration from existing DBs, import/export, and if users import and older 
entire DB file it blows up the app -- we might be able to just stop doing that 
though, take it out of the menu, make them use CSV import/export). Beyond the 
data structures though I see them as useful for sorting/filtering the main 
list. I don't care so much about seeing the tags on the detail screen (though 
yes they should be there somehwere), but more about using them for 
organization. 

Also, the main listview in trunk has a filter already, just type, it filters. 
This is done with implements FilterQueryProvider on the Adapter. It only uses 
title, but has a filter. Having an explicit Filter like you have is good, but 
maybe it could go through the interface and allow you to select a tag, or book 
attribute (maybe it does that already, just typing aloud here, but filter by 
tag OR TAGS would be good). 

I'm thinking about this more and trying some stuff out in my branch. 

Original comment by charlie....@gmail.com on 1 Aug 2010 at 9:48

[GoogleCodeExporter]

For DB migration, I've been paying close attention in the DB update functions 
to make sure that older DB's are updated to use the tags with no loss of data. 
This includes the current change I'm working on which migrates the "have read" 
field from bookuserdata to a built in "read" tag. This will leave the "have 
read" field in  bookuserdata, but since sqlite stores the DB in binary format I 
wouldn't expect it to waste much space for this.
I haven't fully figured out the CSV structure for tags, since the number of 
tags per book is variable. This may require another CSV file, but that has 
issues with making sure the tag and book file are for the same DB. Let me know 
if you have any thoughts about how to implement this.
Now that I think about, once there are several versions of the app out with 
several incompatible databases, I'd be more worried about someone trying to 
import a database that is new than their copy of BookWorm supports. It might be 
a good idea to add a warning on import if this is detected, but I think that's 
about the most we can do.

I think I understand the confusion about my filtering button now: it 
dynamically updates the filter used for the table, but does not apply the 
current filter text when changed. This means that if I select "by title" and 
enter a filter, it will be filtered by title using that text, but if then 
select "by author" it won't refilter using the same text, but will filter by 
author as soon as I start typing again. I was confused by your question because 
I did base this off of your code for main, I just extended it to be 
configurable. I hope that clears things up.

I don't have a filter by tag working yet, since it will be another complex tag 
like authors, but this is something I'm working on since it is critical for 
doing series order (what I originally started playing with tags to do). Once 
this is ready the same builder I use for my sort and filter on the tags batch 
screen can be added to the main screen with almost no effort which should give 
the organization capability you're talking about. I don't know exactly when 
I'll be adding this since I want to finish my current change and add some sort 
& filter testcases first.

Original comment by JasonLiick@gmail.com on 1 Aug 2010 at 11:26

[GoogleCodeExporter]

Sounds good, well, other than the test case, not sure we want to set that 
precedent ;).  (Kidding, I have been meaning to add many tests, just haven't 
done so, it's not all that convenient with Android, but still, I should be 
doing it, even for a small project like this.)

Original comment by charlie....@gmail.com on 1 Aug 2010 at 11:47

[GoogleCodeExporter]

Also, I can do the CSV stuff with tags, I just use one field for it and 
separate them with pipe, sort of like I do authors. I'll get that once 
everything else is ready to go. Also, as to DB changes, I did see your 
onUpgrade stuff, and that should work, but I meant if a user imports an entire 
old DB from the SD card on top of a newer version of the app (then they won't 
have tag tables). That blows up. I think we might be able to just remove that 
option though, so long as the CSV stuff is very robust (that was there before 
the CSV stuff, it was the only backup, now it's not so critical and it will 
cause us headaches -- we could put a version field IN the db, and detect before 
we use it for anything, but that is painful). 

Original comment by charlie....@gmail.com on 1 Aug 2010 at 11:49

[GoogleCodeExporter]

Ah, I didn't follow that before since I hadn't looked that closely at your 
import functionality: I figured that when you reopen the DB, the update would 
be called and take care of this. 

Original comment by JasonLiick@gmail.com on 1 Aug 2010 at 11:54

[GoogleCodeExporter]

I'm almost finished my current update which replaces "Have Read?" with a 
built-in tag. However as usual one thing leads to another and once I changed 
the display of the tag on the BookDetail screen, I ended up needing to re-work 
the entire screen and ended up adding the scrolling functionality we discussed. 
I've attached a composite screen shot showing all the fields: the scroll pane 
starts below the cover image in portrait and to the right of it in landscape.

I've made some fairly drastic changes here so I wanted to get your opinion 
before I commit anything.
I've reduced the font size of the title and hidden the sub-title if it's blank, 
to make more room for the scroll box. There's still enough room to use it with 
sub-titles or long titles, but it's easier to read with more room.
I've completely gotten rid of the slider drawer, which was mostly a way to 
avoid changing existing screens and my orginal idea of making it look like a 
turning page never worked out anyway.
One thing I'm not sure about was the idea to use buttons for the user tags and 
blurb titles to declutter the screen. The user tags button pops up the selector 
that was previously accessed from the menu and the user blurb button pops up a 
text editor dialog (I tried to reuse you blurb editor, but the upper image pane 
forces the blurb text to be on the lower half of the screen which is covered 
when the soft keyboard is opened in portrait mode). I like the look of having 
the section title in the button rather than as separate title and buttons and I 
think that the functionality is easily discoverable if not exactly obvious, but 
this may just be the usual problem in design where I already know how it's 
meant to be used. What do you think here?
I've considered adding section separator lines as well, but I think the whole 
thing looks cleaner without.

I've also added a popup zoomed image of the cover when it's touched that just 
re-uses the same image to avoid bloating the image folder.

I've got some more work to do before I submit, but I'd appreciate any comments 
you have on this.

Original comment by JasonLiick@gmail.com on 3 Aug 2010 at 4:54

Attachments:

• bookDetail_overview.gif

[GoogleCodeExporter]

That looks damn good, and I was thinking along the same lines. 

I also was thinking of coming up with a separator (maybe the red) that we could 
use and then just have different "groups" of data. The main book info (with 
pages, description, and format), the user book info (with read, rating, tags), 
and maybe separate for blurb. I don't have any set in stone ideas, just 
throwing it out, either way I think it's better than the limited screen we have 
now, and once we get all the data on the screen, and the UI metaphors for using 
tags down, we can always later tweak the design. I agree with you too about the 
buttons, not sure about that, but I'd really have to use it to get a feel for 
it.

If you get it set in the branch let me know and I'll run it and mess with it.

Original comment by charlie....@gmail.com on 3 Aug 2010 at 5:12

[GoogleCodeExporter]

Thanks for the quick response. I'm glad you like it.
I'm just tweaking some inconsistencies between Android 1.6 and 2.2 and I'm 
going to try to convert the description from HTML since the tags aren't handled 
by TextView.
I should be ready to commit it in the next hour or so. 

Original comment by JasonLiick@gmail.com on 3 Aug 2010 at 5:57