Refactoring applied offline branch

[tormodh]

Hi,

Did some additional changes to my refactor based on feedback from Tim

The changes are in the branch 43-further-refactoring

This pull request does the necessary merging to get this and Tim's changes to TaskBag preferences + the "workofflinepref" bugfix into one pull.

In addition I set up so that local changes will be pushed to remote/dropbox when leaving offline mode.

Also removed the purging of local storage when signing out of dropbox, as per suggestion from Tim.

Sorry about having more than one commit here, but I felt it was needed to separate issues.

[tormodh]

That is; if the offline-use branch is going into master soon, use this pull request. If the offline-use branch is going to wait for a week or so, please pull the branch named 43-further-refactoring.

Thanks :)

[timbarlotta]

Thanks for pushing this together into one commit. I'll try to take a closer look later today.

[ginatrapani]

Updates from both of you looking good on my end. Tim, thanks for the preferences work. Tormod, thank you for adding that offline click push. Tim I know you want to review Tormod's updates based on your comments, but in the meantime I added to the work offline setting work (hiding online menu items/action bar sync buttons when it's enabled), and pushed to master.

Let's continue to review and test. I won't release a new version until all of us feel confident about what we have, even if that means releasing later this week or next weekend.

[tormodh]

I'll spend some time tonight and/or tomorrow - that is today and/or tonight for you - looking at how to deal with starting without internet connection and losing connection. I had some code for that at some point, but it got lost in time, or something.

I think Tim and I are agreeing on how things should be, but not necessarily how they should be while we figure out the perfect architecture. Or at least I mostly agree with him :)

I'll keep working off of this (offline-use) branch on offline stuff in the mean time, then. Apart from handling loss of / no connection - what more needs be done?

[ginatrapani]

I'll keep working off of this (offline-use) branch on offline stuff in the mean time, then. Apart from handling loss of / no connection - what more needs be done?

Excellent. The latest code is all in my master branch now, so be sure to work from that. Dealing with the loss of and no connection circumstances are important.

In terms of the work offline feature, there's only one thing that needs to be done: Now that the "Work offline" setting exists, when you first install the app or log out of Dropbox, the welcome screen should have TWO buttons: Log into Dropbox, and a new one, Work Offline.

Eventually, I'd like to add a new syncing method preference in the Dropbox settings. One is "Sync automatically on every change" (as syncing works now) and the other is "Sync manually." The manual sync setting will let you update the todo.txt file locally without pushing every time--the user will have to tap the sync button to push changes up to Dropbox.

[tormodh]

In terms of the work offline feature, there's only one thing that needs to be done: Now that the "Work offline" setting exists, when you first install the app or log out of Dropbox, the welcome screen should have TWO buttons: Log into Dropbox, and a new one, Work Offline.

This would be impractical. Working offline implies working offline from something. The welcome screen shows only if the user isn't authenticated with the currently selected remote service (i.e. dropbox). If the user should be able to freely switch between local-only and different remote services, this should be made available with a "select remote service" preference / choice at the welcome screen. For that to work we should have a (as said before) a "LocalOnly" remote implementation in addition to the remote services.

Eventually, I'd like to add a new syncing method preference in the Dropbox settings. One is "Sync automatically on every change" (as syncing works now) and the other is "Sync manually." The manual sync setting will let you update the todo.txt file locally without pushing every time--the user will have to tap the sync button to push changes up to Dropbox.

This is how it works with the "work offline" preference now, except that push to remote happens when you turn it off.

Here we must also think out a nice way of determining when to push and when to pull. After working on the computer and firing up the phone in "manual sync" mode, there must be an easy way to initiate a pull. A push would overwrite the changes in the remote file. I mentioned something about this in an email to the mailing list a bit ago. ( here )

[ginatrapani]

If the user should be able to freely switch between local-only and different remote services, this should be made available with a "select remote service" preference / choice at the welcome screen. For that to work we should have a (as said before) a "LocalOnly" remote implementation

Yep, this is the long-term solution. I was thinking that in the short-term (ie this week's release), we'd just use the Work Offline preference to handle the local-only option.

This is how it works with the "work offline" preference now, except that push to remote happens when you turn it off.

It doesn't anymore. The sync option is not available when work offline is set. I want to make a distinction between work offline (phone-only) and manual sync.

One of the repetitive complaints we get about the app is that logging into Dropbox is required to use it, so with this Work Offline release, I was hoping to lift that requirement so you can just use the app as a phone-only list app.

[timbarlotta]

I believe that tormod's last commit will cause a push to remote (dropbox) if you disable working offline (go online).

See line 222 of TodoTxtTouch:
taskBag.pushToRemote();

[ginatrapani]

I believe that tormod's last commit will cause a push to remote (dropbox) if you disable working offline (go online).

Yes it does, and I like it (except for the fact that it can take some time and it's not clear to the user what's happening, we could use a toast or spinner there to indicate the push is happening).

I think that should stay as is, as long as you're authenticated. If you're not authenticated, when you go to the task list, you should get the prompt to log in or work offline.

[tormodh]

Yes, Tim is right here. I made that change because that fit my need. I needed to be connected, go offline, do local changes, turn off offline (which pushes changes to dropbox) to sync.

It would be easier to make a Local only "Remote" client that is selected by another preference in RemoteFactory when starting the app for local only changes. That would be accomplished by having "Log In Locally" set a preference, say "remote_client" to "local" and Log In with Dropbox setting it to "dropbox". Then TaskBag would need to have a setter for RemoteTaskRepository, and the preference screen should have "Log out of Local Only" instead of "Log out of Dropbox".

This is probably possible to whip up fairly quickly with low risk involved.

[timbarlotta]

I like the push when going back online as well. I think I am misreading your (Gina) comment and thought you were saying it does not push it to remote when coming back online...

[tormodh]

I read it the same way as Tim did :)

And I still think we can have both offline (which should be offline while authenticated with a remote service) and Local Only which should be two different things.

[ginatrapani]

Oh sorry, when I said "It doesn't (work that way) anymore" I meant that you can't manually sync when work offline is checked, because the sync buttons are now hidden. (I did that in my latest commit.) But yes, the push does happen when you uncheck Work Offline. We just need some sort of a progress indicator there.

[tormodh]

Progress indicators and toasts disappeared because RemoteTaskRepository no longer had anything to do with any UI elements.

Suggest fixing a Progress thing in TodoTxtTouch#onSharedPreferencesChanged

[ginatrapani]

Okay, Tormod I see what you're saying, and I think we're all on the same page:

Dropbox/Work Offline will be the same as manual sync--you have to tap the sync buttons to push. (I need to undo my last commit which hides sync buttons when work offline is checked.)

Separately, there will be a phone-only remote client, which will give the user the ability to not log into any cloud service at all and just update files on the phone.

[tormodh]

Except that we're really not ready for manual sync yet. Determining if we should pull or push on sync is a real issue. Pushing to remote when leaving offline mode is something different than syncing manually. Syncing implies both pulling and pushing changes.

[timbarlotta]

I don't think you need to get rid of your last commit...

1. if offline, when you come back online it will push to dropbox overwriting any settings - "sync" button is disabled
2. if online, writes to dropbox on all changes and pulls from dropbox if the "sync" button is tapped

[timbarlotta]

Except that we're really not ready for manual sync yet. Determining if we should pull or push on sync is a real issue. Pushing to remote when leaving offline mode is something different than syncing manually. Syncing implies both pulling and pushing changes.

I agree. I think we can avoid the push/pull question this release if we leave Gina's disable the "sync" button (pull button) in for this release. We just need to make sure that folks know not to change the dropbox version and the mobile version at the same time. Mobile will always overwrite when it comes back online.

[tormodh]

I don't think you need to get rid of your last commit...

Agree with Tim on both points. Sync button can stay away. Possibly replace with a "leave offline mode" button?

[ginatrapani]

Okay, agreed. Will leave as is, with sync buttons invisible when offline.

Instead of "leave offline mode," maybe we could replace the action bar sync button with an "upload/push changes" button? Need to find a good icon for that.

[timbarlotta]

Other change I'd like to make is to move the dropbox.* classes into the remote package and make them package-private. I don't see a real need to have sub-package per client. Thoughts?

[tormodh]

No, that's ok for me, also said so in your review

[timbarlotta]

I did a number of changes but couldn't get all the way through see this branch. (Still a work in progress but maybe someone else can see it home -I need to get to bed)

https://github.com/timbarlotta/todo.txt-touch/tree/tim-offline-use

[ginatrapani]

Alright gentlemen, I've been testing what we have so far all day and even without the full local-only functionality that Tim's working on, we're at a stable enough place to release. Just the refactoring, plus Work Offline from Dropbox and another "Show task age" feature I added today is worth a release.

I'm going to get that out tonight before I have to head back to work tomorrow.

We'll get completely local-only done next cycle, and continue the work Tim started in his tim-offline-use branch.

Thanks to both of you for all your time and effort this weekend. The app is going to be much more usable for all of us!