Closed
Bug 964724
Opened 11 years ago
Closed 11 years ago
Hide regular Offline Error Screen When Connectivity Resumes - App Foreground and Background
Categories
(Firefox OS Graveyard :: Gaia::System, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
1.4 S1 (14feb)
People
(Reporter: crdlc, Assigned: crdlc)
References
Details
(Whiteboard: [systemsfe])
Attachments
(3 files)
(deleted),
text/html
|
mikehenrty
:
review+
|
Details |
(deleted),
image/png
|
djabber
:
ui-review-
|
Details |
(deleted),
image/png
|
djabber
:
ui-review-
|
Details |
No description provided.
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → crdlc
Status: NEW → ASSIGNED
Assignee | ||
Updated•11 years ago
|
Updated•11 years ago
|
Whiteboard: [systemsfe]
Target Milestone: --- → 1.3 C3/1.4 S3(31jan)
Assignee | ||
Comment 1•11 years ago
|
||
Status: tt is already implemented and right now I am working on unit tests
Assignee | ||
Comment 2•11 years ago
|
||
Hi Michael,
Vivien told me that you are the best :) to review this patch!
Thanks for your time mate
Attachment #8370083 -
Flags: review?(mhenretty)
Updated•11 years ago
|
Target Milestone: 1.3 C3/1.4 S3(31jan) → 1.4 S1 (14feb)
Comment 3•11 years ago
|
||
Hi Crisian! Thank you for the patch (and especially the unit tests)! I am working on some other bugs at the moment, so I might not get to this today, but it's on my radar :)
Assignee | ||
Comment 4•11 years ago
|
||
Thanks a lot for your help Michael
Comment 5•11 years ago
|
||
Looking good Cristian! Thank you especially for adding unit tests! I left some comments on the PR.
Also, I think we will need a quick review from UX on these changes. I'm not sure if all the scenarios were considered with respect to hiding the retry button and tap to retry message. Can you upload some screenshots of both the framed and non-framed screens for netOffline, and flag :djabber for UX review?
Assignee | ||
Comment 6•11 years ago
|
||
Hi Michael,
Thanks for the review, I gonna address all your comments today
(In reply to Michael Henretty [:mhenretty] from comment #5)
> Looking good Cristian! Thank you especially for adding unit tests! I left
> some comments on the PR.
>
> Also, I think we will need a quick review from UX on these changes. I'm not
> sure if all the scenarios were considered with respect to hiding the retry
> button and tap to retry message. Can you upload some screenshots of both the
> framed and non-framed screens for netOffline, and flag :djabber for UX
> review?
I am hiding the retry button just for net errors because this patch implements the automatic reload of apps when the device is online. If the user is seeing the network error is because of the app cannot be reloaded.
Francis, do you think what there are some scenario where the retry-button makes sense for network error?. If the error is for DNS, server, etc.. there is a retry button for sure like before.
I will upload some pics of this change today too
Thanks mates
Assignee | ||
Comment 8•11 years ago
|
||
No retry button for apps
Attachment #8371386 -
Flags: ui-review?(fdjabri)
Comment 10•11 years ago
|
||
Thanks again Cristian! I'll take a look at this next week.
Flags: needinfo?(mhenretty)
Assignee | ||
Comment 11•11 years ago
|
||
Please Michael, take a look to this one in order to continue working on bug 969267 :) I think that it is fast because it is just a second review after addressing your comments. Thanks a lot
Comment 12•11 years ago
|
||
Comment on attachment 8370083 [details]
Patch v1
A couple more comments/questions on the PR. Other than that this is looking good. Go ahead and re-flag me when it's ready, or ping me on IRC if you have questions.
Attachment #8370083 -
Flags: review?(mhenretty)
Assignee | ||
Comment 13•11 years ago
|
||
Comment on attachment 8370083 [details]
Patch v1
Thanks, addressed comments.
Attachment #8370083 -
Flags: review?(mhenretty)
Comment 14•11 years ago
|
||
Comment on attachment 8370083 [details]
Patch v1
Looks like you have a debug commit in that PR, so I added a couple of comments to the first commit. They are two very small nits, other than that this looks awesome. Thanks!
We still need UX signoff, and I am waiting to hear back from :pdol about whether we can land this independently of the other offline stuff. My guess is yes, but we'll wait on them. Nice work!
Comments here.
https://github.com/crdlc/gaia/commit/a3ff11fc1d8505a3417efc50794c3185786e8256
Attachment #8370083 -
Flags: review?(mhenretty) → review+
Comment 15•11 years ago
|
||
Comment on attachment 8371385 [details]
framed offline.png
I agree that it doesn't make any sense to show this in-frame error without the option of tapping to retry. Please keep the tap-to-retry button in for this case.
Attachment #8371385 -
Flags: ui-review?(fdjabri) → ui-review-
Comment 16•11 years ago
|
||
Comment on attachment 8371386 [details]
unframed offline.png
I also agree that it makes no sense to show this dialog without a retry button. However, if it is a network error that is the problem and not the device, we should alter the message, because this gives the user the impression that there is an issue with the device and then gives them no means to fix it. This should be fixed by Bug #912445.
Attachment #8371386 -
Flags: ui-review?(fdjabri) → ui-review-
Comment 17•11 years ago
|
||
After speaking with Francis over vidyo now, we decided the issue with unframed offline screen is not hiding the retry button (we need to do this), but that it doesn't make sense to have this screen unless we provide resolution actions (ie going to settings an enabling wifi, or something). So we are going to have to block landing this bug on that unfortunately.
As far as the framed case, even though the retry button doesn't in a sense do anything (since the frame will automatically refresh), he still thinks its nicer to the user to give him the ability to manually try.
Assignee | ||
Comment 18•11 years ago
|
||
I am going to keep the UIs and messages like they are currently in master because this bug is "Hide regular Offline Error Screen When Connectivity Resumes - App Foreground and Background". The UIs and messages will be addressed in bugs related to "Offline error handling"
http://i.imgur.com/LNGb6eu.png
Assignee | ||
Comment 19•11 years ago
|
||
Messages will be modified in bug 969267 as they appear in the wireframe, thanks a lot
Comment 20•11 years ago
|
||
As Cristian said, in this bug there is no change in the UI (also keeping retry button), because all of them will be applied in bug 969267, so after geting r+ we can land this bug as is
Assignee | ||
Comment 21•11 years ago
|
||
You are right, no changes on UI anymore here
Assignee | ||
Comment 22•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
Blocks: 1.4-systems-fe
You need to log in
before you can comment on or make changes to this bug.
Description
•