From 9e907da312ff942288e693bc14454aa13ada4d63 Mon Sep 17 00:00:00 2001 From: Mike de Boer Date: Tue, 17 Feb 2015 15:03:59 +0100 Subject: [PATCH] Bug 1131581: move screensharing options into a dropdown anchored to the screenshare toolbar button. r=Standard8 --- .../loop/content/shared/css/conversation.css | 38 +++++++++++++++---- .../loop/content/shared/img/icons-10x10.svg | 6 ++- .../loop/content/shared/img/icons-16x16.svg | 5 +++ .../loop/content/shared/js/mixins.js | 25 ++++++++++++ .../loop/content/shared/js/views.js | 37 ++++++++++++++---- .../loop/content/shared/js/views.jsx | 37 ++++++++++++++---- .../components/loop/test/shared/views_test.js | 33 ++++++++++++---- .../en-US/chrome/browser/loop/loop.properties | 1 + 8 files changed, 153 insertions(+), 29 deletions(-) diff --git a/browser/components/loop/content/shared/css/conversation.css b/browser/components/loop/content/shared/css/conversation.css index fcd768890f1..01f0f54d7f9 100644 --- a/browser/components/loop/content/shared/css/conversation.css +++ b/browser/components/loop/content/shared/css/conversation.css @@ -17,6 +17,8 @@ /* desktop version */ .fx-embedded .conversation-toolbar { + /* required to have dropdowns float atop the .room-invitation-overlay: */ + z-index: 1020; position: absolute; top: 0; left: 0; @@ -40,13 +42,13 @@ height: 100%; } -.conversation-toolbar li { +.conversation-toolbar > li { float: left; font-size: 0; /* prevents vertical bottom padding added to buttons in google chrome */ } - .standalone .conversation-toolbar li { + .standalone .conversation-toolbar > li { /* XXX Might make sense to use relative units here. */ margin-right: 16px; @@ -165,7 +167,8 @@ background-color: transparent; opacity: 1; } -.conversation-toolbar .transparent-button:hover { +.conversation-toolbar .transparent-button:hover, +.conversation-toolbar .transparent-button.menu-showing { background-color: rgba(255,255,255,.35); opacity: 1; } @@ -208,9 +211,15 @@ /* Screen share button */ .btn-screen-share { - /* XXX Replace this with the real button: bug 1126286 */ + position: relative; background-image: url(../img/icons-16x16.svg#screen-white); background-size: 16px 16px; + width: 42px; +} + +/* Make room for the chevron. */ +.conversation-toolbar .btn-screen-share:not(.active) { + background-position: 5px center; } .btn-screen-share.active { @@ -220,7 +229,21 @@ } .btn-screen-share.disabled { - /* XXX Add css here for disabled state: bug 1126286 */ + background-image: url(../img/icons-16x16.svg#screen-disabled); +} + +.btn-screen-share .chevron { + background-image: url(../img/icons-10x10.svg#dropdown-white); + background-size: 10px 10px; + position: absolute; + right: 2px; + top: 8px; + width: 10px; + height: 10px; +} + +.btn-screen-share.disabled .chevron { + background-image: url(../img/icons-10x10.svg#dropdown-disabled); } .fx-embedded .remote_wrapper { @@ -364,9 +387,10 @@ background-color: #111; } -.conversation-window-dropdown li { +.conversation-window-dropdown > li { padding: 2px; - font-size: .9em; + font-size: .7rem; + white-space: nowrap; } /* Expired call url page */ diff --git a/browser/components/loop/content/shared/img/icons-10x10.svg b/browser/components/loop/content/shared/img/icons-10x10.svg index 2dc7a27a597..62b564b7846 100644 --- a/browser/components/loop/content/shared/img/icons-10x10.svg +++ b/browser/components/loop/content/shared/img/icons-10x10.svg @@ -26,7 +26,11 @@ use[id$="-active"] { } use[id$="-white"] { - fill: rgba(255, 255, 255, 0.8); + fill: rgba(255,255,255,0.8); +} + +use[id$="-disabled"] { + fill: rgba(255,255,255,0.4); } diff --git a/browser/components/loop/content/shared/img/icons-16x16.svg b/browser/components/loop/content/shared/img/icons-16x16.svg index 40e1ac8ef16..5229c29418b 100644 --- a/browser/components/loop/content/shared/img/icons-16x16.svg +++ b/browser/components/loop/content/shared/img/icons-16x16.svg @@ -32,6 +32,10 @@ use[id$="-red"] { use[id$="-white"] { fill: #fff; } + +use[id$="-disabled"] { + fill: rgba(255,255,255,.6); +} + diff --git a/browser/components/loop/content/shared/js/mixins.js b/browser/components/loop/content/shared/js/mixins.js index cc09b036a2b..85d01eb4873 100644 --- a/browser/components/loop/content/shared/js/mixins.js +++ b/browser/components/loop/content/shared/js/mixins.js @@ -101,6 +101,31 @@ loop.shared.mixins = (function() { componentDidMount: function() { this.documentBody.addEventListener("click", this._onBodyClick); this.documentBody.addEventListener("blur", this.hideDropdownMenu); + + var menu = this.refs.menu; + if (!menu) { + return; + } + + // Correct the position of the menu if necessary. + var menuNode = menu.getDOMNode(); + var menuNodeRect = menuNode.getBoundingClientRect(); + var bodyRect = { + height: this.documentBody.offsetHeight, + width: this.documentBody.offsetWidth + }; + + // First we check the vertical overflow. + var y = menuNodeRect.top + menuNodeRect.height; + if (y >= bodyRect.height) { + menuNode.style.marginTop = bodyRect.height - y + "px"; + } + + // Then we check the horizontal overflow. + var x = menuNodeRect.left + menuNodeRect.width; + if (x >= bodyRect.width) { + menuNode.style.marginLeft = bodyRect.width - x + "px"; + } }, componentWillUnmount: function() { diff --git a/browser/components/loop/content/shared/js/views.js b/browser/components/loop/content/shared/js/views.js index efd34613e72..e7d3f1203b6 100644 --- a/browser/components/loop/content/shared/js/views.js +++ b/browser/components/loop/content/shared/js/views.js @@ -85,6 +85,8 @@ loop.shared.views = (function(_, l10n) { * loop.shared.utils.SCREEN_SHARE_STATES */ var ScreenShareControlButton = React.createClass({displayName: "ScreenShareControlButton", + mixins: [sharedMixins.DropdownMenuMixin], + propTypes: { dispatcher: React.PropTypes.instanceOf(loop.Dispatcher).isRequired, visible: React.PropTypes.bool.isRequired, @@ -96,11 +98,14 @@ loop.shared.views = (function(_, l10n) { this.props.dispatcher.dispatch( new sharedActions.EndScreenShare({})); } else { - this.props.dispatcher.dispatch( - new sharedActions.StartScreenShare({})); + this.toggleDropdownMenu(); } }, + _handleShareWindows: function() { + this.props.dispatcher.dispatch(new sharedActions.StartScreenShare({})); + }, + _getTitle: function() { var prefix = this.props.state === SCREEN_SHARE_STATES.ACTIVE ? "active" : "inactive"; @@ -113,18 +118,36 @@ loop.shared.views = (function(_, l10n) { return null; } - var screenShareClasses = React.addons.classSet({ + var cx = React.addons.classSet; + + var isActive = this.props.state === SCREEN_SHARE_STATES.ACTIVE; + var screenShareClasses = cx({ "btn": true, "btn-screen-share": true, "transparent-button": true, - "active": this.props.state === SCREEN_SHARE_STATES.ACTIVE, + "menu-showing": this.state.showMenu, + "active": isActive, "disabled": this.props.state === SCREEN_SHARE_STATES.PENDING }); + var dropdownMenuClasses = cx({ + "native-dropdown-menu": true, + "conversation-window-dropdown": true, + "visually-hidden": !this.state.showMenu + }); return ( - React.createElement("button", {className: screenShareClasses, - onClick: this.handleClick, - title: this._getTitle()}) + React.createElement("div", null, + React.createElement("button", {className: screenShareClasses, + onClick: this.handleClick, + title: this._getTitle()}, + isActive ? null : React.createElement("span", {className: "chevron"}) + ), + React.createElement("ul", {ref: "menu", className: dropdownMenuClasses}, + React.createElement("li", {onClick: this._handleShareWindows}, + l10n.get("share_windows_button_title") + ) + ) + ) ); } }); diff --git a/browser/components/loop/content/shared/js/views.jsx b/browser/components/loop/content/shared/js/views.jsx index dbc8598c753..7bc9b3e419a 100644 --- a/browser/components/loop/content/shared/js/views.jsx +++ b/browser/components/loop/content/shared/js/views.jsx @@ -85,6 +85,8 @@ loop.shared.views = (function(_, l10n) { * loop.shared.utils.SCREEN_SHARE_STATES */ var ScreenShareControlButton = React.createClass({ + mixins: [sharedMixins.DropdownMenuMixin], + propTypes: { dispatcher: React.PropTypes.instanceOf(loop.Dispatcher).isRequired, visible: React.PropTypes.bool.isRequired, @@ -96,11 +98,14 @@ loop.shared.views = (function(_, l10n) { this.props.dispatcher.dispatch( new sharedActions.EndScreenShare({})); } else { - this.props.dispatcher.dispatch( - new sharedActions.StartScreenShare({})); + this.toggleDropdownMenu(); } }, + _handleShareWindows: function() { + this.props.dispatcher.dispatch(new sharedActions.StartScreenShare({})); + }, + _getTitle: function() { var prefix = this.props.state === SCREEN_SHARE_STATES.ACTIVE ? "active" : "inactive"; @@ -113,18 +118,36 @@ loop.shared.views = (function(_, l10n) { return null; } - var screenShareClasses = React.addons.classSet({ + var cx = React.addons.classSet; + + var isActive = this.props.state === SCREEN_SHARE_STATES.ACTIVE; + var screenShareClasses = cx({ "btn": true, "btn-screen-share": true, "transparent-button": true, - "active": this.props.state === SCREEN_SHARE_STATES.ACTIVE, + "menu-showing": this.state.showMenu, + "active": isActive, "disabled": this.props.state === SCREEN_SHARE_STATES.PENDING }); + var dropdownMenuClasses = cx({ + "native-dropdown-menu": true, + "conversation-window-dropdown": true, + "visually-hidden": !this.state.showMenu + }); return ( - +
+ +
    +
  • + {l10n.get("share_windows_button_title")} +
    • +
+
); } }); diff --git a/browser/components/loop/test/shared/views_test.js b/browser/components/loop/test/shared/views_test.js index 1771b0d7a1d..0537a8b8e93 100644 --- a/browser/components/loop/test/shared/views_test.js +++ b/browser/components/loop/test/shared/views_test.js @@ -118,8 +118,9 @@ describe("loop.shared.views", function() { state: SCREEN_SHARE_STATES.PENDING })); - expect(comp.getDOMNode().classList.contains("active")).eql(false); - expect(comp.getDOMNode().classList.contains("disabled")).eql(true); + var node = comp.getDOMNode().querySelector(".btn-screen-share"); + expect(node.classList.contains("active")).eql(false); + expect(node.classList.contains("disabled")).eql(true); }); it("should render an active share button", function() { @@ -130,11 +131,12 @@ describe("loop.shared.views", function() { state: SCREEN_SHARE_STATES.ACTIVE })); - expect(comp.getDOMNode().classList.contains("active")).eql(true); - expect(comp.getDOMNode().classList.contains("disabled")).eql(false); + var node = comp.getDOMNode().querySelector(".btn-screen-share"); + expect(node.classList.contains("active")).eql(true); + expect(node.classList.contains("disabled")).eql(false); }); - it("should dispatch a StartScreenShare action on click when the state is not active", + it("should show the screenshare dropdown on click when the state is not active", function() { var comp = TestUtils.renderIntoDocument( React.createElement(sharedViews.ScreenShareControlButton, { @@ -143,7 +145,24 @@ describe("loop.shared.views", function() { state: SCREEN_SHARE_STATES.INACTIVE })); - TestUtils.Simulate.click(comp.getDOMNode()); + expect(comp.state.showMenu).eql(false); + + TestUtils.Simulate.click(comp.getDOMNode().querySelector(".btn-screen-share")); + + expect(comp.state.showMenu).eql(true); + }); + + it("should dispatch a StartScreenShare action on option click in screenshare dropdown", + function() { + var comp = TestUtils.renderIntoDocument( + React.createElement(sharedViews.ScreenShareControlButton, { + dispatcher: dispatcher, + visible: true, + state: SCREEN_SHARE_STATES.INACTIVE + })); + + TestUtils.Simulate.click(comp.getDOMNode().querySelector( + ".conversation-window-dropdown > li")); sinon.assert.calledOnce(dispatcher.dispatch); sinon.assert.calledWithExactly(dispatcher.dispatch, @@ -159,7 +178,7 @@ describe("loop.shared.views", function() { state: SCREEN_SHARE_STATES.ACTIVE })); - TestUtils.Simulate.click(comp.getDOMNode()); + TestUtils.Simulate.click(comp.getDOMNode().querySelector(".btn-screen-share")); sinon.assert.calledOnce(dispatcher.dispatch); sinon.assert.calledWithExactly(dispatcher.dispatch, diff --git a/browser/locales/en-US/chrome/browser/loop/loop.properties b/browser/locales/en-US/chrome/browser/loop/loop.properties index c8251797f93..20031cc51d0 100644 --- a/browser/locales/en-US/chrome/browser/loop/loop.properties +++ b/browser/locales/en-US/chrome/browser/loop/loop.properties @@ -185,6 +185,7 @@ mute_local_video_button_title=Mute your video unmute_local_video_button_title=Unmute your video active_screenshare_button_title=Stop sharing inactive_screenshare_button_title=Share your screen +share_windows_button_title=Share other Windows ## LOCALIZATION NOTE (call_with_contact_title): The title displayed ## when calling a contact. Don't translate the part between {{..}} because