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

RESOLVED FIXED in Firefox 44

Status

()

Firefox for Android
Overlays
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: antlam, Assigned: mcomella, Mentored)

Tracking

(Blocks: 1 bug)

unspecified
Firefox 44
All
Android
Points:
---

Firefox Tracking Flags

(firefox44 verified)

Details

(Whiteboard: [lang=java])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(4 attachments)

(Reporter)

Description

3 years ago
Created attachment 8659379 [details]
Screenshot_2015-09-10-10-03-22.png

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
(Reporter)

Comment 1

3 years ago
Created attachment 8659380 [details]
prev_shareoverlay_device.png
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]
(Reporter)

Comment 3

3 years ago
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)
(Reporter)

Comment 5

3 years ago
(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!

Comment 7

3 years ago
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)
Created attachment 8663211 [details]
Post patch

That looks right – thanks Sergej!
Flags: needinfo?(michael.l.comella)
Assignee: nobody → michael.l.comella
Created attachment 8663212 [details]
MozReview Request: Bug 1203628 - Don't use gecko style on share overlay dialog. r=sebastian

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
Last Resolved: 3 years ago
status-firefox44: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 44

Comment 15

3 years ago
Verified as fixed on latest Nightly(44.0a1 10/01/2015) Android using Sony Xperia Z2 (Android 5.0.2)
status-firefox44: fixed → verified
You need to log in before you can comment on or make changes to this bug.