Closed Bug 1080820 Opened 5 years ago Closed 5 years ago

[Messages][RTL] Sticky header does not take the whole width

Categories

(Firefox OS Graveyard :: Gaia::SMS, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(b2g-v2.2 verified, b2g-master verified)

VERIFIED FIXED
2.1 S8 (7Nov)
Tracking Status
b2g-v2.2 --- verified
b2g-master --- verified

People

(Reporter: julienw, Assigned: steveck)

References

Details

(Whiteboard: [p=1])

Attachments

(2 files)

46 bytes, text/x-github-pull-request
julienw
: review+
kgrandon
: review+
Details | Review
73.59 KB, image/png
Details
In the thread list, the sticky header does not take the whole width. As a result, we see the contact pictures aside the sticky header when we scroll.

In LTR we see also a related issue but this is more visible in RTL.
Whiteboard: [p=1]
Target Milestone: --- → 2.1 S8 (7Nov)
QA Contact: schung
Assignee: nobody → schung
QA Contact: schung
Attached file Link to github
Hi, Julien, it simply css change that adjust the header position and with white background color. BTW I'm fine if you prefer -moz-margin-end for scrollbar.
Attachment #8514901 - Flags: review?(felash)
Comment on attachment 8514901 [details] [review]
Link to github

Because of the background color we don't see the sticky header while loading. We need to fix this.

There are IMO 3 solutions I outlined in github (there may be others :) ). I don't have a real preference so pick the one you prefer :)
Attachment #8514901 - Flags: review?(felash)
Comment on attachment 8514901 [details] [review]
Link to github

Thanks for the suggestion, I followed your 2nd option, with a little bit changes since I saw some possible regression in call log: If we set sticky header to white background by default, we will need to force to update when call log filter switch. Otherwise it's possible to see the blank header at top. And I even hide the header if there is no need to display it. Since this patch also affect dialer, it defiantly need dialer team's feedback as well.
Attachment #8514901 - Flags: feedback?(felash)
Attachment #8514901 - Flags: feedback?(drs.bugzilla)
Comment on attachment 8514901 [details] [review]
Link to github

Gave some feedback.

I think we can do it without impacting the call log.

Please ask review from Kevin Grandon as well once you're ready (he's peer for shared and I think he helped Vivien initially writing this)
Attachment #8514901 - Flags: feedback?(felash)
Comment on attachment 8514901 [details] [review]
Link to github

Clearing feedback based on comment 4. Please ask again if you need it.
Attachment #8514901 - Flags: feedback?(drs.bugzilla)
Comment on attachment 8514901 [details] [review]
Link to github

If we can leave call log as it is, I came out another solution that didn't touch the call log and even sticky header. It only need changes the header styling in message app, and the original header is still visible until sticky header ready. But the drawback is it will need additional css attribute selector.
Attachment #8514901 - Flags: review?(felash)
Comment on attachment 8514901 [details] [review]
Link to github

Hi Kevin, since this sticky header is implemented by you, it would be helpful if you could have suggestion about this change: We add a new class to represent the header content is set, but only message app will utilize this class(call log will not broken because it doesn't need to know it). Do you satisfy with this change?
Attachment #8514901 - Flags: review?(kgrandon)
Comment on attachment 8514901 [details] [review]
Link to github

r=me unless Kevin would like another solution
Attachment #8514901 - Flags: review?(felash) → review+
Comment on attachment 8514901 [details] [review]
Link to github

So I was unfortunately unable to test this on a device as reference workloads seem to have broken for me *sad*. The code looks simple enough though, so I am fine putting my R+ stamp on this.

Please squash before landing, thanks!
Attachment #8514901 - Flags: review?(kgrandon) → review+
Thanks!
In master: 32bfbc4b0086615374eaa8f4bccac19b23dba630
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
See Also: → 1102325
Duplicate of this bug: 1120348
Test case has been added in moztrap:

https://moztrap.mozilla.org/manage/case/15372/
Flags: in-moztrap+
Attached image verify as pass.png
This issue has been verified as pass on latest build of Flame 2.2&3.0 and Nexus 5 2.2&3.0 by STRs:
Prerequisite: Have some message threads in device.
1. Set system language as Arabic.
2. Launch Message and observe the view.
3. Scroll the thread list.
Result:The message list view is shown correctly and the sticky header takes the whole width.
See attachment:verify as pass.png
Rate:0/3

Device: Flame 2.2 (pass)
Build ID               20150525002504
Gaia Revision          144673a413586f98b5e2c27b781c1a539611f754
Gaia Date              2015-05-25 02:01:14
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/c4db2af40b1b
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150525.041143
Firmware Date          Mon May 25 04:11:54 EDT 2015
Bootloader             L1TC000118D0

Device: Flame 3.0 (pass)
Build ID               20150525160205
Gaia Revision          5bcc08a732163087999251b523e3643db397412c
Gaia Date              2015-05-24 14:44:40
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/b6623a27fa64
Gecko Version          41.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150525.192755
Firmware Date          Mon May 25 19:28:07 EDT 2015
Bootloader             L1TC000118D0

Device: Nexus 5 2.2 (pass)
Build ID               20150525002504
Gaia Revision          144673a413586f98b5e2c27b781c1a539611f754
Gaia Date              2015-05-25 02:01:14
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/c4db2af40b1b
Gecko Version          37.0
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150525.041303
Firmware Date          Mon May 25 04:13:19 EDT 2015
Bootloader             HHZ12f

Device: Nexus 5 3.0 (pass)
Build ID               20150525160205
Gaia Revision          5bcc08a732163087999251b523e3643db397412c
Gaia Date              2015-05-24 14:44:40
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/b6623a27fa64
Gecko Version          41.0a1
Device Name            hammerhead
Firmware(Release)      5.1
Firmware(Incremental)  eng.cltbld.20150525.192207
Firmware Date          Mon May 25 19:22:24 EDT 2015
Bootloader             HHZ12f
Status: RESOLVED → VERIFIED
QA Whiteboard: [MGSEI-Triage+]
You need to log in before you can comment on or make changes to this bug.