Closed Bug 1196077 Opened 9 years ago Closed 9 years ago

[email] Refactor message_list into some smaller custom elements

Categories

(Firefox OS Graveyard :: Gaia::E-Mail, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jrburke, Assigned: jrburke)

Details

Attachments

(1 file)

There is some front-end only refactoring that can be done of message_list and some base mixins to support card custom elements being made up of smaller email custom elements. 

The refactoring was part of the general convoy (email conversations) effort, but landing them now will help amortize the overall convoy review hit and allows for some other front end refactors that do not depend on convoy to occur in the meantime.
Comment on attachment 8649616 [details] [review]
[gaia] jrburke:bug1196077-email-message-list-refactor > mozilla-b2g:master

See github pull request for review notes. Not urgent for review, just when there is a window in between conversations work. 

Would likely want to land this in master before changing the front end convoy branch to update to any new backend convoy updates, just to avoid too much front end branch drifting. This pull request now has fixes to some of the early commits in the front end convoy branch.
Attachment #8649616 - Flags: review?(bugmail)
Comment on attachment 8649616 [details] [review]
[gaia] jrburke:bug1196077-email-message-list-refactor > mozilla-b2g:master

r=asuth as-is, feel free to address nits.  There are some idiomatic questions I have in there, but there is no need to wait on landing/anything or even change how things are done.

I think a lot of it boils down to the interaction with DOM elements potentially happening via three means:
- via JS properties/setters/getters
- via explicit HTML attributes
- via CSS classes
And the complexities of cloneNode() and custom elements (and I assume JS expandos not transferring when cloneNode is invoked).

Back in the XBL days there was a critical difference between HTML attributes and the JS-exposed values because the XBL constructors would fire at unpredictable times (which I suppose our custom elements also do right now until that one bug gets fixed.)  A nice thing about XBL and explicit attributes (besides the safe life-cycles) is that it made the static-ish/dynamic-ish interactions potentially more clear.  Using explicit HTML attributes was something that could be seen inside the JS (because of magic or constructors I guess, some of this is hazy), but CSS could also get at the stuff.

In the potential idiom of our JS code assigning to magical setters to cause other JS code to fire to manipulate the DOM, and especially with that state not persisted, it gets a bit awkward.  (And arguably even further removed from the react.js ideal of having the DOM state as a functional transform on underlying state that is updated coherently.  When we have a setter directly manipulate DOM as a side-effect, we're not even tracking what the underlying conceptual state is.)

I don't think the HTML attribute thing is practical since it probably implies mutation observers, which are somewhat nuts, but it'd be great if we could pick an idiom that makes it ridiculously clear when an action is going to cascade into DOM manipulations and is not some type of inert data-structure state-stashing.

In any event, this is a great step forward in terms of cleaning up our codebase.  Woo!
Attachment #8649616 - Flags: review?(bugmail) → review+
Keywords: checkin-needed
I will do the merge myself, automation on pull request is passing, and will help save people from some manual work I can do.
Keywords: checkin-needed
Merged in gaia master:
https://github.com/mozilla-b2g/gaia/commit/54245dc8c326c788b0aa8c1aedcd6f1b076fa262
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: