tomboy-notes / tomdroid

Tomboy notes on Android
https://play.google.com/store/apps/details?id=org.tomdroid
GNU Lesser General Public License v2.1
18 stars 14 forks source link

Create a interface for the web synchronization code #74

Closed obilodeau closed 8 years ago

obilodeau commented 8 years ago

The current code doesn't provide many cues about what's happening behind the scenes and may confuse the user. It could use some work in order to be able to tell the user about the state of the authentication/sync.

It could be divided in two subtasks:


Imported from Launchpad using lp2gh.

obilodeau commented 8 years ago

(by trappe) I've done some work in the lp:~trappe/tomdroid/sync-ui branch. It's not all done but you can see where I've been hadded.

obilodeau commented 8 years ago

(by benoit.garret) Yep, I saw you linked this bug to your branch. I won't have time to take a look at your work this week, but I'll try to check this out this weekend and give you some feedback. Thanks a lot for tackling this!

obilodeau commented 8 years ago

(by benoit.garret) Sorry for the delay. I played a bit with your code, it looks quite nice and unobstrusive (which is good!).

I have a few nitpick though, from a user perspective as I haven't looked at the code:

You may already be aware of these but I don't know if you're planning to do further work on this, so I'm throwing them here just in case.

And again nice work, I'm really looking forward to merge your work.

obilodeau commented 8 years ago

(by trappe) Thank you for the review! I just worked on some of your valuable comments.

  1. sd card syncing does now send correct sync progress infos to the UI
  2. I've now added a pulsing dot when sync is in progress to have an indication when using a slow connection. I decided against constant rotation because I like to have the angle of the sync icon to sbtile indicate the progress (just started, half done, two-third done, ...). Unfortunately my sync-Icon is not perfectly centered so the dot is not always in the center. Maybe I can fix this in a few days.

The sync button in the Actionbar is already deactivated when sync is in progress. I would suggest to remove the sync-menu icon completly in favor for the action bar.

obilodeau commented 8 years ago

(by benoit.garret) Much better, I can now at least see that something is happening.

I took a quick look at your animation code, but couldn't spot any obvious mistake. I'm afraid I won't be of much help here.

I'm against removing the sync menu icon for two reasons:

Feel free to disagree though ;-) . I may not have completely understood your reasons and I would gladly be proven wrong.

One last thing, I'm curious about the reasons that led you to use a custom notification. There are at least two feedback mechanisms that are already implemented by system applications:

I'm not trying to downplay your work, I'm really, really glad that you stepped up to get in charge of this. I'm just trying to get an understanding of the reasons that led you to your UI choices.

obilodeau commented 8 years ago

(by trappe) I've now fixed the animation problem and decided to keep the additional sync menu button for now. It was'nt too difficult to make it work correctly.

About your arguments to keep the menu item:

  1. Accessibility The menu items are also only accessable via touch. There may be a hardware button to launch the menu, but the menu it self is always represended on the display.
  2. Follow the guidelines Me too! Have you seen the recent blog article about the Twitter App: http://android-developers.blogspot.com/2010/05/twitter-for-android-closer-look-at.html ? Google recommends to use the Action bar for common and often used global actions. See http://code.google.com/intl/de-DE/events/io/2010/sessions/android-ui-design-patterns.html at minute 12:40 for the whole Action bar talking, and note at 14:35: "What are the actions the user should not have to press menu to get at".

About my decision to use a custom notification:

Using the standard progress bar would mean to have some dedicated UI real estade whre it could be placed. I think it's not worth the space plus it would be decupled from the sync button and hence unclear to the user how synchronisation and a orange bar somewhere else in the UI are related.

Status bar notifications are fine and I think we should implement it sometime. Currently we do not have a background service and I think this a essential precondition.

obilodeau commented 8 years ago

(by sanfordarmstrong) On Fri, Jun 11, 2010 at 1:18 AM, Rodja

  1. Accessibility The menu items are also only accessable via touch. There may be a hardware button to launch the menu, but the menu it self is always represended on the display.

Once the menu button appears, it is accessible via the scroll ball (or whatever navigational hardware is on the device).

obilodeau commented 8 years ago

(by benoit.garret) If the Google designers say it's allowed, then who am I to question this? ;-) . On a more serious note, I agree with your arguments against the window progress bar. I would say your work is ready to be merged in a very near future, please request a merge when you think your work is finished.

I checked the Twitter app and the item in the ActionBar can be used through the trackball. If you manage to do it in Tomdroid, feel free to remove the menu item as it would remove the last objection I had against this.

obilodeau commented 8 years ago

(by trappe) Done!

The focus indication could be better, but for now it's good enough. Also there are some glitches when leaving the activity while sync is in progress. This should be fixed by doing the actual sync in a dedicated Android Service which is tracked in #549646.

obilodeau commented 8 years ago

(by ukev) Hi Rodja,

as in the mailinglist promised, I've tested your code on my nexus one with my ~410 (production) notes and will give you some feedback. I'm really impressed how good it works already.

positive: • Syncronized more than 410 notes in a reasonable time (~10-15 sec. first time sync) • Already really useful, works as expected in all main parts (Hey it does sync my notes to my android phone - really great :)! ) • It didn't delete my notes :)

negative: (only some small fine-tuning related stuff)

Please don't be depressed because my "negative" list has more entrys than my "positive" one. The positive points are the significant ones! Thanks for your code and I'm already pleased to see your next contributions :).

obilodeau commented 8 years ago

(by trappe) Thanks for testing and the good feedback!

  1. sync icon not intuitvely understand as "button" The action bar "design pattern" from Google is very new. After your objection I looked closer to the Twitter App. They are using 1px vertical lines to frame each button in the action bar. Would be great if you can find the time to enhance our action bar with this, too. I've changed the owner of the sync-ui branch to Tomdroid Developers, so you can push your changes right into the branch.
  2. sync button to small + 3. graphics aliased I did bad scaling and pixel pushing with Gimp for the icons as a first shot. Also they are too small and not as high as the action bar itself. They need to be redone properly with inkscape as SVG-graphics. I just filed this as bug #593351 because this will take some time and does not block this ticket in general.
  3. freezing ui when orientation is changed I could not reproduce this behaviour on my DROID, but there is indeed a sync button update issue when orientation is changed. It's not so important for me to invest a lot of time into this right now. Maybe you can have a look...
  4. slow re-sync I do not have as many notes as you have. Maybe it would be good to file a bug for this because I do not think it's caused by the UI changes I introduced.
  5. This should also be a seperate bug. I have not invested a lot of time in the Note view because I guess this part will be modified a log when note editing is implemented.
obilodeau commented 8 years ago

(by obilodeau) Can I mark this as 'fix released'? Can you file valid remaining tasks into separate bugs if any?

Thanks,

obilodeau commented 8 years ago

(by trappe) Ok. Moved all remaining informations to other bugs. I consider this issue as fixed. The 0.4.1 gets great feedback.