937291 - contacts triggers large reflows rendering while scrolling

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Contacts, defect, P1)

Description

[Ben Kelly [:bkelly, not reviewing]]

While working on bug 936908 I noticed that contacts seems to trigger relatively large reflows (30ms to 60ms) while scrolling.  This is occurring because we render contacts as we scroll.

After talking with dholbert it seems we either need to reduce the DOM or constrain the reflow.  At this point we don't have many options to reduce the DOM size, but we can reduce the size of the reflow to a smaller area.

Adding overflow: hidden to the group list sections reduces the reflows down to ~10ms or less.

Comment 1

[Ben Kelly [:bkelly, not reviewing]]

Attachment: Pull request at https://github.com/mozilla-b2g/gaia/pull/13593

Jose, this patch limits reflows when we render contacts while scrolling but using overflow:hidden on the individual group lists.

Comment 2

[Jose Manuel Cantera]

Comment on attachment 830380 [details] [review]
Pull request at https://github.com/mozilla-b2g/gaia/pull/13593

good work thanks

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

Thanks for the review!  Green travis, so landed:

  https://github.com/mozilla-b2g/gaia/commit/5b14442e76b6a03255ed7b94185378f983704f79

Comment 4

[Ben Kelly [:bkelly, not reviewing]]

Uplifted to v1.2:

  https://github.com/mozilla-b2g/gaia/commit/3c73873c8e8ddc8cea9665db0317785e926b286b

Comment 5

[Ben Kelly [:bkelly, not reviewing]]

Reverted due to regression in checkboxes getting clipped in selection view:

master:  6728e0746e9302079564993f6e11317d3e4bf39b
v1.2:    e54a2d40e52bd66e95ed37ae8705afe4d55ab9ea

Comment 6

[Ben Kelly [:bkelly, not reviewing]]

Attachment: Pull request at https://github.com/mozilla-b2g/gaia/pull/13886

Francisco, here is another attempt at limiting the overflow for the list.  In order to avoid clipping the checkboxes I moved them 1 rem to the right so that they no longer overhang the edge of the list section.

Is this slight move of the checkbox to the right acceptable?

I also tried just using overflow-y:hidden, but that still seemed to clip the checkboxes along the x-axis for some unknown reason.

Comment 7

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8335522 [details] [review]
Pull request at https://github.com/mozilla-b2g/gaia/pull/13886

Actually, I want to look at this further.  There are other visual differences.  For example, the header underline is shorter when its in the group list vs when presented in the fixed header.

Comment 8

[Ben Kelly [:bkelly, not reviewing]]

Attachment: Pull request at https://github.com/mozilla-b2g/gaia/pull/13925

Francisco, can you review this patch adding overflow:hidden each group <section> element?  This greatly reduces reflows when we are rendering just-in-time while scrolling.  Contacts scrolling is currently a koi+ issue, so I would like to land and uplift as soon as possible.

This landed once and had to be backed out due to clipping of the checkboxes in the export selection mode.

This new patch fixes the checkbox issue by moving some of the screen padding out of the parent #groups-container section and into each individual group section.

I verified that the list looks correctly in normal list mode, search mode, selection mode, and search selection mode.

Thanks!

Comment 9

[Francisco Jordano [:arcturus] [:francisco]]

(In reply to Ben Kelly [:bkelly] from comment #8)
> 
> I verified that the list looks correctly in normal list mode, search mode,
> selection mode, and search selection mode.
> 
> Thanks!

That's a lot of modes!

Thanks a lot for taking care of this, doing the review right now!

Comment 10

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 8336370 [details] [review]
Pull request at https://github.com/mozilla-b2g/gaia/pull/13925

Tried on the phone and works perfectly.

r+

Thanks!

Comment 11

[Ben Kelly [:bkelly, not reviewing]]

Thanks for the quick review!  Merged:

  https://github.com/mozilla-b2g/gaia/commit/93a5242bbdd4f98a1bd4259abc64afdebab34c34

Comment 12

[Ben Kelly [:bkelly, not reviewing]]

Uplifted to v1.2:   3c05682dfe8463a1f64740379009fb4363c027b4