1104930 - Create a mixin for handling updating of the video container

Status: RESOLVED FIXED

(Hello (Loop) :: Client, defect, P2)

Description

[Mark Banner (:standard8)]

In all our media based views we've got updateVideoContainer to update the video on resize or orientation change.

We should create a mixin to share that.

Comment 1

[Mark Banner (:standard8)]

Attachment: Create a mixin for handling updating of the video container.

I've pulled this forward, as it should make handling the changes in bug 1093780 easier, as it'll centralize the locations of these setup values.

Comment 2

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

Comment on attachment 8550247 [details] [diff] [review]
Create a mixin for handling updating of the video container.

Review of attachment 8550247 [details] [diff] [review]:
-----------------------------------------------------------------

Looks great, much cleaner. r=NiKo` with minor comments addressed.

::: browser/components/loop/content/shared/js/mixins.js
@@ +186,5 @@
> +     * Returns the default configuration for publishing media on the sdk.
> +     *
> +     * @param {Boolean} publishVideo True to publish video initially
> +     */
> +    getDefaultPublisherConfig: function(publishVideo) {

Let's use an options object to avoid falling in the Boolean trap[0].

[0]: http://ariya.ofilabs.com/2011/08/hall-of-api-shame-boolean-trap.html

::: browser/components/loop/test/desktop-local/conversationViews_test.js
@@ +93,5 @@
>      fakeWindow = {
>        navigator: { mozLoop: fakeMozLoop },
> +      close: sinon.stub(),
> +      addEventListener: sinon.stub(),
> +      removeEventListener: sinon.stub()

As we're not really using these two stubs, maybe we could just use a noop function.

::: browser/components/loop/test/shared/mixins_test.js
@@ +207,5 @@
> +      rootObject = {
> +        events: {},
> +        addEventListener: function(eventName, listener) {
> +          this.events[eventName] = listener;
> +        },

Love this!

@@ +215,5 @@
> +      };
> +
> +      sharedMixins.setRootObject(rootObject);
> +
> +

Nit: One blank line too many.

Comment 3

[Mark Banner (:standard8)]

https://hg.mozilla.org/integration/fx-team/rev/68fbd452c2d9

Comment 4

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/68fbd452c2d9