Closed
Bug 945481
Opened 11 years ago
Closed 11 years ago
[Messages] Implement position:sticky for fixed headers
Categories
(Firefox OS Graveyard :: Gaia::SMS, defect, P1)
Tracking
(blocking-b2g:1.3+, b2g-v1.3 fixed)
Tracking | Status | |
---|---|---|
b2g-v1.3 | --- | fixed |
People
(Reporter: kgrandon, Assigned: kgrandon)
References
Details
(Keywords: perf, Whiteboard: [c=handeye p=2 s= u=1.3])
Attachments
(2 files)
We should migrate our Fixed Header javascript to the new position: sticky CSS position. This is also necessary for smooth headers when APZ is turned on.
Assignee | ||
Comment 1•11 years ago
|
||
Already blocks 943849, so cleaning up the dependency chain.
No longer depends on: 916315
Assignee | ||
Comment 2•11 years ago
|
||
Updated•11 years ago
|
Component: Gaia::Dialer → Gaia::SMS
Summary: [Sms] Implement position:sticky for fixed headers → [Messages] Implement position:sticky for fixed headers
Comment 3•11 years ago
|
||
Comment on attachment 8341345 [details]
Pull request - Use position:sticky for fixed headers
I'd like to get this in ASAP.
Attachment #8341345 -
Flags: review+
Assignee | ||
Comment 4•11 years ago
|
||
(In reply to Rick Waldron [:rwaldron] from comment #3) > Comment on attachment 8341345 [details] > Pull request - Use position:sticky for fixed headers > > I'd like to get this in ASAP. Us too :) Unfortunately there's a crazy dependency map of getting APZ working properly and position sticky turned on. We need to leave this un-landed until we handle all of this, but thank you very much for the review!
Comment 5•11 years ago
|
||
Is there a bug about enabling position:sticky for B2G? We should make this bug depends on it.
Assignee | ||
Updated•11 years ago
|
Whiteboard: [c=handeye p2= s= u=] → [c=handeye p=2 s= u=]
Comment 6•11 years ago
|
||
Just tried this again, and this is still not ready for prime time. Also, the headers in the threads are also made sticky, I don't know if this is due to the patch or to a gecko issue. So I'm removing the review flag for now.
Updated•11 years ago
|
Attachment #8341345 -
Flags: review+
Assignee | ||
Comment 7•11 years ago
|
||
Hi Julien. Thanks for taking a look here. I applied the patch again today, and it looks like the behavior has changed since it was originally written. My guess is that this is due to bitrot. Did you apply the patch on top of master, or take only the app from my branch? In any case I will rebase, get this in working shape again, and ask for another review.
Comment 8•11 years ago
|
||
Yeah, I unbitrotted it when I tried it on master. But what I really meant is that the whole position:sticky feature is not ready: the scroll feels more laggy than without the patch. Note that I tried on a Peak so it might be different on other devices.
Comment 9•11 years ago
|
||
(In reply to Julien Wajsberg [:julienw] from comment #8) > Yeah, I unbitrotted it when I tried it on master. But what I really meant is > that the whole position:sticky feature is not ready: the scroll feels more > laggy than without the patch. > I would like to have position: sticky in 1.4. Any lagginess can be fixed by the APZC team.
Assignee | ||
Comment 10•11 years ago
|
||
Comment on attachment 8341345 [details]
Pull request - Use position:sticky for fixed headers
Hi Julien - this has now been rebased and un-bitrotted. If you have time to give this a quick review it would be helpful. Please note, you will need to enable the layout.css.sticky.enabled preference first. You could either apply the patch of the blocked bug, or with build/config/custom-prefs.js:
user_pref("layout.css.sticky.enabled", true);
Attachment #8341345 -
Flags: review?(felash)
Comment 11•11 years ago
|
||
Comment on attachment 8341345 [details] Pull request - Use position:sticky for fixed headers I find no issue with this patch, except small graphics issues we'll need to file separately when we'll have good STR. r=me after it's unbitrotted and with a nit. Of course we need bug 945777 first, so if it takes too long and you need to unbitrot too much, we might need another review before merging.
Attachment #8341345 -
Flags: review?(felash) → review+
Assignee | ||
Comment 12•11 years ago
|
||
Landed: https://github.com/mozilla-b2g/gaia/commit/9d135e867cc9cb22adbe41f481de3aa9657e11a2
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Comment 13•11 years ago
|
||
1.3+, blocked existing 1.3 blocker Bug 943849.
blocking-b2g: --- → 1.3+
Whiteboard: [c=handeye p=2 s= u=] → [c=handeye p=2 s= u=1.3]
Target Milestone: --- → 1.4 S1 (14feb)
Updated•11 years ago
|
Priority: -- → P1
Comment 14•11 years ago
|
||
Are we sure all the fixes for position:sticky landed in 1.3?
Flags: needinfo?(mlee)
Comment 15•11 years ago
|
||
I was not able to uplift this bug to v1.3. If this bug has dependencies which are not marked in this bug, please comment on this bug. If this bug depends on patches that aren't approved for v1.3, we need to re-evaluate the approval. Otherwise, if this is just a merge conflict, you might be able to resolve it with: git checkout v1.3 git cherry-pick -x -m1 9d135e867cc9cb22adbe41f481de3aa9657e11a2 <RESOLVE MERGE CONFLICTS> git commit
Flags: needinfo?(kgrandon)
Comment 16•11 years ago
|
||
(In reply to Julien Wajsberg [:julienw] from comment #14) > Are we sure all the fixes for position:sticky landed in 1.3? Kevin please confirm.
Flags: needinfo?(mlee)
Assignee | ||
Comment 17•11 years ago
|
||
(In reply to Julien Wajsberg [:julienw] from comment #14) > Are we sure all the fixes for position:sticky landed in 1.3? It should work, but I was hoping to have some of the issues in bug 916315 landed for 1.4. I am a bit concerned about this being available to all third party apps in 1.3 with it not being super polished.
Flags: needinfo?(kgrandon)
Assignee | ||
Comment 18•11 years ago
|
||
Comment 19•11 years ago
|
||
I'm also quite concerned about this. I think the risk outweights greatly the benefit here.
Assignee | ||
Comment 20•11 years ago
|
||
(In reply to Julien Wajsberg [:julienw] from comment #19) > I'm also quite concerned about this. I think the risk outweights greatly the > benefit here. Yeah... apparently they all got flipped to 1.3 due to APZ. Worst case scenario - it's not hard to back these out if needed.
Assignee | ||
Comment 21•11 years ago
|
||
Landed in 1.3: https://github.com/mozilla-b2g/gaia/commit/a094ecdcb71bb8fc73afceeb32001fdc616d6945
Assignee | ||
Updated•11 years ago
|
status-b2g-v1.3:
--- → fixed
Comment 24•11 years ago
|
||
I have no explanation for why I completely missed https://bugzilla.mozilla.org/show_bug.cgi?id=945481#c11 accept that I was reviewing from my phone maybe I scrolled past it? Sorry for the noise!
You need to log in
before you can comment on or make changes to this bug.
Description
•