1203098 - Option to start an audio-only call to a contact is missing

Status: VERIFIED FIXED

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

Description

[Mark Banner (:standard8)]

This was missed on the original UX refresh mock-ups. The menu for contacts should have two options at the top for "Video Conversation" and "Audio-Only Conversation".

Comment 1

[Mark Banner (:standard8)]

Attachment: Option to start an audio-only call to a contact is missing from the Loop contact menu.

Bug 1183618 removed the options as they weren't in the spec, but I confirmed with Sevaan and they're still wanted.

The patch adds back the removed display of the menu items - the code behind them to make the video/audio call is still there.

The changes for "blocked" are because react was complaining that the required prop wasn't being supplied to the dropdown.

I've written extended tests for the menu options in the best way possible given the current code layout. Really this code needs a bit of refactoring.

The additional changes in the test files are:

- start using fakeMozLoop a bit more, to aid removal of navigator.mozLoop
- create the fakeManyContacts array as a clone each time to avoid side-effects between tests

Comment 2

[Mark Banner (:standard8)]

Note: I've filed bug 1203529 on a bad display for audio-only calls. Its a separate issue, and one we need to fix for 42.

Comment 3

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 8659247 [details] [diff] [review]
Option to start an audio-only call to a contact is missing from the Loop contact menu.

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

Looks good, r=dmose with nits fixed.

::: browser/components/loop/content/js/contacts.jsx
@@ +177,5 @@
>        let listNode = document.getElementsByClassName("contact-list")[0];
> +      // XXX Workaround the contact-list element not being available in tests.
> +      // Really we should rework this view with the DropdownMenu mixin, which
> +      // would save some of this pain.
> +      if (!listNode) {

If you can also add a comment clarifying making assumptions about the embedded context DOM is bad, that'd be great.

@@ +305,5 @@
>      render: function() {
>        let names = getContactNames(this.props.contact);
>        let email = getPreferred(this.props.contact, "email");
>        let avatarSrc = navigator.mozLoop.getUserAvatar(email.value);
> +      let blocked = "blocked" in this.props.contact && this.props.contact.blocked;

How about removing isRequested on the propType?

::: browser/components/loop/test/desktop-local/contacts_test.js
@@ +675,5 @@
> +          node = view.getDOMNode();
> +
> +          // Open the menu
> +          var menuButton = node.querySelector(".contact > .icons > .icon-vertical-ellipsis");
> +          TestUtils.Simulate.click(menuButton);

Can you switch to something less fragile than child selectors for both menuButton and contactMenu?

Comment 4

[Pulsebot]

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

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f22ddc342818

Comment 6

[Ada [:adalucinet]]

Reproduced with Nightly 2015-09-06
Verified fixed with latest 43.0a1 (from 2015-09-17), across platforms [1].

[1] Ubuntu 14.04 32-bit, Mac OS X 10.9.5 and Windows 7 64-bit