Closed Bug 973407 Opened 9 years ago Closed 9 years ago

[Messages] Reduce reflows when element heights are updated


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

Not set


(Not tracked)



(Reporter: yor, Assigned: yor)



(1 file)

46 bytes, text/x-github-pull-request
: review+
: feedback+
Details | Review
When element heights need to be updated, several reflows are triggered.
This happens when opening the Thread screen and composing a new message.

Analyzing the code, there are a few specific tweaks possible:

1. updateSubjectHeight() already calls updateElementsHeight() so messageComposerInputHandler() does not need to call updateElementsHeight().

2. cancelEdit() should call updateElementsHeight() only if necessary, i.e., don't call if not in Edit mode

3. updateInputMaxHeight() is usually called before updateElementsHeight() so we should rearrange the code in updateElementsHeight to avoid an extra reflow
Assignee: nobody → yor
Summary: Reduce reflows on SMS app when element heights are updated → [Messages] Reduce reflows when element heights are updated
Yan, I'm fine with any quick win here, but please also look at bug 949457.
Attached file Pull request
Julien, bug 949457 certainly is a better fix.  With the proposed fix here, the number of reflows in updateElementsHeight() went from between 2-5 (5 when opening and closing composer, 2-3 when bringing up the keyboard) to 1.

Will let you decide if it's worth landing this patch.
Attachment #8377330 - Flags: feedback?(felash)
If we have only 1 reflow and it's ready, then it's definitely worth it.

reviewing now.
Comment on attachment 8377330 [details] [review]
Pull request

good stuff, let's land this

please fix the nits and regressions, and add one or 2 unit tests if possible.

Thanks a lot, I can really feel the difference!
Attachment #8377330 - Flags: feedback?(felash) → feedback+
Comment on attachment 8377330 [details] [review]
Pull request

Fixed regression and made changes based on feedback.
Attachment #8377330 - Flags: review?(felash)
Comment on attachment 8377330 [details] [review]
Pull request

r=me, thanks

I think this patch enters in the "perf improvement" use case for landing stuff, so let's land this!
Attachment #8377330 - Flags: review?(felash) → review+
Note that there is a nit, so please land with a green travis.

Also, please file another bug to do an integration tests about reflows.
Please rewrite the commit log before merging, as Rick asked you already.
The commit log should read:

Bug 973407 - [Messages] Reduce reflows when element heights are updated r=julien
Fixed jshint issues.  Updated commit log.  Filed ticket for integration tests - Bug 975499.
Then just land it, you got r+ and it's a performance-related patch :) Do you have the rights to do it yourself?
No I don't have permission.
merged in master: c95013dfe4c40e4d93bc7c5641936b981cf649a8
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.