From 26838111eef0d4880e34e94fe73d1c7c43d41b30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Sun, 25 Aug 2019 21:08:57 +0200 Subject: [PATCH 1/9] Move checkbox to change between public and group room to a menu MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- css/style.scss | 71 ++++++++++++---------- js/views/callinfoview.js | 8 ++- js/views/templates.js | 44 ++++++++------ js/views/templates/callinfoview.handlebars | 23 +++++-- 4 files changed, 91 insertions(+), 55 deletions(-) diff --git a/css/style.scss b/css/style.scss index 8b9779fe68a..493f12761cd 100644 --- a/css/style.scss +++ b/css/style.scss @@ -937,6 +937,7 @@ body:not(#body-public) .participantWithList > li > span:not(.currentUser):not(.g .call-controls-container { display: flex; + align-items: center; .call-button, .share-link-options { @@ -972,40 +973,32 @@ body:not(#body-public) .participantWithList > li > span:not(.currentUser):not(.g } } + .room-moderation-button { + /* Needed for proper positioning of the menu. */ + position: relative; + } + + .share-link-options .button, + .room-moderation-button .button { + cursor: pointer; + width: 44px; + height: 44px; + display: block; + background-color: transparent; + border: none; + margin: 0; + opacity: .5; + + &:hover, + &:focus, + &:active { + opacity: 1; + } + } + .share-link-options { display: flex; align-items: center; - .link-checkbox-label { - white-space: nowrap; - padding: 12px; - - /* If ".icon-loading-small" is set hide the checkbox and show a - * loading icon instead */ - &.icon-loading-small:before { - background-image: none !important; - background-color: transparent !important; - border-color: transparent !important; - } - &.icon-loading-small:after { - top: 21px; - left: 23px; - } - } - .button { - cursor: pointer; - width: 44px; - height: 44px; - display: block; - background-color: transparent; - border: none; - margin: 0; - opacity: .5; - &:hover, - &:focus, - &:active { - opacity: 1; - } - } .password-button { position: relative; @@ -1034,6 +1027,22 @@ body:not(#body-public) .participantWithList > li > span:not(.currentUser):not(.g } } } + + /* The specific locator is needed to have higher priority than other + * important rules set in the server. */ + input.checkbox + label.link-checkbox-label { + /* If ".icon-loading-small" is set hide the checkbox and show a + * loading icon instead. */ + &.icon-loading-small:before { + background-image: none !important; + background-color: transparent !important; + border-color: transparent !important; + } + &.icon-loading-small:after { + top: 22px; + left: 21px; + } + } } /** diff --git a/js/views/callinfoview.js b/js/views/callinfoview.js index 43071ecc7aa..0da9ff4cada 100644 --- a/js/views/callinfoview.js +++ b/js/views/callinfoview.js @@ -45,12 +45,14 @@ templateContext: function() { var canModerate = this._canModerate(); + var canFullModerate = this._canFullModerate(); return $.extend(this.model.toJSON(), { isRoomForFile: this.model.get('objectType') === 'file', fileLink: OC.generateUrl('/f/{fileId}', { fileId: this.model.get('objectId') }), fileLinkTitle: t('spreed', 'Go to file'), + showRoomModerationMenu: canModerate && canFullModerate, canModerate: canModerate, - canFullModerate: this._canFullModerate(), + canFullModerate: canFullModerate, linkCheckboxLabel: t('spreed', 'Share link'), isPublic: this.model.get('type') === 3, passwordInputPlaceholder: this.model.get('hasPassword')? t('spreed', 'Change password'): t('spreed', 'Set password'), @@ -77,6 +79,9 @@ 'passwordConfirm': '.password-confirm', 'passwordLoading': '.password-loading', + 'roomModerationButton': '.room-moderation-button .button', + 'roomModerationMenu': '.room-moderation-button .menu', + 'menu': '.password-menu', }, @@ -205,6 +210,7 @@ $(self.ui.passwordInput).focus(); }); + OC.registerMenu(this.ui.roomModerationButton, this.ui.roomModerationMenu); }, _canModerate: function() { diff --git a/js/views/templates.js b/js/views/templates.js index 2ff1e34a03b..426f116ffb1 100644 --- a/js/views/templates.js +++ b/js/views/templates.js @@ -48,38 +48,43 @@ templates['callinfoview'] = template({"1":function(container,depth0,helpers,part + alias4(((helper = (helper = helpers.fileLinkTitle || (depth0 != null ? depth0.fileLinkTitle : depth0)) != null ? helper : alias2),(typeof helper === alias3 ? helper.call(alias1,{"name":"fileLinkTitle","hash":{},"data":data}) : helper))) + "\">\n \n \n"; },"3":function(container,depth0,helpers,partials,data) { - var stack1, alias1=depth0 != null ? depth0 : (container.nullContext || {}); + var stack1; return "
\n" - + ((stack1 = helpers["if"].call(alias1,(depth0 != null ? depth0.canFullModerate : depth0),{"name":"if","hash":{},"fn":container.program(4, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") - + ((stack1 = helpers["if"].call(alias1,(depth0 != null ? depth0.isPublic : depth0),{"name":"if","hash":{},"fn":container.program(7, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") + + ((stack1 = helpers["if"].call(depth0 != null ? depth0 : (container.nullContext || {}),(depth0 != null ? depth0.isPublic : depth0),{"name":"if","hash":{},"fn":container.program(4, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") + "
\n"; },"4":function(container,depth0,helpers,partials,data) { var stack1, helper, alias1=depth0 != null ? depth0 : (container.nullContext || {}); - return " \n \n"; -},"5":function(container,depth0,helpers,partials,data) { - return " checked=\"checked\""; -},"7":function(container,depth0,helpers,partials,data) { - var stack1, helper, alias1=depth0 != null ? depth0 : (container.nullContext || {}); - return "
\n
\n
\n
\n
    \n
  • \n \n
    \n \n \n \n \n
    \n
    • \n
\n
\n
\n"; -},"8":function(container,depth0,helpers,partials,data) { +},"5":function(container,depth0,helpers,partials,data) { return "icon-password"; -},"10":function(container,depth0,helpers,partials,data) { +},"7":function(container,depth0,helpers,partials,data) { return "icon-no-password"; -},"12":function(container,depth0,helpers,partials,data) { +},"9":function(container,depth0,helpers,partials,data) { return "
\n
\n
\n"; +},"11":function(container,depth0,helpers,partials,data) { + var stack1; + + return "
\n
\n \n
\n
\n
    \n" + + ((stack1 = helpers["if"].call(depth0 != null ? depth0 : (container.nullContext || {}),(depth0 != null ? depth0.canFullModerate : depth0),{"name":"if","hash":{},"fn":container.program(12, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") + + "
\n
\n
\n"; +},"12":function(container,depth0,helpers,partials,data) { + var stack1, helper, alias1=depth0 != null ? depth0 : (container.nullContext || {}); + + return "
  • \n \n \n \n \n
    • \n"; +},"13":function(container,depth0,helpers,partials,data) { + return " checked=\"checked\""; },"compiler":[7,">= 4.0.0"],"main":function(container,depth0,helpers,partials,data) { var stack1, alias1=depth0 != null ? depth0 : (container.nullContext || {}); @@ -87,7 +92,8 @@ templates['callinfoview'] = template({"1":function(container,depth0,helpers,part + ((stack1 = helpers["if"].call(alias1,(depth0 != null ? depth0.isRoomForFile : depth0),{"name":"if","hash":{},"fn":container.program(1, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") + "\n
    \n
    \n" + ((stack1 = helpers["if"].call(alias1,(depth0 != null ? depth0.canModerate : depth0),{"name":"if","hash":{},"fn":container.program(3, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") - + ((stack1 = helpers["if"].call(alias1,(depth0 != null ? depth0.showShareLink : depth0),{"name":"if","hash":{},"fn":container.program(12, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") + + ((stack1 = helpers["if"].call(alias1,(depth0 != null ? depth0.showShareLink : depth0),{"name":"if","hash":{},"fn":container.program(9, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") + + ((stack1 = helpers["if"].call(alias1,(depth0 != null ? depth0.showRoomModerationMenu : depth0),{"name":"if","hash":{},"fn":container.program(11, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") + "
    \n"; },"useData":true}); templates['chatview'] = template({"compiler":[7,">= 4.0.0"],"main":function(container,depth0,helpers,partials,data) { diff --git a/js/views/templates/callinfoview.handlebars b/js/views/templates/callinfoview.handlebars index a4648ce6424..6d9551207ab 100644 --- a/js/views/templates/callinfoview.handlebars +++ b/js/views/templates/callinfoview.handlebars @@ -10,10 +10,6 @@
    {{#if canModerate}} From 452a46f968ee9037b4575fc1bf386aaefca625db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Sun, 25 Aug 2019 21:46:16 +0200 Subject: [PATCH 2/9] Remove unused UI locators MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- js/views/callinfoview.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/js/views/callinfoview.js b/js/views/callinfoview.js index 0da9ff4cada..02892f556ee 100644 --- a/js/views/callinfoview.js +++ b/js/views/callinfoview.js @@ -64,7 +64,6 @@ ui: { 'roomName': 'div.room-name', 'fileLink': '.file-link', - 'shareLinkOptions': '.share-link-options', 'clipboardButton': '.clipboard-button', 'linkCheckbox': '.link-checkbox', 'linkCheckboxLabel': '.link-checkbox-label', @@ -73,16 +72,12 @@ 'passwordButton': '.password-button .button', 'passwordForm': '.password-form', - 'passwordMenu': '.password-menu', - 'passwordOption': '.password-option', 'passwordInput': '.password-input', 'passwordConfirm': '.password-confirm', 'passwordLoading': '.password-loading', 'roomModerationButton': '.room-moderation-button .button', 'roomModerationMenu': '.room-moderation-button .menu', - - 'menu': '.password-menu', }, regions: { From 039a0e8968ecab43ec1a7020fbaa81e8826b20f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Sun, 25 Aug 2019 22:58:24 +0200 Subject: [PATCH 3/9] Do not defer rendering CallInfoView again until the menu is inactive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In Nextcloud 17 "OC._currentMenu" was removed, so "renderWhenInactive()" always renders immediately. Instead of fixing that the method was completly removed, as keeping the menu open instead of rendering again the view does not provide much value (if the room is no longer public the password will be unusable anyway, and having the password menu open when another moderator sets or removes the password should not happen too often). Signed-off-by: Daniel Calviño Sánchez --- js/views/callinfoview.js | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/js/views/callinfoview.js b/js/views/callinfoview.js index 02892f556ee..0e1cd3fcf15 100644 --- a/js/views/callinfoview.js +++ b/js/views/callinfoview.js @@ -95,7 +95,7 @@ modelEvents: { 'change:hasPassword': function() { - this.renderWhenInactive(); + this.render(); }, 'change:participantType': function() { this._updateNameEditability(); @@ -107,7 +107,7 @@ 'change:type': function() { this._updateNameEditability(); - this.renderWhenInactive(); + this.render(); } }, @@ -144,15 +144,6 @@ this._updateNameEditability(); }, - renderWhenInactive: function() { - if (!OC._currentMenu || !OC._currentMenu.hasClass('password-menu') || this.ui.passwordInput.length === 0 || this.ui.passwordInput.val() === '') { - this.render(); - return; - } - - this.renderTimeout = setTimeout(_.bind(this.renderWhenInactive, this), 500); - }, - onBeforeRender: function() { // During the rendering the regions of this view are reset, which // destroys its child views. If a child view has to be detached From 7a2e19553f57e9d7792adf0e4d3913a00ffca30e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 26 Aug 2019 00:08:48 +0200 Subject: [PATCH 4/9] Move password from its own menu to the room moderation menu MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The password input is no longer the only element of the menu so pressing "Esc" does not close the menu. There is no longer a password button in the CallInfoView, so now the acceptance tests need to open the room moderation menu to know if the conversation is password protected or not. Signed-off-by: Daniel Calviño Sánchez --- css/style.scss | 75 +++++++++++++------ js/views/callinfoview.js | 31 ++------ js/views/templates.js | 45 +++++------ js/views/templates/callinfoview.handlebars | 29 +++---- .../bootstrap/ConversationInfoContext.php | 53 ++++++++++--- 5 files changed, 135 insertions(+), 98 deletions(-) diff --git a/css/style.scss b/css/style.scss index 493f12761cd..7533e8a98ea 100644 --- a/css/style.scss +++ b/css/style.scss @@ -999,30 +999,59 @@ body:not(#body-public) .participantWithList > li > span:not(.currentUser):not(.g .share-link-options { display: flex; align-items: center; - .password-button { - position: relative; + } + + .room-moderation-button .menu { + /* Values copied from apps.scss in server. */ + $popoveritem-height: 44px; + $popovericon-size: 16px; + $outter-margin: ($popoveritem-height - $popovericon-size) / 2; + + /* The server sets a top/bottom margin to forms and inputs of the + * first/last item in a menu, but that misaligns the left icon; this + * sets the top/bottom margin to the whole item instead of only to the + * forms or inputs. */ + li:first-of-type { + > .menuitem.password-option { + + margin-top: $outter-margin - 2px; // minus the input margin + + > form, > input { + margin-top: 0; + } + } + } + li:last-of-type { + > .menuitem.password-option { + + margin-bottom: $outter-margin - 2px; // minus the input margin + + > form, > input { + margin-bottom: 0; + } + } + } + + .menuitem.password-option { + /* Override rule for menu items from server, as in this case + * only the button in the password field is clickable, so the + * pointer cursor should not be used for the whole item. */ + cursor: default; + + .password-form { + position: relative; + + .password-confirm, + .password-loading { + /* Inputs in menu items do not have a right margin, so + * it does not need to be compensated. */ + right: 0; + } - .menuitem { - /* Override rule for menu items from server, as in this case - * only the button in the password field is clickable, so the - * pointer cursor should not be used for the whole item. */ - cursor: default; - - .password-form { - position: relative; - - .password-confirm, - .password-loading { - /* Inputs in menu items do not have a right margin, so - * it does not need to be compensated. */ - right: 0; - } - - .password-confirm { - /* Needed to override an important rule set in the - * server. */ - background-color: transparent !important; - } + .password-confirm { + /* Needed to override an important rule set in the + * server. */ + background-color: transparent !important; } } } diff --git a/js/views/callinfoview.js b/js/views/callinfoview.js index 0e1cd3fcf15..6fd06b728c0 100644 --- a/js/views/callinfoview.js +++ b/js/views/callinfoview.js @@ -46,17 +46,18 @@ templateContext: function() { var canModerate = this._canModerate(); var canFullModerate = this._canFullModerate(); + var isPublic = this.model.get('type') === OCA.SpreedMe.app.ROOM_TYPE_PUBLIC; return $.extend(this.model.toJSON(), { isRoomForFile: this.model.get('objectType') === 'file', fileLink: OC.generateUrl('/f/{fileId}', { fileId: this.model.get('objectId') }), fileLinkTitle: t('spreed', 'Go to file'), - showRoomModerationMenu: canModerate && canFullModerate, + showRoomModerationMenu: canModerate && (canFullModerate || isPublic), canModerate: canModerate, canFullModerate: canFullModerate, linkCheckboxLabel: t('spreed', 'Share link'), - isPublic: this.model.get('type') === 3, + isPublic: isPublic, passwordInputPlaceholder: this.model.get('hasPassword')? t('spreed', 'Change password'): t('spreed', 'Set password'), - showShareLink: !canModerate && this.model.get('type') === 3, + showShareLink: !canModerate && isPublic, isDeletable: canModerate && (Object.keys(this.model.get('participants')).length > 2 || this.model.get('numGuests') > 0) }); }, @@ -70,7 +71,6 @@ 'callButton': 'div.call-button', - 'passwordButton': '.password-button .button', 'passwordForm': '.password-form', 'passwordInput': '.password-input', 'passwordConfirm': '.password-confirm', @@ -88,7 +88,6 @@ events: { 'change @ui.linkCheckbox': 'toggleLinkCheckbox', - 'keyup @ui.passwordInput': 'keyUpPassword', 'click @ui.passwordConfirm': 'confirmPassword', 'submit @ui.passwordForm': 'confirmPassword', }, @@ -181,21 +180,10 @@ }); this.initClipboard(); - this.ui.passwordButton.tooltip({ - placement: 'bottom', - trigger: 'hover', - title: (this.model.get('hasPassword')) ? t('spreed', 'Change password') : t('spreed', 'Set password') - }); - // Set the body as the container to show the tooltip in front of the // header. this.ui.fileLink.tooltip({container: $('body')}); - var self = this; - OC.registerMenu($(this.ui.passwordButton), $(this.ui.passwordMenu), function() { - $(self.ui.passwordInput).focus(); - }); - OC.registerMenu(this.ui.roomModerationButton, this.ui.roomModerationMenu); }, @@ -287,7 +275,7 @@ this.ui.passwordInput.val(''); restoreState(); OC.hideMenus(); - this.ui.passwordButton.focus(); + this.ui.roomModerationButton.focus(); }.bind(this), error: function() { restoreState(); @@ -297,15 +285,6 @@ }); }, - keyUpPassword: function(e) { - e.preventDefault(); - if (e.keyCode === 27) { - // ESC - OC.hideMenus(); - this.ui.passwordButton.focus(); - } - }, - /** * Clipboard */ diff --git a/js/views/templates.js b/js/views/templates.js index 426f116ffb1..17e87bee21e 100644 --- a/js/views/templates.js +++ b/js/views/templates.js @@ -54,37 +54,38 @@ templates['callinfoview'] = template({"1":function(container,depth0,helpers,part + ((stack1 = helpers["if"].call(depth0 != null ? depth0 : (container.nullContext || {}),(depth0 != null ? depth0.isPublic : depth0),{"name":"if","hash":{},"fn":container.program(4, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") + " \n"; },"4":function(container,depth0,helpers,partials,data) { - var stack1, helper, alias1=depth0 != null ? depth0 : (container.nullContext || {}); - - return "
    \n
    \n
    \n
    \n
      \n
    • \n \n
      \n \n \n \n \n
      \n
      • \n
    \n
    \n
    \n"; -},"5":function(container,depth0,helpers,partials,data) { - return "icon-password"; -},"7":function(container,depth0,helpers,partials,data) { - return "icon-no-password"; -},"9":function(container,depth0,helpers,partials,data) { + return "
    \n"; +},"6":function(container,depth0,helpers,partials,data) { return "
    \n
    \n
    \n"; -},"11":function(container,depth0,helpers,partials,data) { - var stack1; +},"8":function(container,depth0,helpers,partials,data) { + var stack1, alias1=depth0 != null ? depth0 : (container.nullContext || {}); return "
    \n
    \n \n
    \n
    \n
      \n" - + ((stack1 = helpers["if"].call(depth0 != null ? depth0 : (container.nullContext || {}),(depth0 != null ? depth0.canFullModerate : depth0),{"name":"if","hash":{},"fn":container.program(12, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") + + ((stack1 = helpers["if"].call(alias1,(depth0 != null ? depth0.canFullModerate : depth0),{"name":"if","hash":{},"fn":container.program(9, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") + + ((stack1 = helpers["if"].call(alias1,(depth0 != null ? depth0.isPublic : depth0),{"name":"if","hash":{},"fn":container.program(12, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") + "
    \n
    \n
    \n"; -},"12":function(container,depth0,helpers,partials,data) { +},"9":function(container,depth0,helpers,partials,data) { var stack1, helper, alias1=depth0 != null ? depth0 : (container.nullContext || {}); return "
  • \n \n \n \n \n
    • \n"; -},"13":function(container,depth0,helpers,partials,data) { +},"10":function(container,depth0,helpers,partials,data) { return " checked=\"checked\""; +},"12":function(container,depth0,helpers,partials,data) { + var stack1, helper, alias1=depth0 != null ? depth0 : (container.nullContext || {}); + + return "
  • \n \n
    \n \n \n \n \n
    \n
    • \n"; +},"13":function(container,depth0,helpers,partials,data) { + return "icon-password"; +},"15":function(container,depth0,helpers,partials,data) { + return "icon-no-password"; },"compiler":[7,">= 4.0.0"],"main":function(container,depth0,helpers,partials,data) { var stack1, alias1=depth0 != null ? depth0 : (container.nullContext || {}); @@ -92,8 +93,8 @@ templates['callinfoview'] = template({"1":function(container,depth0,helpers,part + ((stack1 = helpers["if"].call(alias1,(depth0 != null ? depth0.isRoomForFile : depth0),{"name":"if","hash":{},"fn":container.program(1, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") + "\n
    \n
    \n" + ((stack1 = helpers["if"].call(alias1,(depth0 != null ? depth0.canModerate : depth0),{"name":"if","hash":{},"fn":container.program(3, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") - + ((stack1 = helpers["if"].call(alias1,(depth0 != null ? depth0.showShareLink : depth0),{"name":"if","hash":{},"fn":container.program(9, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") - + ((stack1 = helpers["if"].call(alias1,(depth0 != null ? depth0.showRoomModerationMenu : depth0),{"name":"if","hash":{},"fn":container.program(11, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") + + ((stack1 = helpers["if"].call(alias1,(depth0 != null ? depth0.showShareLink : depth0),{"name":"if","hash":{},"fn":container.program(6, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") + + ((stack1 = helpers["if"].call(alias1,(depth0 != null ? depth0.showRoomModerationMenu : depth0),{"name":"if","hash":{},"fn":container.program(8, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") + "
    \n"; },"useData":true}); templates['chatview'] = template({"compiler":[7,">= 4.0.0"],"main":function(container,depth0,helpers,partials,data) { diff --git a/js/views/templates/callinfoview.handlebars b/js/views/templates/callinfoview.handlebars index 6d9551207ab..968bfe829f9 100644 --- a/js/views/templates/callinfoview.handlebars +++ b/js/views/templates/callinfoview.handlebars @@ -12,23 +12,6 @@ {{/if}} @@ -52,6 +35,18 @@ {{/if}} + {{#if isPublic}} +
  • + +
    + + +
    +
    • + {{/if}} diff --git a/tests/acceptance/features/bootstrap/ConversationInfoContext.php b/tests/acceptance/features/bootstrap/ConversationInfoContext.php index 29a7e4d346f..3186bc7de4e 100644 --- a/tests/acceptance/features/bootstrap/ConversationInfoContext.php +++ b/tests/acceptance/features/bootstrap/ConversationInfoContext.php @@ -84,10 +84,19 @@ public static function copyLinkButton() { /** * @return Locator */ - public static function passwordButton() { - return Locator::forThe()->css(".password-button")-> + public static function roomModerationButton() { + return Locator::forThe()->css(".room-moderation-button")-> descendantOf(self::conversationInfoContainer())-> - describedAs("Password button in conversation info"); + describedAs("Room moderation button in conversation info"); + } + + /** + * @return Locator + */ + public static function roomModerationMenu() { + return Locator::forThe()->css(".menu")-> + descendantOf(self::roomModerationButton())-> + describedAs("Room moderation menu in conversation info"); } /** @@ -95,8 +104,8 @@ public static function passwordButton() { */ public static function passwordIcon() { return Locator::forThe()->css(".icon-password")-> - descendantOf(self::passwordButton())-> - describedAs("Password icon in conversation info"); + descendantOf(self::roomModerationMenu())-> + describedAs("Password icon in room moderation menu in conversation info"); } /** @@ -104,8 +113,8 @@ public static function passwordIcon() { */ public static function noPasswordIcon() { return Locator::forThe()->css(".icon-no-password")-> - descendantOf(self::passwordButton())-> - describedAs("No password icon in conversation info"); + descendantOf(self::roomModerationMenu())-> + describedAs("No password icon in room moderation menu in conversation info"); } /** @@ -113,8 +122,8 @@ public static function noPasswordIcon() { */ public static function passwordField() { return Locator::forThe()->css(".password-input")-> - descendantOf(self::conversationInfoContainer())-> - describedAs("Password field in conversation info"); + descendantOf(self::roomModerationMenu())-> + describedAs("Password field in room moderation menu in conversation info"); } /** @@ -142,7 +151,7 @@ public function iWriteDownThePublicConversationLink() { * @When I protect the conversation with the password :password */ public function iProtectTheConversationWithThePassword($password) { - $this->actor->find(self::passwordButton(), 2)->click(); + $this->showRoomModerationMenu(); $this->actor->find(self::passwordField(), 2)->setValue($password . "\r"); } @@ -151,14 +160,38 @@ public function iProtectTheConversationWithThePassword($password) { * @Then I see that the conversation is password protected */ public function iSeeThatTheConversationIsPasswordProtected() { + $this->showRoomModerationMenu(); + PHPUnit_Framework_Assert::assertTrue($this->actor->find(self::passwordIcon(), 10)->isVisible(), "Password icon is visible"); + + // Hide menu again after checking the icon. + $this->actor->find(self::roomModerationButton(), 2)->click(); } /** * @Then I see that the conversation is not password protected */ public function iSeeThatTheConversationIsNotPasswordProtected() { + $this->showRoomModerationMenu(); + PHPUnit_Framework_Assert::assertTrue($this->actor->find(self::noPasswordIcon(), 10)->isVisible(), "No password icon is visible"); + + // Hide menu again after checking the icon. + $this->actor->find(self::roomModerationButton(), 2)->click(); + } + + private function showRoomModerationMenu() { + // The room moderation menu is hidden after clicking on an action of the + // menu. Therefore, if the menu is visible, wait a little just in case + // it is in the process of being hidden due to a previous action. + if (!WaitFor::elementToBeEventuallyNotShown( + $this->actor, + self::roomModerationMenu(), + $timeout = 5 * $this->actor->getFindTimeoutMultiplier())) { + PHPUnit_Framework_Assert::fail("The room moderation menu is still shown after $timeout seconds"); + } + + $this->actor->find(self::roomModerationButton(), 10)->click(); } } From 7dc9653a5994e72dffabcf791bf21056286ed32b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 26 Aug 2019 01:19:45 +0200 Subject: [PATCH 5/9] Reorder flex CSS rules for call button MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- css/style.scss | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/css/style.scss b/css/style.scss index 7533e8a98ea..054db3a18d2 100644 --- a/css/style.scss +++ b/css/style.scss @@ -939,14 +939,13 @@ body:not(#body-public) .participantWithList > li > span:not(.currentUser):not(.g display: flex; align-items: center; - .call-button, .share-link-options { flex-grow: 1; - flex-basis: 50%; } .call-button { flex-grow: 0; + flex-basis: 50%; .join-call, .leave-call { From 4425b04e7ce380eb6403e8f612722d509c63ad06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 26 Aug 2019 01:36:08 +0200 Subject: [PATCH 6/9] Unify "Copy link" button for all participant types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The same "Copy link" button was defined separately in the templates for moderators and non moderator participants. Signed-off-by: Daniel Calviño Sánchez --- js/views/callinfoview.js | 2 -- js/views/templates.js | 33 ++++++++-------------- js/views/templates/callinfoview.handlebars | 9 +----- 3 files changed, 13 insertions(+), 31 deletions(-) diff --git a/js/views/callinfoview.js b/js/views/callinfoview.js index 6fd06b728c0..096e1d76a69 100644 --- a/js/views/callinfoview.js +++ b/js/views/callinfoview.js @@ -52,12 +52,10 @@ fileLink: OC.generateUrl('/f/{fileId}', { fileId: this.model.get('objectId') }), fileLinkTitle: t('spreed', 'Go to file'), showRoomModerationMenu: canModerate && (canFullModerate || isPublic), - canModerate: canModerate, canFullModerate: canFullModerate, linkCheckboxLabel: t('spreed', 'Share link'), isPublic: isPublic, passwordInputPlaceholder: this.model.get('hasPassword')? t('spreed', 'Change password'): t('spreed', 'Set password'), - showShareLink: !canModerate && isPublic, isDeletable: canModerate && (Object.keys(this.model.get('participants')).length > 2 || this.model.get('numGuests') > 0) }); }, diff --git a/js/views/templates.js b/js/views/templates.js index 17e87bee21e..4071390306d 100644 --- a/js/views/templates.js +++ b/js/views/templates.js @@ -48,43 +48,35 @@ templates['callinfoview'] = template({"1":function(container,depth0,helpers,part + alias4(((helper = (helper = helpers.fileLinkTitle || (depth0 != null ? depth0.fileLinkTitle : depth0)) != null ? helper : alias2),(typeof helper === alias3 ? helper.call(alias1,{"name":"fileLinkTitle","hash":{},"data":data}) : helper))) + "\">\n \n \n"; },"3":function(container,depth0,helpers,partials,data) { - var stack1; - - return "
    \n" - + ((stack1 = helpers["if"].call(depth0 != null ? depth0 : (container.nullContext || {}),(depth0 != null ? depth0.isPublic : depth0),{"name":"if","hash":{},"fn":container.program(4, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") - + "
    \n"; -},"4":function(container,depth0,helpers,partials,data) { - return "
    \n"; -},"6":function(container,depth0,helpers,partials,data) { return "
    \n
    \n
    \n"; -},"8":function(container,depth0,helpers,partials,data) { +},"5":function(container,depth0,helpers,partials,data) { var stack1, alias1=depth0 != null ? depth0 : (container.nullContext || {}); return "
    \n
    \n \n
    \n
    \n
      \n" - + ((stack1 = helpers["if"].call(alias1,(depth0 != null ? depth0.canFullModerate : depth0),{"name":"if","hash":{},"fn":container.program(9, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") - + ((stack1 = helpers["if"].call(alias1,(depth0 != null ? depth0.isPublic : depth0),{"name":"if","hash":{},"fn":container.program(12, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") + + ((stack1 = helpers["if"].call(alias1,(depth0 != null ? depth0.canFullModerate : depth0),{"name":"if","hash":{},"fn":container.program(6, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") + + ((stack1 = helpers["if"].call(alias1,(depth0 != null ? depth0.isPublic : depth0),{"name":"if","hash":{},"fn":container.program(9, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") + "
    \n
    \n
    \n"; -},"9":function(container,depth0,helpers,partials,data) { +},"6":function(container,depth0,helpers,partials,data) { var stack1, helper, alias1=depth0 != null ? depth0 : (container.nullContext || {}); return "
  • \n \n \n \n \n
    • \n"; -},"10":function(container,depth0,helpers,partials,data) { +},"7":function(container,depth0,helpers,partials,data) { return " checked=\"checked\""; -},"12":function(container,depth0,helpers,partials,data) { +},"9":function(container,depth0,helpers,partials,data) { var stack1, helper, alias1=depth0 != null ? depth0 : (container.nullContext || {}); return "
  • \n \n
    \n \n \n \n \n
    \n
    • \n"; -},"13":function(container,depth0,helpers,partials,data) { +},"10":function(container,depth0,helpers,partials,data) { return "icon-password"; -},"15":function(container,depth0,helpers,partials,data) { +},"12":function(container,depth0,helpers,partials,data) { return "icon-no-password"; },"compiler":[7,">= 4.0.0"],"main":function(container,depth0,helpers,partials,data) { var stack1, alias1=depth0 != null ? depth0 : (container.nullContext || {}); @@ -92,9 +84,8 @@ templates['callinfoview'] = template({"1":function(container,depth0,helpers,part return "
    \n
    \n" + ((stack1 = helpers["if"].call(alias1,(depth0 != null ? depth0.isRoomForFile : depth0),{"name":"if","hash":{},"fn":container.program(1, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") + "
    \n
    \n
    \n" - + ((stack1 = helpers["if"].call(alias1,(depth0 != null ? depth0.canModerate : depth0),{"name":"if","hash":{},"fn":container.program(3, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") - + ((stack1 = helpers["if"].call(alias1,(depth0 != null ? depth0.showShareLink : depth0),{"name":"if","hash":{},"fn":container.program(6, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") - + ((stack1 = helpers["if"].call(alias1,(depth0 != null ? depth0.showRoomModerationMenu : depth0),{"name":"if","hash":{},"fn":container.program(8, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") + + ((stack1 = helpers["if"].call(alias1,(depth0 != null ? depth0.isPublic : depth0),{"name":"if","hash":{},"fn":container.program(3, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") + + ((stack1 = helpers["if"].call(alias1,(depth0 != null ? depth0.showRoomModerationMenu : depth0),{"name":"if","hash":{},"fn":container.program(5, data, 0),"inverse":container.noop,"data":data})) != null ? stack1 : "") + "
    \n"; },"useData":true}); templates['chatview'] = template({"compiler":[7,">= 4.0.0"],"main":function(container,depth0,helpers,partials,data) { diff --git a/js/views/templates/callinfoview.handlebars b/js/views/templates/callinfoview.handlebars index 968bfe829f9..a2a5e5e4ed3 100644 --- a/js/views/templates/callinfoview.handlebars +++ b/js/views/templates/callinfoview.handlebars @@ -8,14 +8,7 @@
    - {{#if canModerate}} - - {{/if}} - {{#if showShareLink}} + {{#if isPublic}} From 7dec802cfac990d429b445ab193aa92c7cd6c260 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 26 Aug 2019 01:40:19 +0200 Subject: [PATCH 7/9] Remove no longer needed parent element of "Copy link" button MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "share-link-options" element acts as a spacer to move the room moderation menu button to the right. However, given that the call button width is limited to the 50% of its parent flex element and that element has the full width of the sidebar the same effect can be achieved by setting "margin-left: auto" on the room moderation button. Signed-off-by: Daniel Calviño Sánchez --- css/style.scss | 14 ++++---------- js/views/templates.js | 2 +- js/views/templates/callinfoview.handlebars | 4 +--- 3 files changed, 6 insertions(+), 14 deletions(-) diff --git a/css/style.scss b/css/style.scss index 054db3a18d2..d747b55375b 100644 --- a/css/style.scss +++ b/css/style.scss @@ -939,10 +939,6 @@ body:not(#body-public) .participantWithList > li > span:not(.currentUser):not(.g display: flex; align-items: center; - .share-link-options { - flex-grow: 1; - } - .call-button { flex-grow: 0; flex-basis: 50%; @@ -975,9 +971,12 @@ body:not(#body-public) .participantWithList > li > span:not(.currentUser):not(.g .room-moderation-button { /* Needed for proper positioning of the menu. */ position: relative; + + /* Position the button to the right end of its flex parent. */ + margin-left: auto; } - .share-link-options .button, + .clipboard-button .button, .room-moderation-button .button { cursor: pointer; width: 44px; @@ -995,11 +994,6 @@ body:not(#body-public) .participantWithList > li > span:not(.currentUser):not(.g } } - .share-link-options { - display: flex; - align-items: center; - } - .room-moderation-button .menu { /* Values copied from apps.scss in server. */ $popoveritem-height: 44px; diff --git a/js/views/templates.js b/js/views/templates.js index 4071390306d..1e0817c1dd4 100644 --- a/js/views/templates.js +++ b/js/views/templates.js @@ -48,7 +48,7 @@ templates['callinfoview'] = template({"1":function(container,depth0,helpers,part + alias4(((helper = (helper = helpers.fileLinkTitle || (depth0 != null ? depth0.fileLinkTitle : depth0)) != null ? helper : alias2),(typeof helper === alias3 ? helper.call(alias1,{"name":"fileLinkTitle","hash":{},"data":data}) : helper))) + "\">\n \n \n"; },"3":function(container,depth0,helpers,partials,data) { - return "
    \n
    \n
    \n"; + return "
    \n"; },"5":function(container,depth0,helpers,partials,data) { var stack1, alias1=depth0 != null ? depth0 : (container.nullContext || {}); diff --git a/js/views/templates/callinfoview.handlebars b/js/views/templates/callinfoview.handlebars index a2a5e5e4ed3..807ac107b60 100644 --- a/js/views/templates/callinfoview.handlebars +++ b/js/views/templates/callinfoview.handlebars @@ -9,9 +9,7 @@
    {{#if isPublic}} - +
    {{/if}} {{#if showRoomModerationMenu}}
    From b88d53d918cee60d0f1aac29dd2079f53a7e97c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 26 Aug 2019 02:38:34 +0200 Subject: [PATCH 8/9] Remove unneeded CSS rules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rules just set the same values used by buttons by default. Signed-off-by: Daniel Calviño Sánchez --- css/style.scss | 2 -- 1 file changed, 2 deletions(-) diff --git a/css/style.scss b/css/style.scss index d747b55375b..632f1703659 100644 --- a/css/style.scss +++ b/css/style.scss @@ -978,10 +978,8 @@ body:not(#body-public) .participantWithList > li > span:not(.currentUser):not(.g .clipboard-button .button, .room-moderation-button .button { - cursor: pointer; width: 44px; height: 44px; - display: block; background-color: transparent; border: none; margin: 0; From 3e3506b11ba3d502114b1bac88ddcb2f0bd938c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 26 Aug 2019 02:44:30 +0200 Subject: [PATCH 9/9] Replace "Copy link" tooltip with explicit label MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that the management actions were moved to a menu there is enough room to show a label for the "Copy link" button. Signed-off-by: Daniel Calviño Sánchez --- css/style.scss | 32 ++++++++++++++++++++-- js/views/callinfoview.js | 13 ++++----- js/views/templates.js | 6 +++- js/views/templates/callinfoview.handlebars | 2 +- 4 files changed, 40 insertions(+), 13 deletions(-) diff --git a/css/style.scss b/css/style.scss index 632f1703659..42b69ed5039 100644 --- a/css/style.scss +++ b/css/style.scss @@ -976,10 +976,30 @@ body:not(#body-public) .participantWithList > li > span:not(.currentUser):not(.g margin-left: auto; } - .clipboard-button .button, + .clipboard-button button { + display: flex; + align-items: center; + + > span { + cursor: pointer; + + /* Override reduced opacity set for button icons in server, as the + * button is already dimmed. */ + opacity: 1; + } + + /* Override appearance set for button text in server, as this button has + * no background. */ + padding: 0; + + > span { + color: var(--color-main-text); + font-weight: normal; + } + } + + .clipboard-button button, .room-moderation-button .button { - width: 44px; - height: 44px; background-color: transparent; border: none; margin: 0; @@ -992,6 +1012,12 @@ body:not(#body-public) .participantWithList > li > span:not(.currentUser):not(.g } } + .clipboard-button .icon, + .room-moderation-button .button { + width: 44px; + height: 44px; + } + .room-moderation-button .menu { /* Values copied from apps.scss in server. */ $popoveritem-height: 44px; diff --git a/js/views/callinfoview.js b/js/views/callinfoview.js index 096e1d76a69..886390af6f7 100644 --- a/js/views/callinfoview.js +++ b/js/views/callinfoview.js @@ -54,6 +54,7 @@ showRoomModerationMenu: canModerate && (canFullModerate || isPublic), canFullModerate: canFullModerate, linkCheckboxLabel: t('spreed', 'Share link'), + copyLinkLabel: t('spreed', 'Copy link'), isPublic: isPublic, passwordInputPlaceholder: this.model.get('hasPassword')? t('spreed', 'Change password'): t('spreed', 'Set password'), isDeletable: canModerate && (Object.keys(this.model.get('participants')).length > 2 || this.model.get('numGuests') > 0) @@ -173,8 +174,8 @@ this.ui.clipboardButton.attr('data-clipboard-text', completeURL); this.ui.clipboardButton.tooltip({ placement: 'bottom', - trigger: 'hover', - title: t('spreed', 'Copy link') + trigger: 'manual', + title: t('core', 'Link copied!') }); this.initClipboard(); @@ -305,9 +306,7 @@ .tooltip({placement: 'bottom', trigger: 'manual'}) .tooltip('show'); _.delay(function() { - $input.tooltip('hide') - .attr('data-original-title', t('core', 'Copy link')) - .tooltip('_fixTitle'); + $input.tooltip('hide'); }, 3000); }); this._clipboard.on('error', function (e) { @@ -327,9 +326,7 @@ .tooltip({placement: 'bottom', trigger: 'manual'}) .tooltip('show'); _.delay(function () { - $input.tooltip('hide') - .attr('data-original-title', t('spreed', 'Copy link')) - .tooltip('_fixTitle'); + $input.tooltip('hide'); }, 3000); }); } diff --git a/js/views/templates.js b/js/views/templates.js index 1e0817c1dd4..e1c4004f275 100644 --- a/js/views/templates.js +++ b/js/views/templates.js @@ -48,7 +48,11 @@ templates['callinfoview'] = template({"1":function(container,depth0,helpers,part + alias4(((helper = (helper = helpers.fileLinkTitle || (depth0 != null ? depth0.fileLinkTitle : depth0)) != null ? helper : alias2),(typeof helper === alias3 ? helper.call(alias1,{"name":"fileLinkTitle","hash":{},"data":data}) : helper))) + "\">\n \n \n"; },"3":function(container,depth0,helpers,partials,data) { - return "
    \n"; + var helper; + + return "
    \n"; },"5":function(container,depth0,helpers,partials,data) { var stack1, alias1=depth0 != null ? depth0 : (container.nullContext || {}); diff --git a/js/views/templates/callinfoview.handlebars b/js/views/templates/callinfoview.handlebars index 807ac107b60..4a3e52dbe74 100644 --- a/js/views/templates/callinfoview.handlebars +++ b/js/views/templates/callinfoview.handlebars @@ -9,7 +9,7 @@
    {{#if isPublic}} -
    +
    {{/if}} {{#if showRoomModerationMenu}}