Closed Bug 1203628 Opened 6 years ago Closed 6 years ago

"Send to other devices" from Share overlay in other apps is styled incorrectly

Categories

(Firefox for Android Graveyard :: Overlays, defect)

All
Android
defect
Not set
normal

Tracking

(firefox44 verified)

RESOLVED FIXED
Firefox 44
Tracking Status
firefox44 --- verified

People

(Reporter: antlam, Assigned: mcomella, Mentored)

References

Details

(Whiteboard: [lang=java])

Attachments

(4 files)

STR:
 - "Add to Firefox"
 - Press "Send to other devices"
 - See ICS styled Modal

Expected:
 - Same style for devices list as "Send to other devices" while in Firefox
Worth noting that with 2 (or is it 3?) other synced devices, as most users have, the device list is shown inline so this won't affect most users.
Mentor: michael.l.comella
Whiteboard: [lang=java]
I can't remember either, did we decide on 2 or 3 devices because of some data or was it more arbitrary than that? 

Also a modal might not be so bad for a long list of devices... if it at least matched our theme (L). :)
(In reply to Anthony Lam (:antlam) from comment #3)
> I can't remember either, did we decide on 2 or 3 devices because of some
> data or was it more arbitrary than that? 

It was that way when I got to it, but I'd infer it had something to do with the fact that users generally have fewer than 2 or 3 other devices synced and wanting to keep context from the previous app.

> Also a modal might not be so bad for a long list of devices... if it at
> least matched our theme (L). :)

Do we have other dialogs that match our theme? If so, which are those (besides the share overlay dialog)?
Flags: needinfo?(alam)
(In reply to Michael Comella (:mcomella) from comment #4)
> (In reply to Anthony Lam (:antlam) from comment #3)
> > I can't remember either, did we decide on 2 or 3 devices because of some
> > data or was it more arbitrary than that? 
> 
> It was that way when I got to it, but I'd infer it had something to do with
> the fact that users generally have fewer than 2 or 3 other devices synced
> and wanting to keep context from the previous app.

I think you might be right. Also because we only had 3 items in the list at the time. How hard would it be to just keep it the same as what we have as our "Shareplane" list UI that slides up?

> > Also a modal might not be so bad for a long list of devices... if it at
> > least matched our theme (L). :)
> 
> Do we have other dialogs that match our theme? If so, which are those
> (besides the share overlay dialog)?

Yep, all our dialogs match the L theme. Not sure why this one is not doing it. Might be worth a check. I sent you a link to another bug in about:logins that uses L theme dialogs for example.
Flags: needinfo?(alam)
(In reply to Anthony Lam (:antlam) from comment #5)
> How hard would it be to just keep it the same as what we have as
> our "Shareplane" list UI that slides up?

The code would probably be gross and hacky, but I think it can be done. Annoying but not too hard.

> Yep, all our dialogs match the L theme. Not sure why this one is not doing
> it. Might be worth a check. I sent you a link to another bug in about:logins
> that uses L theme dialogs for example.

Might be that the ShareOverlay Activity inherits from a different Activity - another reason for bug 1202076!
Line 127 in SendTabList.java causes the problem, "builder = new AlertDialog.Builder(context, R.style.Gecko_Dialog);". It sets some custom styling.

Anyway, about:logins uses default AlertDialog inside Prompt.java without the title, so it kinda looks the same, but in reality what you will see depends on Android version.

If you remove "Select device" title and remove that line, it will look like about:login dialog, but it still be different on various Android versions.
Flags: needinfo?(michael.l.comella)
Attached image Post patch
That looks right – thanks Sergej!
Flags: needinfo?(michael.l.comella)
Assignee: nobody → michael.l.comella
Bug 1203628 - Don't use gecko style on share overlay dialog. r=sebastian
Attachment #8663212 - Flags: review?(s.kaspari)
(btw, feel free to ask to be assigned or just post a patch in the future, Sergej!)
(In reply to Michael Comella (:mcomella) from comment #2)
> Worth noting that with 2 (or is it 3?) other synced devices, as most users
> have, the device list is shown inline so this won't affect most users.

Bug 1122977.
Comment on attachment 8663212 [details]
MozReview Request: Bug 1203628 - Don't use gecko style on share overlay dialog. r=sebastian

https://reviewboard.mozilla.org/r/19751/#review17825
Attachment #8663212 - Flags: review?(s.kaspari) → review+
https://hg.mozilla.org/mozilla-central/rev/b59d92a6d64b
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 44
Verified as fixed on latest Nightly(44.0a1 10/01/2015) Android using Sony Xperia Z2 (Android 5.0.2)
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.