Closed Bug 1217174 Opened 6 years ago Closed 5 months ago

Implement helper UI for send to device when user is not ready to send a tab (e.g. no account)

Categories

(Firefox for Android Graveyard :: General, defect)

All
Android
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED INCOMPLETE

People

(Reporter: mcomella, Unassigned, Mentored)

References

()

Details

(Whiteboard: [lang=java][see comment 13])

Attachments

(2 files, 8 obsolete files)

In bug 1213490, we introduce the flow that when the user signs into their sync account but has no other devices connected, we show a toast telling the user to connect another device but there isn't enough space to be specific and really tell the user what to do.

Anthony, anything better we can do here? SUMO, or a tab queue style dialog?
Flags: needinfo?(alam)
A super toast (soon to be snackbar) would be a nice start. Do we have an article on SUMO about this feature?
Flags: needinfo?(alam) → needinfo?(michael.l.comella)
Flags: needinfo?(michael.l.comella) → needinfo?(rtanglao)
The experience is super crappy atm – I'm thinking tracking 44?
tracking-fennec: --- → ?
:mcomella i don't think we have an article about this aspect of sync, right:joni?
here are the articles we have:

https://support.mozilla.org/en-US/products/mobile/save-share-and-sync-firefox-android
Flags: needinfo?(rtanglao) → needinfo?(jsavage)
Anthony, what if we used the tab queue contextual hint to display a longer message than we would otherwise be able to? There would only be one call-to-action button (e.g. "Got it").

If there's a sumo article on adding a sync device, we can have a "Learn more" button too.
Flags: needinfo?(alam)
It's very late for UX changes for 44, but let's see if we can do something simple.
Assignee: nobody → michael.l.comella
tracking-fennec: ? → 44+
We decided to punt to 45 – see https://bugzilla.mozilla.org/show_bug.cgi?id=1140048#c74

We want to do a helper UI here to step the user through using this feature if they're not in the correct state (logged into a Firefox account with another device to send a tab to).

Keeping the NI for Anthony for mocks.
tracking-fennec: 44+ → 45+
Flags: needinfo?(jsavage)
Not ready to share means:
  * Not logged into a Firefox account
  * Logged into a Firefox account but there are no other synced devices to send a tab to
Summary: Improve flow when the user clicks shareplane and there are no other synced devices → Improve flow when the user clicks shareplane and they are not ready to share
I met with Barbara and Anthony. We decided to go ahead with Anthony's specs for the full helper UI. However, since we can't respond to the user signing into sync without significant work, the onboarding flow will "drop" the user – they'll have to click the menu and shareplane again to continue the flow. Ideally, when the user who is already signed in clicks the shareplane but there are no devices to share to and they're shown the, "You need to sign in on another device" screen, when they click "Got it", we'll sync and show the device list if applicable, but we'll see how much work that is too.

In the event that this doesn't hit 45, we'll back out again rather than put in the work to make a Nightly flag (bug 1220910).

Anthony, can you post your specs for the helper UI?
(In reply to Michael Comella (:mcomella) from comment #8)
> I met with Barbara and Anthony. We decided to go ahead with Anthony's specs
> for the full helper UI. However, since we can't respond to the user signing
> into sync without significant work, the onboarding flow will "drop" the user
> – they'll have to click the menu and shareplane again to continue the flow.
> Ideally, when the user who is already signed in clicks the shareplane but
> there are no devices to share to and they're shown the, "You need to sign in
> on another device" screen, when they click "Got it", we'll sync and show the
> device list if applicable, but we'll see how much work that is too.

Agreed, this would be a nice to have and if we can't get to it, we should split it off into a follow up bug post-45 I think :)

> In the event that this doesn't hit 45, we'll back out again rather than put
> in the work to make a Nightly flag (bug 1220910).
> 
> Anthony, can you post your specs for the helper UI?

I've collected a rough flow here. https://invis.io/324PYKYQX

I'll also attach the images in this bug (with the device and the plane illustration)
Flags: needinfo?(alam)
images!
Flags: needinfo?(michael.l.comella)
Flags: needinfo?(michael.l.comella)
Bug 1140048 was backed out. We plan to implement this helper UI in the current menu, then reland bug 1140048 and attach this helper UI to that new menu.
Assignee: michael.l.comella → nobody
Mentor: michael.l.comella
tracking-fennec: 45+ → ---
Depends on: 1140048
No longer depends on: 1213490
Whiteboard: [lang=java]
Blocks: 1140048
No longer depends on: 1140048
Summary: Improve flow when the user clicks shareplane and they are not ready to share → Implement helper UI for send to device when user is not ready to send a tab (e.g. no account)
I'd like to pick this up if no one's currently working on it.
No longer blocks: 1230761
(In reply to Alex Johnson(:alex_johnson) from comment #12)
> I'd like to pick this up if no one's currently working on it.

Sure! To be explicit, here's what you have to do:

1) Make the "Send to other devices" menu item appear in the share menu all the time (i.e. no matter account/device state).
2) When the user is not signed into an account and they click the "Send to other devices" item, show [1]. The button should open the "Sign into Firefox" account flow (look at the settings code to find out how to do this). Once the user finishes this flow, you don't need to take any action.
3) When the user is signed in but there are no other devices to share to and they click the "Send to other devices" item, show [2]. When they click the "Got it" button, the dialog should be dismissed
4) When there are devices to share to, we activate the current flow that shows the share overlay.

The dialogs we show should re-use the tab queue prompt code – unfortunately, I'm not sure where that is or if it's abstract enough at the moment to support arbitrary UI. Let me know if you need help with this.

A follow-up we considered is to make the transitions between steps smoother so after signing in from 2), we'll open the dialog in 3) (if applicable), and once there is a synced device, we open 4).

Is that enough info to get going on this?

[1]: https://mozilla.invisionapp.com/share/324PYKYQX#/screens/113495935
[2]: https://mozilla.invisionapp.com/share/324PYKYQX#/screens/113496012
Assignee: nobody → alex_johnson24
Whiteboard: [lang=java] → [lang=java][see comment 13]
Status: NEW → ASSIGNED
Michael,
Are there any docs on fxaccounts somewhere?  I'm having a hard time figuring out how to close the tab and go to the page the user was on after a successful login.  Or should we file a bug in fxaccounts to close the tab after a successful login if the request came with a "sendToDevice" entrypoint url parameter (aka. about:accounts?action=signup&entrypoint=sendToDevice)?
Flags: needinfo?(michael.l.comella)
(In reply to Alex Johnson (:alex_johnson) from comment #16)
> Are there any docs on fxaccounts somewhere?

Redirect nalexander.

> I'm having a hard time figuring
> out how to close the tab and go to the page the user was on after a
> successful login.  Or should we file a bug in fxaccounts to close the tab
> after a successful login if the request came with a "sendToDevice"
> entrypoint url parameter (aka.
> about:accounts?action=signup&entrypoint=sendToDevice)?

I don't think there's currently a way to do this and a bug exists – bug 1221217. Sorry that I was unclear – reacting to steps the user takes in the helper UI should be left to the follow-up.
Flags: needinfo?(michael.l.comella) → needinfo?(nalexander)
Comment on attachment 8702208 [details]
MozReview Request: Bug 1217174 - Part 1: Always show 'Send to other devices' menu item. r?mcomella

https://reviewboard.mozilla.org/r/29099/#review25959

r+ w/ nits discussed.

::: mobile/android/base/java/org/mozilla/gecko/widget/ActivityChooserModel.java
(Diff revision 1)
> -                    if (!hasOtherSyncClients()) {

I think this method is unused now – you could remove it here.

::: mobile/android/base/java/org/mozilla/gecko/widget/ActivityChooserModel.java:783
(Diff revision 1)
> -                    resolveInfo.icon = R.drawable.icon_shareplane;
> +                    resolveInfo.icon = R.drawable.shareplane;

What was the motivation moving to the other icon? Was there any visible change in position?

I think we used the other icon to preserve certain whitespace requirements.
Attachment #8702208 - Flags: review?(michael.l.comella)
Comment on attachment 8702209 [details]
MozReview Request: Bug 1217174 - Removed unused icon_shareplane drawables. r?mcomella

https://reviewboard.mozilla.org/r/29101/#review25961

Provided we don't actually need this (part 1), lgtm.
Attachment #8702209 - Flags: review?(michael.l.comella) → review+
Comment on attachment 8702208 [details]
MozReview Request: Bug 1217174 - Part 1: Always show 'Send to other devices' menu item. r?mcomella

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29099/diff/1-2/
Attachment #8702208 - Attachment description: MozReview Request: Bug 1217174 - Always show 'Send to other devices' menu item. r?mcomella → MozReview Request: Bug 1217174 - Part 1: Always show 'Send to other devices' menu item. r?mcomella
Attachment #8702208 - Flags: review?(michael.l.comella)
Attachment #8702209 - Attachment is obsolete: true
https://reviewboard.mozilla.org/r/29099/#review25959

> What was the motivation moving to the other icon? Was there any visible change in position?
> 
> I think we used the other icon to preserve certain whitespace requirements.

I was looking at Bug 1210989 where we renamed overlay_send_tab_icon to shareplane.  That was a different scenario.  I changed it back.
Attachment #8702659 - Flags: feedback?(michael.l.comella)
Comment on attachment 8702208 [details]
MozReview Request: Bug 1217174 - Part 1: Always show 'Send to other devices' menu item. r?mcomella

https://reviewboard.mozilla.org/r/29099/#review25971
Attachment #8702208 - Flags: review?(michael.l.comella) → review+
Attachment #8702659 - Attachment is obsolete: true
Attachment #8702659 - Flags: feedback?(michael.l.comella)
Attachment #8702659 - Attachment description: MozReview Request: wip: sendtodevice fxaccounts → MozReview Request: Bug 1217174 - Part 2: Start about:accounts get started intent if no account found. r?mcomella
Attachment #8702659 - Attachment is obsolete: false
Attachment #8702659 - Flags: review?(michael.l.comella)
Comment on attachment 8702659 [details]
MozReview Request: Bug 1217174 - Part 2: Created NoAccountsDialog Activity and send url parameter to about:accounts with the webpage the user wanted to send. r?mcomella

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29169/diff/1-2/
(In reply to Michael Comella (:mcomella) from comment #17)
> (In reply to Alex Johnson (:alex_johnson) from comment #16)
> > Are there any docs on fxaccounts somewhere?
> 
> Redirect nalexander.
> 
> > I'm having a hard time figuring
> > out how to close the tab and go to the page the user was on after a
> > successful login.  Or should we file a bug in fxaccounts to close the tab
> > after a successful login if the request came with a "sendToDevice"
> > entrypoint url parameter (aka.
> > about:accounts?action=signup&entrypoint=sendToDevice)?
> 
> I don't think there's currently a way to do this and a bug exists – bug
> 1221217.

This is correct.  I'm not sure what this should be, tbh, because a user could create an account and be in many states, including both unverified, so they cannot Sync (and cannot send a tab); and verified, but haven't yet Synced, so we have no devices for them.  They could also have an Android account that doesn't actually reflect a server account, due to the server deleting a server account immediately upon recognizing a bad email address.

I think we should surface "account added" messages throughout the app, and then try to centralize "account is synced and healthy" more generally.  We don't have that right now.

 Sorry that I was unclear – reacting to steps the user takes in the
> helper UI should be left to the follow-up.

The interaction with Send Tab in particular is hard.  We might want to add a "sendTab" parameter to about:accounts, and then have the fxa-content-server web UI display a button to tap to "continue to the send tab" experience.

We might also want to understand what Desktop does in this situation.

markh, vlad: can you try to interpret Comment 16 and Comment 17 and explain to me how Desktop manages "continuation after sign up/sign in" actions?  We should be as similar as we can be.
Flags: needinfo?(vlad)
Flags: needinfo?(nalexander)
Flags: needinfo?(markh)
I think we should make sure the sync has completed after login before we send the user back to the "send to other device" activity. That might make for a smoother flow. We should show a loading screen or something after login until the sync has successfully completed or show a toast and go back to the Web page if sync failed.
s/should show/could show/
(In reply to Nick Alexander :nalexander [Vacation PTO from Dec 23--Jan 4] from comment #25)
> markh, vlad: can you try to interpret Comment 16 and Comment 17 and explain
> to me how Desktop manages "continuation after sign up/sign in" actions?  We
> should be as similar as we can be.

On desktop we have observers that could be used to listen for relevant state changes. However, I suspect that on desktop we wouldn't bother with attempts at continuation in this scenario. There are 3 interesting states here.

* User has an account but isn't logged in.
* User has no account.
* Account has no other devices.

Only the first 1 is possible to continue from. I suspect that in the first 2 cases we'd just show UI saying, eg, "You need to sign in before sending a tab", throw them to about:accounts and leave them to re-initiate the operation after completion. In the second case we'd show a promo for downloading a mobile Firefox. This is what we do for, eg, the new "synced tabs" panel - we throw them to about:accounts, but don't make any attempt to detect the login and reopen the panel UI after completion.
Flags: needinfo?(markh)
Comment on attachment 8702208 [details]
MozReview Request: Bug 1217174 - Part 1: Always show 'Send to other devices' menu item. r?mcomella

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29099/diff/2-3/
Comment on attachment 8702659 [details]
MozReview Request: Bug 1217174 - Part 2: Created NoAccountsDialog Activity and send url parameter to about:accounts with the webpage the user wanted to send. r?mcomella

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29169/diff/2-3/
(Clearing NI from vlad since markh hit the request.)

(In reply to Alex Johnson (:alex_johnson) from comment #26)
> I think we should make sure the sync has completed after login before we
> send the user back to the "send to other device" activity. That might make
> for a smoother flow. We should show a loading screen or something after
> login until the sync has successfully completed or show a toast and go back
> to the Web page if sync failed.

In general, this is hard, since:

1) Sync might not be triggered when requested (by the system);
2) even when triggered, a sync might take a long time to complete, or never complete;
3) Sync is rather a wholistic system and doesn't surface individual errors cleanly.
Flags: needinfo?(vlad)
Comment on attachment 8702659 [details]
MozReview Request: Bug 1217174 - Part 2: Created NoAccountsDialog Activity and send url parameter to about:accounts with the webpage the user wanted to send. r?mcomella

https://reviewboard.mozilla.org/r/29169/#review26459

Technically correct and feasible as a basic implementation, but this doesn't follow the mocks Anthony included in comment 9 – can you try to implement that UI instead?

::: mobile/android/base/java/org/mozilla/gecko/overlays/ui/ShareDialog.java:111
(Diff revision 3)
> -        // Escape hatch: we don't show the option to open this dialog in this state so this should
> +        if (account == null) {

Maybe you want to use `FirefoxAccounts.firefoxAccountsExist` here if you're not going to use the returned account?

::: mobile/android/base/java/org/mozilla/gecko/overlays/ui/ShareDialog.java:112
(Diff revision 3)
> -        // never be run. However, due to potential inconsistencies in synced client state
> +            final Intent fxIntent = new Intent(FxAccountConstants.ACTION_FXA_GET_STARTED);

We should keep this comment unless we remove the code it's associated with.

::: mobile/android/base/java/org/mozilla/gecko/overlays/ui/ShareDialog.java:120
(Diff revision 3)
> +        else {

Rather than indenting all of this code (since excessively indented code becomes difficult to read), you should return early from the branch above. This is typically with error/assumption handling.

::: mobile/android/base/java/org/mozilla/gecko/overlays/ui/ShareDialog.java
(Diff revision 3)
> -

nit: Was this whitespace change intentional? If so, we ususally make whitespace changes in a separate commit (unless we're overwriting the lines we're changing) to make it clear it was intentional. If it was not intentional, try not to change unrelated lines! :) You can use `hg diff` and `hg diff -c .` to ensure you're making the changes you intend to.

::: mobile/android/base/locales/en-US/android_strings.dtd:143
(Diff revision 3)
> +     Localization note (overlay_no_firefox_account): Used when the menu option

You should move this directly above the String it refers to. Don't worry about reducing the number of comment blocks.
Attachment #8702659 - Flags: review?(michael.l.comella)
Comment on attachment 8702659 [details]
MozReview Request: Bug 1217174 - Part 2: Created NoAccountsDialog Activity and send url parameter to about:accounts with the webpage the user wanted to send. r?mcomella

I'm gonna go ahead and redo this one.  Sorry about that!
Attachment #8702659 - Attachment is obsolete: true
Anthony, do we have a specific font size and color for the "you'll need to connect 2 devices" text or is it the same style as the hint in the History empty state?
Flags: needinfo?(alam)
(In reply to Alex Johnson (:alex_johnson) from comment #35)
> Anthony, do we have a specific font size and color for the "you'll need to
> connect 2 devices" text or is it the same style as the hint in the History
> empty state?

This takes from the same design specs as our "Opening multiple links?" helper UI in bug 1112195. So if we could just inherit the same padding, etc, that would be great :)

It should be Roboto, italic, 14sp, #E66000

Thanks Alex!
Flags: needinfo?(alam)
Attached image device-2016-01-11-182707.png (obsolete) —
Flags: needinfo?(me)
Attachment #8706658 - Flags: feedback?(alam)
Comment on attachment 8702208 [details]
MozReview Request: Bug 1217174 - Part 1: Always show 'Send to other devices' menu item. r?mcomella

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29099/diff/3-4/
Comment on attachment 8702659 [details]
MozReview Request: Bug 1217174 - Part 2: Created NoAccountsDialog Activity and send url parameter to about:accounts with the webpage the user wanted to send. r?mcomella

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29169/diff/3-4/
Attachment #8702659 - Attachment description: MozReview Request: Bug 1217174 - Part 2: Start about:accounts get started intent if no account found. r?mcomella → MozReview Request: Bug 1217174 - Part 2: Created NoAccountsDialog Activity and send url parameter to about:accounts with the webpage the user wanted to send. r?mcomella
Attachment #8702659 - Attachment is obsolete: false
Attachment #8702659 - Flags: review?(michael.l.comella)
Comment on attachment 8702208 [details]
MozReview Request: Bug 1217174 - Part 1: Always show 'Send to other devices' menu item. r?mcomella

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29099/diff/4-5/
Comment on attachment 8702659 [details]
MozReview Request: Bug 1217174 - Part 2: Created NoAccountsDialog Activity and send url parameter to about:accounts with the webpage the user wanted to send. r?mcomella

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29169/diff/4-5/
Comment on attachment 8702659 [details]
MozReview Request: Bug 1217174 - Part 2: Created NoAccountsDialog Activity and send url parameter to about:accounts with the webpage the user wanted to send. r?mcomella

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29169/diff/5-6/
Added the animateout for the NoAccountsDialog and fixed some nits, might be a good idea to move the animateOut and some of the other common functions to a different "Dialog.java" file. But I'm gonna leave that for a different bug.  It's a little complicated since the ShareDialog and NoAccountsDialog have different layouts.
Attached image Tablet Screenshot (obsolete) —
Attachment #8706745 - Flags: review?(alam)
Attached image Tablet Screenshot (obsolete) —
Attachment #8706745 - Attachment is obsolete: true
Attachment #8706745 - Flags: review?(alam)
Attachment #8706746 - Flags: feedback?(alam)
Comment on attachment 8706658 [details]
device-2016-01-11-182707.png

Nice! this is almost there Alex!

 - The padding above the image and below the "Sign in to Fennec" button should both be 40 dp :)

 - In addition, the padding below the graphic should be 30 dp

Thanks!
Attachment #8706658 - Flags: feedback?(alam) → feedback-
Comment on attachment 8706746 [details]
Tablet Screenshot

same feedback regarding the padding :)
Attachment #8706746 - Flags: feedback?(alam) → feedback-
Attached image Screenshot (obsolete) —
How does this look?
Attachment #8706658 - Attachment is obsolete: true
Attachment #8706746 - Attachment is obsolete: true
Attachment #8707206 - Flags: feedback?(alam)
Attached image Tablet Screenshot (obsolete) —
Attachment #8707212 - Flags: feedback?(alam)
Comment on attachment 8707206 [details]
Screenshot

The margins around the image looks better. But the button should stay the same size as before. I meant to increase the negative space surrounding it, not the size of the button itself.

Sorry! I must've used "padding" much too loosely here. The "padding" I was referring to should be created with margins outside the button.
Attachment #8707206 - Flags: feedback?(alam) → feedback-
Comment on attachment 8707212 [details]
Tablet Screenshot

Same comments about the button size as above :)

The margins under the image look too large here. Is it 30 dp? 

To be clear: 

 - the white space between the bottom of the image and the top of the "Want to send this tab?" should be 30 dp.

 - the white space between the top of the sheet and the top of the image should be 40 dp.
Attachment #8707212 - Flags: feedback?(alam) → feedback-
(In reply to Anthony Lam (:antlam) from comment #52)
> The margins under the image look too large here. Is it 30 dp? 
> 
> To be clear: 
> 
>  - the white space between the bottom of the image and the top of the "Want
> to send this tab?" should be 30 dp.
> 
>  - the white space between the top of the sheet and the top of the image
> should be 40 dp.

same comments for the phone UI :)

Thanks Alex!
Flags: needinfo?(me)
How does this look?  Sorry, the text had padding on top causing the image to show a bottom padding of larger than 30dp.
Attachment #8707206 - Attachment is obsolete: true
Attachment #8707212 - Attachment is obsolete: true
Flags: needinfo?(me)
Attachment #8708050 - Flags: feedback?(alam)
Comment on attachment 8708050 [details]
Screenshot_2016-01-14-15-37-28.png

Looks good! nice job \o/
Attachment #8708050 - Flags: feedback?(alam) → feedback+
Comment on attachment 8702208 [details]
MozReview Request: Bug 1217174 - Part 1: Always show 'Send to other devices' menu item. r?mcomella

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29099/diff/5-6/
Comment on attachment 8702659 [details]
MozReview Request: Bug 1217174 - Part 2: Created NoAccountsDialog Activity and send url parameter to about:accounts with the webpage the user wanted to send. r?mcomella

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29169/diff/6-7/
Michael, I've got the "Sign in" dialog here, but should we wait till we figure out the about:accounts callback stuff till we implement the last part of Anthony's mock?
Flags: needinfo?(michael.l.comella)
Hey Alex – I'll get to this tomorrow. Sorry for the delay.
Comment on attachment 8702208 [details]
MozReview Request: Bug 1217174 - Part 1: Always show 'Send to other devices' menu item. r?mcomella

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29099/diff/6-7/
Comment on attachment 8702659 [details]
MozReview Request: Bug 1217174 - Part 2: Created NoAccountsDialog Activity and send url parameter to about:accounts with the webpage the user wanted to send. r?mcomella

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29169/diff/7-8/
(In reply to Alex Johnson (:alex_johnson) from comment #61)
> Comment on attachment 8702659 [details]
> MozReview Request: Bug 1217174 - Part 2: Created NoAccountsDialog Activity
> and send url parameter to about:accounts with the webpage the user wanted to
> send. r?mcomella
> 
> Review request updated; see interdiff:
> https://reviewboard.mozilla.org/r/29169/diff/7-8/

Fixed a crash this was causing on one of my devices.
(In reply to Alex Johnson (:alex_johnson) from comment #58)
> Michael, I've got the "Sign in" dialog here, but should we wait till we
> figure out the about:accounts callback stuff till we implement the last part
> of Anthony's mock?

No. It was my impression that Anthony and I decided that the user would just have to click the shareplane icon again to move forward in the process since implementing the continuation would be difficult.

Anthony, do you agree?
Flags: needinfo?(michael.l.comella) → needinfo?(alam)
https://reviewboard.mozilla.org/r/29099/#review28445

mozreview tells me I need to review this again but I seem to have already r+'d it on bz!
Attachment #8702659 - Flags: review?(michael.l.comella)
Comment on attachment 8702659 [details]
MozReview Request: Bug 1217174 - Part 2: Created NoAccountsDialog Activity and send url parameter to about:accounts with the webpage the user wanted to send. r?mcomella

https://reviewboard.mozilla.org/r/29169/#review28447

I was hoping to re-use some existing code here – let's see how much work it is. If it's too much, maybe we should just move forward with this approach.

::: mobile/android/base/java/org/mozilla/gecko/overlays/ui/NoAccountsDialog.java:25
(Diff revision 8)
> +public class NoAccountsDialog extends Locales.LocaleAwareActivity {

Sorry if I was unclear but I was hoping to repurpose the `TabQueuePrompt` for this. Did you take a look into that and if so, is there a reason you chose not to? Would it be much work to move this code over there? It looks like there is a lot of tab queue specific code in there so maybe it'd be more trouble than it's worth...

::: mobile/android/base/java/org/mozilla/gecko/overlays/ui/NoAccountsDialog.java:34
(Diff revision 8)
> +        ((ImageView)findViewById(R.id.image)).setImageResource(R.drawable.mobile_to_pc);

Make sure you run the new png assets through an image compressor before `hg add`ing them. We typically use `trimage` on Linux or ImageOptim on OS X.

::: mobile/android/base/java/org/mozilla/gecko/overlays/ui/ShareDialog.java
(Diff revision 8)
> -

nit: You removed this whitespace line – try not to change unrelated whitespace!

::: mobile/android/base/locales/en-US/android_strings.dtd:144
(Diff revision 8)
> +<!-- Localization note: The following strings show in the no accounts dialog when

afaik, a localization note can only describe a single String. The localization tools automatically parse out the notes so they need to follow the format of the other notes in the file.

::: mobile/android/base/resources/layout/overlay.xml:3
(Diff revision 8)
> +   This Source Code Form is subject to the terms of the Mozilla Public

nit: can you follow the formatting for the other files with this comment?

::: mobile/android/base/resources/layout/overlay.xml:16
(Diff revision 8)
> +        style="@style/Overlay">

If styles aren't re-used, it's best practice to leave the style declarations within the xml file itself. Putting them outside (via `@style` references) causes the developer to jump to a different file in order to understand the structure when it could be simpler and flat. Similarly with inheritance – don't inherit unless you have to!

::: mobile/android/base/resources/layout/overlay.xml:24
(Diff revision 8)
> +            style="@style/Overlay.Title" />

Best practice with text is to put text attributes in `textAppearance` and other attributes in `style`.

::: mobile/android/base/resources/layout/overlay.xml:51
(Diff revision 8)
> +         entire Activity with an onClickListener. -->

Could you set the hit listener on the root view of the activity instead?  I'm not sure how the root view's children might intercept the presses though.

Or perhaps preferrably, override `onInterceptTouchEvent` (or whichever touch event it is that handles the initial touch)?
(In reply to Michael Comella (:mcomella) from comment #63)
> (In reply to Alex Johnson (:alex_johnson) from comment #58)
> > Michael, I've got the "Sign in" dialog here, but should we wait till we
> > figure out the about:accounts callback stuff till we implement the last part
> > of Anthony's mock?
> 
> No. It was my impression that Anthony and I decided that the user would just
> have to click the shareplane icon again to move forward in the process since
> implementing the continuation would be difficult.
> 
> Anthony, do you agree?

Ah.  I think this makes more sense to me now.  I thought we were going to show the "Where do we send it?" dialog after the login has completed.  But, we wanna show it if the user taps the sendtab dialog and a sync device isn't found?  Am I understanding that correctly?
(In reply to Alex Johnson (:alex_johnson) from comment #66)
> But, we wanna show it if the user taps the sendtab dialog and a sync device isn't
> found?  Am I understanding that correctly?

We want to show it if the user taps the shareplane icon (the "send tab to device" button) and the user has an account but no other devices are found.
Also, if you were curious about more style best-practices, this is a really good blog post:
 http://blog.danlew.net/2014/11/19/styles-on-android/
(In reply to Michael Comella (:mcomella) from comment #63)
> (In reply to Alex Johnson (:alex_johnson) from comment #58)
> > Michael, I've got the "Sign in" dialog here, but should we wait till we
> > figure out the about:accounts callback stuff till we implement the last part
> > of Anthony's mock?
> 
> No. It was my impression that Anthony and I decided that the user would just
> have to click the shareplane icon again to move forward in the process since
> implementing the continuation would be difficult.
> 
> Anthony, do you agree?

Yep - what :mcomella said.
Flags: needinfo?(alam)
Comment on attachment 8702208 [details]
MozReview Request: Bug 1217174 - Part 1: Always show 'Send to other devices' menu item. r?mcomella

I'm gonna redo this one.  Some recent changes broke my patch (won't let me rebase).
Attachment #8702208 - Attachment is obsolete: true
Attachment #8702659 - Attachment is obsolete: true
I'm gonna unassign this because I will not have time to continue working on this until sometime.
Assignee: me → nobody
Status: ASSIGNED → NEW
Anthony, are we still interested in this?
Flags: needinfo?(alam)
Hm, what's missing here?

I don't think this is prioritized in our road map. But this contextual hint seems close enough that it might be good to finish off.
Flags: needinfo?(alam) → needinfo?(michael.l.comella)
I spoke to Anthony IRL – he mentioned we still want this. I offered the possibility that we should try to prioritize our mentored bugs more readily and he said we can discuss it during the funnel meeting.

(In reply to Anthony Lam (:antlam) from comment #73)
> Hm, what's missing here?

I'm not sure how complete the functionality in the obsolete patches was but they were using an unideal approach – there was a lot of duplicated code. I think it'd be non-trivial to finish it off so I'd say it should remain mentorable.
Flags: needinfo?(michael.l.comella)
We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.
Status: NEW → RESOLVED
Closed: 5 months ago
Resolution: --- → INCOMPLETE
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.