Casey Yee Reporter
	Description • 11 years ago

After entering credentials for new CalDav account pressing "save" brings you to a "Please wait while your account is being setup" progress screen.  

There should be a way for the user to cancel-out of the setup with a back button.   Currently the user will have to wait for a time-out or kill the app to do so.

	Josh Carpenter [:jcarpenter]
	Comment 1 • 11 years ago

This is important for usability in our constrained-bandwidth target markets. Current implementation is a big papercut in the foyer of the Calendar app experience (setup). Seems relatively low LOE to resolve.

	Gareth Aye [:gaye] (back from PTO)
	Comment 2 • 11 years ago

If LOE means level of effort, then I think I disagree. I tried to write a patch yesterday, but there are a couple of architectural issues that lightsofapollo brought up that I still need to think through...

https://github.com/mozilla-b2g/gaia/pull/8952

On the flip-side, if you were talking about LOE, the village in Estonia, I am in complete agreement. We won't need much of them to get resolution here ;).

	Stephany Wilkes
	Comment 3 • 11 years ago

Hi Mike - Did this make it into 1.2?

	Evan Tseng [:evanxd]
	Comment 4 • 10 years ago

Hi Harly,

Could you give the UX spec for the "back button" on the "Please wait while your account is being setup" page?

Thanks. :)

	Harly Hsu[:harly]
	Comment 5 • 10 years ago

Hi Evan,
Attached is the UX spec for canceling CalDav account setup.
Thanks

	Evan Tseng [:evanxd]
	Comment 6 • 10 years ago

Hi Harly,

Got you.
It is very clear.
Thanks. :)

	Evan Tseng [:evanxd]
	Comment 7 • 10 years ago

Hi Kevin,

I would like to get your feedback for the path,
then I will add the related tests.
Thanks. :)

	Kevin Grandon :kgrandon
	Comment 8 • 10 years ago

Comment on attachment 8382901 [details] [review]
Pull request

As gaye has mentioned, the proper patch for a cancel button is extremely complex. We would need to delete local data, as well as abort in progress XHRs in lower level libraries.

For now I would instead recommend a `hide` button, which would just hide the screen and allow the calendar to still sync. I'm not sure if this is acceptable from a UX point of view though.

	Evan Tseng [:evanxd]
	Comment 9 • 10 years ago

Hi Kevin,

Got you.
I never know the delete local data things before here, learned.
To do that sounds not easy things, just like you said extremely complex.

Currently, I have no idea to deal with the low level libraries.
Do you just mean that it is extremely complex to do aborting the in progress XHR?
Or we also need to do many other things, for example, delete local data, and other things I don't know yet.

If we need to do many things to abort the XHRs, could you give me some tips for that? I would like to try to figure out that first.
Then we might cc UX guys for your suggestions.
Really thanks.

	Kevin Grandon :kgrandon
	Comment 10 • 10 years ago

I don't know all of the details as I haven't thought of this too much, but I did read the conversation at: https://github.com/mozilla-b2g/gaia/pull/8952

I would recommend you, me, gareth, and james get on a vidyo call to figure out the best way forward here and share knowledge. Thanks!

	Evan Tseng [:evanxd]
	Comment 11 • 10 years ago

Hi Kevin,

Thanks for the suggestions.
Before the meeting, I should learn and make sure something here.
I'm doing needinfo to James and Gareth.

	Evan Tseng [:evanxd]
	Comment 12 • 10 years ago

Hi Gareth,

For this issue, I have a question.

In the https://github.com/mozilla-b2g/gaia/pull/8952 pull request,
why did just stop to do updating the patch on that time?
Did we meet some big problem is very hard to resolve, or some UX issues?

	Gareth Aye [:gaye] (back from PTO)
	Comment 13 • 10 years ago

Lots of code complexity!

	Evan Tseng [:evanxd]
	Comment 14 • 10 years ago

Comment on attachment 8382901 [details] [review]
Pull request

Hi all,

So how about we do this way https://github.com/mozilla-b2g/gaia/pull/16702/files#diff-dc01c7982a21f4e3a531d2c8574a05b5R154?
Please give me your feedback.
Thanks.

	Evan Tseng [:evanxd]
	Comment 15 • 10 years ago

I think I should say that could you guys give me some feedback here.
Thanks.

	Kevin Grandon :kgrandon
	Comment 16 • 10 years ago

Comment on attachment 8382901 [details] [review]
Pull request

It looks like James left some feedback on github which I generally agree with. If I can think of anything else I will leave a comment. Thanks!

	Evan Tseng [:evanxd]
	Comment 17 • 10 years ago

Hi James,

For the id things in https://github.com/mozilla-b2g/gaia/pull/16702/files#diff-dc01c7982a21f4e3a531d2c8574a05b5R154,
I think we could generate the id from at the `getAccount` in https://github.com/mozilla-b2g/gaia/blob/master/apps/calendar/js/provider/caldav.js#L175 and return the id to the function caller.
Then we could cancel the xhr request by the id.

How do you think?

	Evan Tseng [:evanxd]
	Comment 18 • 10 years ago

Hi James,

Sorry, and one more thing.
Why do we need to use worker thread for the CalDAV things?
Does it do a lot of heavy computing?

Thanks.

	Evan Tseng [:evanxd]
	Comment 19 • 10 years ago

Hi James,

For the Comment 17, the implementation is like here https://github.com/mozilla-b2g/gaia/pull/16702/files#diff-bc1a6a7ca2a79166302cb3fad1ff0358R181.

How do you think?
Thanks. :)

	Evan Tseng [:evanxd]
	Comment 20 • 10 years ago

Comment on attachment 8382901 [details] [review]
Pull request

Hi James,

Could you help me review the patch.
Thanks. :)

Evan

	Kevin Grandon :kgrandon
	Comment 21 • 10 years ago

Comment on attachment 8382901 [details] [review]
Pull request

I am happy to provide feedback, but I think Gareth or James should do the full review as they have more experience looking into this. Thanks for sticking with the patch!

	Evan Tseng [:evanxd]
	Comment 22 • 10 years ago

Hi Kevin,

Thanks for your feedback.
I already updated the patch for your comments.

	Gareth Aye [:gaye] (back from PTO)
	Comment 23 • 10 years ago

Comment on attachment 8382901 [details] [review]
Pull request

I want to work toward a place where the calendar backend and frontend are less tightly coupled. In its current state, this patch adds request id generation and knowledge of the caldav backend to our frontend webapp. We also need integration tests including (but not limited to) the cases where

- When we abort before we get the entire propfind response, all of the xhr requests should be aborted and no account data should be written to idb.
- When we don't abort, the cancel ui should disappear once we finish the calendar home request and before we begin calendar sync

It seems as if, in its current state, there are times at which the user could press the cancel button, but their accounts would still be setup. Also in https://github.com/mozilla-b2g/caldav/blob/master/lib/caldav/request/abstract.js#L59, if you abort the xhr request, will the callback that closes the xml parser be issued? If not, I think we could get bad state in the xml parser perhaps?

Like I mentioned above, this is a small feature, but there are a lot of ways that this could go wrong and confuse users.

	Evan Tseng [:evanxd]
	Comment 24 • 10 years ago

patch 1.1: https://github.com/evanxd/gaia/commit/eb1b126d71f32844ef32e04b59cf42f954dfa569

	Evan Tseng [:evanxd]
	Comment 25 • 10 years ago

Hi Gareth,

Let us focus the patch first, then we could do the little feature in Comment 23.

To abort request,
how do you think we do that with this way at https://github.com/mozilla-b2g/gaia/pull/16702/files#r10612182?

Actually, I don't understand all of you comments on GitHub,
and I also left some comments for that.

As you said, it is not an easy bug.
Some complex and risky things might happen in the patch.
So how about that we could do the simple and clear way to fix this bug?
(like the https://github.com/evanxd/gaia/commit/eb1b126d71f32844ef32e04b59cf42f954dfa569 patch)

Or we might need to discuss the details of you idea with vidyo.
I really would like do this bug well.
How do you think?
Hope hear your response.
Thanks.

	Evan Tseng [:evanxd]
	Comment 26 • 10 years ago

update the Comment 25:

To abort request,
how do you think we do that with this way at https://github.com/mozilla-b2g/gaia/pull/16702/files#diff-bc1a6a7ca2a79166302cb3fad1ff0358R211?

	Evan Tseng [:evanxd]
	Comment 27 • 10 years ago

Comment on attachment 8382901 [details] [review]
Pull request

Hi Gareth,

I add RequestGroup at https://github.com/mozilla-b2g/gaia/pull/16702/files#diff-bc1a6a7ca2a79166302cb3fad1ff0358R22.
Could you give me the feedback?

Thanks.
Evan

	Evan Tseng [:evanxd]
	Comment 28 • 10 years ago

(In reply to Gareth Aye [:gaye] from comment #23)
> Comment on attachment 8382901 [details] [review]
> Pull request
> 
> I want to work toward a place where the calendar backend and frontend are
> less tightly coupled. In its current state, this patch adds request id
> generation and knowledge of the caldav backend to our frontend webapp. We
> also need integration tests including (but not limited to) the cases where
> 
> - When we abort before we get the entire propfind response, all of the xhr
> requests should be aborted and no account data should be written to idb.
> - When we don't abort, the cancel ui should disappear once we finish the
> calendar home request and before we begin calendar sync
> 
> It seems as if, in its current state, there are times at which the user
> could press the cancel button, but their accounts would still be setup. Also
> in
> https://github.com/mozilla-b2g/caldav/blob/master/lib/caldav/request/
> abstract.js#L59, if you abort the xhr request, will the callback that closes
> the xml parser be issued? If not, I think we could get bad state in the xml
> parser perhaps?
>
Sorry, what is the xml parser closed by the callback?
 
> Like I mentioned above, this is a small feature, but there are a lot of ways
> that this could go wrong and confuse users.

	James Lal [:lightsofapollo]
	Comment 30 • 10 years ago

Comment on attachment 8382901 [details] [review]
Pull request

:( I missed this when it was important clearing my requests up so I am better in the future

	Dylan Oliver [:doliver]
	Comment 31 • 10 years ago

[priority] --> tracking-b2g:+ conversion

	Tiffanie Shakespeare [:tif]
	Comment 32 • 9 years ago

Harly - is this still needed? Are the specs still relevant and ready?

	Harly Hsu[:harly]
	Comment 33 • 9 years ago

The spec is ready; however, as gaye has mentioned, it seems that we need to do a lot of works for this to happen.

	BMO Automation
	Comment 34 • 6 years ago

Firefox OS is not being worked on