996526 - Loop Backbone views should carry their own templates

Status: VERIFIED FIXED

(Hello (Loop) :: Client, defect)

Description

[Nicolas Perriault (:NiKo`) — needinfo me if you need my attention]

In order to avoid code duplication as well as avoiding unexpected side effects with DOM static templates, our Backbone views should carry their own templates; so then we can handle and maintain them in a single place, the View object.

Neither webL10n nor mozL10n works natively with this though, but we could translate view template fragment at rendering time.

Comment 1

[Nicolas Perriault (:NiKo`) — needinfo me if you need my attention]

Attachment: https://github.com/mozilla/loop-client/pull/20

This loop-client patch refactor all the views so they:

- carry define and carry their own template;
- inherits from a new L10nView base class which provide automatic template frament translation using {moz|web}L10n.

Comment 2

[Nicolas Perriault (:NiKo`) — needinfo me if you need my attention]

Attachment: https://github.com/adamroach/gecko-dev/pull/15

This patch updated the Loop desktop component views to:

- define and carry their own templates;
- benefit from the new L10nView shared class (see loop-client patch).

Comment 3

[Mark Banner (:standard8)]

Comment on attachment 8406845 [details] [review]
https://github.com/mozilla/loop-client/pull/20

I've done an initial pass through the main code of this one and it seemed reasonable, but I haven't checked all effects/tested thoroughly etc.

As I'm likely not going to get any more time until Tuesday, passing this to Dan.

Comment 4

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 8406848 [details] [review]
https://github.com/adamroach/gecko-dev/pull/15

r=dmose; comments in the PR

Comment 5

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 8406845 [details] [review]
https://github.com/mozilla/loop-client/pull/20

r=dmose, once the comments in the PR have been addressed.

Comment 6

[Nicolas Perriault (:NiKo`) — needinfo me if you need my attention]

Landed https://github.com/mozilla/loop-client/pull/20: https://github.com/mozilla/loop-client/commit/243f5c5eb380ce563df16397fd5f0fc6986900fd

Comment 7

[Nicolas Perriault (:NiKo`) — needinfo me if you need my attention]

Landed https://github.com/adamroach/gecko-dev/pull/15: https://github.com/adamroach/gecko-dev/commit/1c237a156104170870098745355ffa27208fdb05

Comment 9

[Mark Banner (:standard8)]

https://hg.mozilla.org/projects/elm/rev/e891781d6324
https://hg.mozilla.org/projects/elm/rev/b4a130ba26da

Comment 10

[Mark Banner (:standard8)]

https://hg.mozilla.org/mozilla-central/rev/e891781d6324
https://hg.mozilla.org/mozilla-central/rev/b4a130ba26da

Comment 11

[u279076]

Does this need automated or manual testing?

Comment 12

[Nicolas Perriault (:NiKo`) — needinfo me if you need my attention]

(In reply to Anthony Hughes, QA Mentor (:ashughes) from comment #11)
> Does this need automated or manual testing?

We recently switched to React for views, and they're all unit tested.

Comment 13

[u279076]

Thanks Nicolas, marking verified fixed.