Code review comment for lp://staging/~tomdroid-dev/tomdroid/sync-ui

Revision history for this message
Olivier Bilodeau (plaxx) wrote :

Hi guys,

I played with the branch for a couple of minutes tonight. Here are my comments:

5) I upgraded and my notes were all still there. However any attempts to open a note resulted in a parsing error. I'm guessing that something changed about the database. If not db then appending note-content tags or not? I would recommend dropping the database on upgrade. It's easy, notes should be on sdcard because sync never existed and nothing should be only in the db since you can't create content yet (oh and we are not 1.0).

6) I'm also unsure about the sync button.. It was not obvious to me at first that I needed to hit that to sync. It's a bit small and the animation for SD card notes was only a dot blinking in the middle. Is that normal? Nothing turning? But this is minor and could be reworked later.

7) Lastly, I'm unsure about the black text on white background for the note list (I haven't tested notes yet since they crash). It is quite the opposite as most android apps (especially the natives one), well.. at least, on my phone (normal android UI). I might revert these changes unless convinced otherwise (or provide an option?).

By the way, I don't want to sound like I'm dumping all this burden on you guys, I'll look into fixing stuff myself but I don't have time now.

I'll be testing more soon. The sync goodness! I can't wait!

review: Needs Fixing

« Back to merge proposal