Attaching sample UX from docs

Updating to match trello

We need a mockup with the state of the dropdown menu when the chevron is clicked. For example this view https://people.mozilla.org/~dhenein/labs/loop-mvp-spec/#start-call-account

For the button, use the hover/active states defined in the spec and native platform-styling for the dropdown: http://cl.ly/image/261j2I280u2m

(In reply to mmaslaney from comment #4) > For the button, use the hover/active states defined in the spec and native > platform-styling for the dropdown: > > http://cl.ly/image/261j2I280u2m Thanks! I still have some questions. The mockup is not similar to the native platform style [0] should I follow your mockup ? What should the hover state background color be ? (at least in osx that color is customizable) In the MVP spec [1] the answer and ignore buttons are at the bottom of the panel and there is no room for the dropdown panel. Should I raise the buttons as in your mockup ? [0] http://i.imgur.com/TuGH0x8.png [1] https://people.mozilla.org/~dhenein/labs/loop-mvp-spec/#call-incoming-unknown

(In reply to Andrei Oprea [:andreio] from comment #5) > (In reply to mmaslaney from comment #4) > > For the button, use the hover/active states defined in the spec and native > > platform-styling for the dropdown: > > > > http://cl.ly/image/261j2I280u2m > > Thanks! I still have some questions. > > The mockup is not similar to the native platform style [0] should I follow > your mockup ? No, please use the native platform styling. > What should the hover state background color be ? (at least in osx that > color is customizable) I would leave the hover state native and avoid customization. > In the MVP spec [1] the answer and ignore buttons are at the bottom of the > panel and there is no room for the dropdown panel. Should I raise the > buttons as in your mockup ? In this case Yes, I would raise the buttons. > [0] http://i.imgur.com/TuGH0x8.png > [1] > https://people.mozilla.org/~dhenein/labs/loop-mvp-spec/#call-incoming-unknown

Comment on attachment 8460354 [details] [diff] [review] Implement revoke generated URL for incoming call view Review of attachment 8460354 [details] [diff] [review]: ----------------------------------------------------------------- This is looking great; once we've sanded off a few rough edges, it should be ready to go. ::: browser/components/loop/content/js/client.js @@ +165,5 @@ > + cb(null); > + > + this.mozLoop.noteCallUrlExpiry((new Date()).getTime() / 1000); > + } catch (err) { > + console.log("Error requesting call info", err); Change this to "Error deleting call info" ::: browser/components/loop/content/js/conversation.jsx @@ +79,3 @@ > var conversationPanelClass = "incoming-call " + this._getTargetPlatform(); > + var cx = React.addons.classSet; > + var dropdownMenu = cx({ A different name? Maybe dropdownMenuClasses? @@ +88,5 @@ > <h2>{__("incoming_call")}</h2> > <div className="button-group"> > + <div className={btnClassDecline} onClick={this._handleDecline}> > + <span className="btn__chevron-text"> > + {__("incoming_call_decline_button")} incoming_call_ignore_button if this string something with the same semantics. @@ +90,5 @@ > + <div className={btnClassDecline} onClick={this._handleDecline}> > + <span className="btn__chevron-text"> > + {__("incoming_call_decline_button")} > + </span> > + <span className="btn__chevron" If we can stick with prevailing local style (hyphens rather than underscores) for classes, I think that'll make the code more coherent. @@ +95,5 @@ > + onMouseEnter={this._toggleDeclineMenu} > + onMouseLeave={this._toggleDeclineMenu}> > + <ul className={dropdownMenu}> > + <li className="btn-block" onClick={this._handleIgnoreBlock}> > + Decline and Block Ignore and Block; needs localization @@ +150,5 @@ > "call/accept": "accept", > "call/decline": "decline", > "call/ongoing": "conversation", > + "call/ended": "ended", > + "call/block": "ignoreAndBlock" Please make the route use the same more explicit description of what it's doing: call/ignoreAndBlock. @@ +213,5 @@ > > /** > + * Decline & block an incoming call > + */ > + ignoreAndBlock: function() { Please change the comment to use the word ignore, and add jsdoc descript (@note i guess) about where the token comes from and what we're making happen. ::: browser/components/loop/content/js/panel.jsx @@ +175,5 @@ > } else { > + var callUrl = callUrlData.callUrl || callUrlData.call_url; > + // XXX the current server vers does not implement the callToken field > + // but it exists in the API. This workaround should be removed in the future > + var token = callUrlData.callToken || callUrl.split('/').pop(); It'd be great to use URL() here if that's practical/easy. @@ +180,2 @@ > > + navigator.mozLoop.setLoopCharPref('loopToken', token); This needs some sort of documentation about why it's doing this. ::: browser/components/loop/content/shared/css/common.css @@ +64,5 @@ > /* Buttons */ > > .btn { > display: inline-block; > + position: relative; Let's avoid fragility as discussed by making two sibling boxes inside a parent. ::: browser/components/loop/content/shared/css/conversation.css @@ +188,5 @@ > +} > + > +.decline-block-menu { > + /* Align the menu with the chevron */ > + bottom: -34px; At least XXX this with a theory about doing better if it's not easily fixable using containing boxes and justify or align properties or whatever. ::: browser/components/loop/test/desktop-local/client_test.js @@ +108,5 @@ > + > + sinon.assert.calledOnce(callback); > + sinon.assert.calledWithMatch(callback, sinon.match(function(err) { > + return /400.*invalid token/.test(err.message); > + })); We now have enough instances of this generic matcher that I think it's worth hoisting this out both to save code, but also to make the calling code more readable. ::: browser/components/loop/test/desktop-local/conversation_test.js @@ +283,5 @@ > + sinon.assert.calledOnce(log); > + sinon.assert.calledWithExactly(log, fakeError); > + }); > + > + it("close the window", function() { This wants to start with the word "should". :-) @@ +389,5 @@ > sinon.assert.calledOnce(model.trigger); > sinon.assert.calledWith(model.trigger, "decline"); > }); > }); > + We also need to test and implement that clicking on the chevron drops down the menu. You'll probably need to experiment a bit and/or ping sevaan about how to make the menu disappear.

Comment on attachment 8460544 [details] declineblock.jpg Looks good :andreio! Thanks.

Comment on attachment 8462010 [details] [diff] [review] Implement revoke generated URL for incoming call view Review of attachment 8462010 [details] [diff] [review]: ----------------------------------------------------------------- r=dmose in person; comments addressed during review. No ux-review required as we have a moratorium until the ux/visual refresh lands.

Comment on attachment 8462879 [details] [diff] [review] Implement revoke generated URL for incoming call view, r=dmose With review comments addressed.

Does this have sufficient in-testsuite coverage or will this require manual smoketesting?

Removing qe-verify since this is basically gone away now.