854417 - [MMS][SMS] Refactor the ThreadListUI rendering method: do not empty the entire list on every render

Status: RESOLVED DUPLICATE of bug 927783

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

Description

[Rick Waldron [:rwaldron]]

Currently the ThreadListUI.renderThreads method will clear the entire list before adding any new threads or updating existing threads, this may be inefficient. 

1. new threads can be inserted at the top of the list 
2. existing threads that are receiving updates can be removed and (re) inserted at the top of the list.

Comment 1

[Jan Jongboom [:janjongboom] (inactive)]

I prototyped something here: https://github.com/mozilla-b2g/gaia/pull/9164

Comment 2

[Rick Waldron [:rwaldron]]

As Julien mentioned in the PR, I was holding off on this until the cursor for getThreads was completely available. 

I don't know what the Mozilla etiquette for taking tickets that are assigned to others is, but in most projects it's considered a best practice to ask the owner before assuming control.

Comment 3

[Jan Jongboom [:janjongboom] (inactive)]

Hi Rick, sorry if you're taking this the wrong way, but it was just the outcome of working on other issues in the messaging application, where I hit the redraw problem as well, f.e. the edit mode and receiving a new message, where Borja showed me a problem last Friday. Plus while working on the multi-message channels that I proposed in the mailing list where incremental updates are nicer as well. While I was working on that I branched out and put all the thread list rendering in a separate branch. 

Ergo: it was not my intention to 'steal' the issue or something, but it was the result of working on some other stuff. If you like this way: feel free to work further on it, if the cursors would solve all these problems anyway then that's totally fine with me as well.

Comment 4

[Julien Wajsberg [:julienw]]

(In reply to Rick Waldron from comment #2)
> As Julien mentioned in the PR, I was holding off on this until the cursor
> for getThreads was completely available. 
> 
> I don't know what the Mozilla etiquette for taking tickets that are assigned
> to others is, but in most projects it's considered a best practice to ask
> the owner before assuming control.

Mozilla etiquette is also to unassign yourself when you're not working on it _now_ ;)

Comment 5

[Corey Frang [:gnarf]]

This also needs to only allow one renderer to be happening at once.

Comment 6

[Jan Jongboom [:janjongboom] (inactive)]

@Corey, this has been implemented in bug 857964

Comment 7

[Jan Jongboom [:janjongboom] (inactive)]

Is this still relevant?

Comment 8

[Corey Frang [:gnarf]]

Every time there is an update that loads the list, it re-renders the whole thing, it seems very relevant.

This is a tech debt issue more than anything.

Comment 9

[Julien Wajsberg [:julienw]]

This is still relevant.

note that Bug 859730 is similar but also different. Here, we'd want to update only part of the list when we need to update it whereas in bug 859730 we want to put in the DOM only what's displayed (and maybe some ahead and above).

Comment 10

[Mike Lee [:mlee]]

Julien, is this still an issue?

Comment 11

[Julien Wajsberg [:julienw]]

I'll need to check, recent patches have changed this and this may be ok now.

Comment 12

[Julien Wajsberg [:julienw]]

Yep, this is obsolete now. This was done as part of the work in Bug 927783 (I asked to split the patch but it wasn't split in the end).