1031258 - Smart collection view does not use APZ

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Everything.me, defect)

Description

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

It seems like the html/css code is formatted in such a way that APZ fails.

Comment 1

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: bug1031258.patch

https://tbpl.mozilla.org/?tree=Gaia-Try&rev=d5e095f0638c29ce4dfa787f93d208a5da64b0fc

Comment 2

[Kevin Grandon :kgrandon]

Comment on attachment 8447117 [details] [diff] [review]
bug1031258.patch

Review of attachment 8447117 [details] [diff] [review]:
-----------------------------------------------------------------

Scrolling is sooo much better in collections with this. Thanks!

::: apps/collection/view.html
@@ +74,5 @@
>            </header>
>          </section>
>  
> +        <div id="content-background-image">
> +          <div id="content-background-color">

nit: can we rename this to something like: content-background-overlay?

At first glance I assumed that this was for the background-color property, and it seemed weird being on top of the image. I'd recommend a rename to potentially save people the same confusion I faced, but would not block the review on it.

Comment 3

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to Kevin Grandon :kgrandon from comment #2)
> Comment on attachment 8447117 [details] [diff] [review]
> bug1031258.patch
> 
> Review of attachment 8447117 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Scrolling is sooo much better in collections with this. Thanks!
> 
> ::: apps/collection/view.html
> @@ +74,5 @@
> >            </header>
> >          </section>
> >  
> > +        <div id="content-background-image">
> > +          <div id="content-background-color">
> 
> nit: can we rename this to something like: content-background-overlay?
> 
> At first glance I assumed that this was for the background-color property,
> and it seemed weird being on top of the image. I'd recommend a rename to
> potentially save people the same confusion I faced, but would not block the
> review on it.

I will fix the name before landing (already in my PR).

Comment 4

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

https://github.com/mozilla-b2g/gaia/commit/e120a13b630f44881fbf7a81cae769f430227fec

Comment 5

[Ran Ben Aharon [:ranbena] EverythingMe]

Thanks guys. APZ is looking good.

I am a bit uncomfortable with the nested structure of the DOM. When everything is position absolute, I guess we can have the bg as a separate element to the content. Something like:

> <header ... >
> <section id="edit-header" ... >
> <div id="background"></div> // applies image and pseudo element acts as overlay
> <div id="content" ... >


Lmk if it's relevant in your opinion.

Comment 6

[Kevin Grandon :kgrandon]

Comment on attachment 8447117 [details] [diff] [review]
bug1031258.patch

This is a small patch that greatly improves scroll performance in collections. It is needed for the vertical homescreen.

Comment 7

[Kevin Grandon :kgrandon]

2.0: https://github.com/mozilla-b2g/gaia/commit/bb9020962a11bf16b52b31899ac71f8a887b6106