From 38e2443ab75d5a069a2a9b8d4f991bcfda2785a0 Mon Sep 17 00:00:00 2001 From: Leonard Kim Date: Tue, 15 Aug 2017 15:14:49 -0700 Subject: [PATCH] feat(small-video): use AtlasKit tooltip --- css/_filmstrip.scss | 3 +- css/_vertical_filmstrip_overrides.scss | 27 +++- css/_videolayout_default.scss | 59 ++++++--- lang/main.json | 5 +- modules/UI/videolayout/SmallVideo.js | 34 +++-- .../components/ConnectionIndicator.js | 2 +- .../components/web/AudioMutedIndicator.js | 15 ++- .../filmstrip/components/web/BaseIndicator.js | 123 ++++++++---------- .../web/DominantSpeakerIndicator.js | 10 +- .../components/web/ModeratorIndicator.js | 23 +++- .../components/web/RaisedHandIndicator.js | 10 +- .../components/web/VideoMutedIndicator.js | 15 ++- 12 files changed, 208 insertions(+), 118 deletions(-) diff --git a/css/_filmstrip.scss b/css/_filmstrip.scss index 96f4abb17..e225c57b3 100644 --- a/css/_filmstrip.scss +++ b/css/_filmstrip.scss @@ -11,13 +11,13 @@ right: 0; padding: 10px 5px; @extend %align-right; + z-index: $filmstripVideosZ; &__toolbar { @include flex(); flex-direction: column-reverse; flex-wrap: nowrap; position: relative; - z-index: $zindex1; // Set z-index to make element visible. width: $filmstripToggleButtonWidth; button { @@ -53,7 +53,6 @@ /* The filmstrip should not be covered by the left toolbar. */ bottom: 0; width:auto; - z-index: $filmstripVideosZ; transition: bottom 2s; overflow: visible !important; /*!!! Removes the gap between the local video container and the remote diff --git a/css/_vertical_filmstrip_overrides.scss b/css/_vertical_filmstrip_overrides.scss index 2443251d1..a1c9cdcba 100644 --- a/css/_vertical_filmstrip_overrides.scss +++ b/css/_vertical_filmstrip_overrides.scss @@ -112,7 +112,7 @@ &__toolbar { text-align: right; - .toolbar-icon { + .right { float: none; margin: auto; } @@ -193,9 +193,28 @@ */ .connection-indicator, .remote-video-menu-trigger, - .videocontainer__toolbar, - .raisehandindicator, - #dominantspeakerindicator { + .indicator-icon-container { transform: translate3d(0, 0, 0); } + + .indicator-container { + float: none; + } + + /** + * FIXME: disable pointer to allow any elements moved below to still be + * clickable. The real fix would to make sure those moved elements are + * actually part of the toolbar instead of positioning being faked. + */ + .videocontainer__toolbar { + pointer-events: none; + + > div { + pointer-events: none; + } + + .toolbar-icon { + pointer-events: all; + } + } } diff --git a/css/_videolayout_default.scss b/css/_videolayout_default.scss index 6c8503e28..21a1fedbc 100644 --- a/css/_videolayout_default.scss +++ b/css/_videolayout_default.scss @@ -41,9 +41,30 @@ &__toptoolbar { position: absolute; left: 0; - z-index: $zindex3; + pointer-events: none; + z-index: $zindex10; width: 100%; box-sizing: border-box; // Includes the padding in the 100% width. + + /** + * FIXME (lenny): Disabling pointer-events is a pretty big sin that + * sidesteps the problems. There are z-index wars occurring within + * videocontainer and AtlasKit Tooltips rely on their parent z-indexe + * being higher than whatever they need to appear over. So set a higher + * z-index for the tooltip containers but make any empty space not block + * mouse overs for various mouseover triggers. + */ + pointer-events: none; + + * { + pointer-events: auto; + } + + .indicator-container { + display: inline-block; + float: left; + pointer-events: all; + } } &__toolbar { @@ -69,24 +90,21 @@ margin-top: $toolbarIconMargin; } - .indicator:nth-child(1) { + .indicator-container:nth-child(1) .indicator { margin-left: $toolbarIconMargin; } .connection-indicator, - span.indicator { - margin-right: em(5, 8); + div.indicator-container, + { + margin-right: 4px; } - span.indicator { - display: none; - - &:last-child { - margin-right: 0; - } + div.indicator:last-child { + margin-right: 0; } - .connection-indicator-container { + .indicator-container { display: inline-block; vertical-align: top; @@ -96,7 +114,7 @@ } .connection-indicator, - span.indicator { + .indicator { position: relative; font-size: 8px; text-align: center; @@ -307,7 +325,6 @@ padding: 0; border: 0; margin: 0px 5px 0px 0px; - float: left; } /** @@ -320,9 +337,17 @@ /** * Toolbar icons positioned on the right. */ -.toolbar-icon.right { - float: right; - margin: 0px 0px 0px 5px; +.moderator-icon { + display: inline-block; + + &.right { + float: right; + margin: 0px 0px 0px 5px; + } + + .toolbar-icon { + margin: 0; + } } .raisehandindicator { @@ -340,7 +365,7 @@ position: absolute; top: 0px; right: 0; - z-index: $zindex3; + z-index: $zindex2; width: 18px; height: 13px; color: #FFF; diff --git a/lang/main.json b/lang/main.json index 55abec919..02bd3957e 100644 --- a/lang/main.json +++ b/lang/main.json @@ -168,9 +168,8 @@ }, "videothumbnail": { - "editnickname": "Click to edit your
display name", - "moderator": "The owner of
this conference", - "videomute": "Participant has
stopped the camera", + "moderator": "Moderator", + "videomute": "Participant has stopped the camera", "mute": "Participant is muted", "kick": "Kick out", "muted": "Muted", diff --git a/modules/UI/videolayout/SmallVideo.js b/modules/UI/videolayout/SmallVideo.js index a6ca49db3..e646a57dd 100644 --- a/modules/UI/videolayout/SmallVideo.js +++ b/modules/UI/videolayout/SmallVideo.js @@ -316,16 +316,27 @@ SmallVideo.prototype.setVideoMutedView = function(isMuted) { SmallVideo.prototype.updateStatusBar = function () { const statusBarContainer = this.container.querySelector('.videocontainer__toolbar'); + const tooltipPosition = interfaceConfig.VERTICAL_FILMSTRIP ? 'left' : 'top'; /* jshint ignore:start */ ReactDOM.render( -
- { this.isAudioMuted ? : null } - { this.isVideoMuted ? : null } - { this._isModerator - && !interfaceConfig.DISABLE_FOCUS_INDICATOR - ? : null } -
, + +
+ { this.isAudioMuted + ? + : null } + { this.isVideoMuted + ? + : null } + { this._isModerator + && !interfaceConfig.DISABLE_FOCUS_INDICATOR + ? + : null } +
+ , statusBarContainer); /* jshint ignore:end */ }; @@ -744,6 +755,7 @@ SmallVideo.prototype.updateIndicators = function () { = this.container.querySelector('.videocontainer__toptoolbar'); const iconSize = UIUtil.getIndicatorFontSize(); + const tooltipPosition = interfaceConfig.VERTICAL_FILMSTRIP ? 'left' : 'top'; /* jshint ignore:start */ ReactDOM.render( @@ -758,10 +770,14 @@ SmallVideo.prototype.updateIndicators = function () { userID = { this.id } /> : null } { this._showRaisedHand - ? + ? : null } { this._showDominantSpeaker - ? + ? : null } , diff --git a/react/features/connection-indicator/components/ConnectionIndicator.js b/react/features/connection-indicator/components/ConnectionIndicator.js index bae11c0d6..5d9c217e5 100644 --- a/react/features/connection-indicator/components/ConnectionIndicator.js +++ b/react/features/connection-indicator/components/ConnectionIndicator.js @@ -175,7 +175,7 @@ class ConnectionIndicator extends Component { */ render() { return ( -
+
+ tooltipKey = 'videothumbnail.mute' + tooltipPosition = { this.props.tooltipPosition } /> ); } } diff --git a/react/features/filmstrip/components/web/BaseIndicator.js b/react/features/filmstrip/components/web/BaseIndicator.js index c61610320..f97cb33c0 100644 --- a/react/features/filmstrip/components/web/BaseIndicator.js +++ b/react/features/filmstrip/components/web/BaseIndicator.js @@ -1,6 +1,7 @@ import React, { Component } from 'react'; +import Tooltip from '@atlaskit/tooltip'; -import { setTooltip } from '../../../../../modules/UI/util/Tooltip'; +import { translate } from '../../../base/i18n'; /** * React {@code Component} for showing an icon with a tooltip. @@ -8,16 +9,27 @@ import { setTooltip } from '../../../../../modules/UI/util/Tooltip'; * @extends Component */ class BaseIndicator extends Component { + /** + * Default values for {@code BaseIndicator} component's properties. + * + * @static + */ static defaultProps = { className: '', iconClassName: '', iconSize: 'auto', - id: '' + id: '', + tooltipPosition: 'top' }; + /** + * {@code BaseIndicator} component's property types. + * + * @static + */ static propTypes = { /** - * The CSS class names to set on the root element of the component. + * Additional CSS class names to set on the icon container. */ className: React.PropTypes.string, @@ -36,44 +48,23 @@ class BaseIndicator extends Component { */ id: React.PropTypes.string, + /** + * Invoked to obtain translated strings. + */ + t: React.PropTypes.func, + /** * The translation key to use for displaying a tooltip when hovering * over the component. */ - tooltipKey: React.PropTypes.string - }; - - /** - * Initializes a new {@code BaseIndicator} instance. - * - * @param {Object} props - The read-only properties with which the new - * instance is to be initialized. - */ - constructor(props) { - super(props); + tooltipKey: React.PropTypes.string, /** - * An internal reference to the HTML element at the top of the - * component's DOM hierarchy. The reference is needed for attaching a - * tooltip. - * - * @type {HTMLElement} + * From which side of the indicator the tooltip should appear from. + * Defaults to "top". */ - this._rootElement = null; - - // Bind event handler so it is only bound once for every instance. - this._setRootElementRef = this._setRootElementRef.bind(this); - } - - /** - * Sets a tooltip which will display when hovering over the component. - * - * @inheritdoc - * @returns {void} - */ - componentDidMount() { - this._setTooltip(); - } + tooltipPosition: React.PropTypes.string + }; /** * Implements React's {@link Component#render()}. @@ -82,46 +73,34 @@ class BaseIndicator extends Component { * @returns {ReactElement} */ render() { + const { + className, + iconClassName, + iconSize, + id, + t, + tooltipKey, + tooltipPosition + } = this.props; + + const iconContainerClassName = `indicator-icon-container ${className}`; + return ( - - - - ); - } - - /** - * Sets the internal reference to the root HTML element for the component. - * - * @param {HTMLIconElement} element - The root HTML element of the - * component. - * @private - * @returns {void} - */ - _setRootElementRef(element) { - this._rootElement = element; - } - - /** - * Associate the component as a tooltip trigger so a tooltip may display on - * hover. - * - * @private - * @returns {void} - */ - _setTooltip() { - // TODO Replace UIUtil with an AtlasKit component when a suitable one - // becomes available for tooltips. - setTooltip( - this._rootElement, - this.props.tooltipKey, - 'top' +
+ + + + + +
); } } -export default BaseIndicator; +export default translate(BaseIndicator); diff --git a/react/features/filmstrip/components/web/DominantSpeakerIndicator.js b/react/features/filmstrip/components/web/DominantSpeakerIndicator.js index 758ec2645..23b4f9cf9 100644 --- a/react/features/filmstrip/components/web/DominantSpeakerIndicator.js +++ b/react/features/filmstrip/components/web/DominantSpeakerIndicator.js @@ -20,7 +20,12 @@ class DominantSpeakerIndicator extends Component { * * @type {number} */ - iconSize: React.PropTypes.number + iconSize: React.PropTypes.number, + + /** + * From which side of the indicator the tooltip should appear from. + */ + tooltipPosition: React.PropTypes.string }; /** @@ -35,7 +40,8 @@ class DominantSpeakerIndicator extends Component { iconClassName = 'indicatoricon fa fa-bullhorn' iconSize = { `${this.props.iconSize}px` } id = 'dominantspeakerindicator' - tooltipKey = 'speaker' /> + tooltipKey = 'speaker' + tooltipPosition = { this.props.tooltipPosition } /> ); } } diff --git a/react/features/filmstrip/components/web/ModeratorIndicator.js b/react/features/filmstrip/components/web/ModeratorIndicator.js index aa4521ef5..7e7a71a5b 100644 --- a/react/features/filmstrip/components/web/ModeratorIndicator.js +++ b/react/features/filmstrip/components/web/ModeratorIndicator.js @@ -8,6 +8,18 @@ import BaseIndicator from './BaseIndicator'; * @extends Component */ class ModeratorIndicator extends Component { + /** + * {@code ModeratorIndicator} component's property types. + * + * @static + */ + static propTypes = { + /** + * From which side of the indicator the tooltip should appear from. + */ + tooltipPosition: React.PropTypes.string + }; + /** * Implements React's {@link Component#render()}. * @@ -16,10 +28,13 @@ class ModeratorIndicator extends Component { */ render() { return ( - +
+ +
); } } diff --git a/react/features/filmstrip/components/web/RaisedHandIndicator.js b/react/features/filmstrip/components/web/RaisedHandIndicator.js index 5ef4ac7f7..33af282b2 100644 --- a/react/features/filmstrip/components/web/RaisedHandIndicator.js +++ b/react/features/filmstrip/components/web/RaisedHandIndicator.js @@ -19,7 +19,12 @@ class RaisedHandIndicator extends Component { * * @type {number} */ - iconSize: React.PropTypes.number + iconSize: React.PropTypes.number, + + /** + * From which side of the indicator the tooltip should appear from. + */ + tooltipPosition: React.PropTypes.string }; /** @@ -33,7 +38,8 @@ class RaisedHandIndicator extends Component { className = 'raisehandindicator indicator show-inline' iconClassName = 'icon-raised-hand indicatoricon' iconSize = { `${this.props.iconSize}px` } - tooltipKey = 'raisedHand' /> + tooltipKey = 'raisedHand' + tooltipPosition = { this.props.tooltipPosition } /> ); } } diff --git a/react/features/filmstrip/components/web/VideoMutedIndicator.js b/react/features/filmstrip/components/web/VideoMutedIndicator.js index f10bc7604..ea4a668bd 100644 --- a/react/features/filmstrip/components/web/VideoMutedIndicator.js +++ b/react/features/filmstrip/components/web/VideoMutedIndicator.js @@ -7,6 +7,18 @@ import BaseIndicator from './BaseIndicator'; * @extends Component */ class VideoMutedIndicator extends Component { + /** + * {@code VideoMutedIndicator} component's property types. + * + * @static + */ + static propTypes = { + /** + * From which side of the indicator the tooltip should appear from. + */ + tooltipPosition: React.PropTypes.string + }; + /** * Implements React's {@link Component#render()}. * @@ -17,7 +29,8 @@ class VideoMutedIndicator extends Component { + tooltipKey = 'videothumbnail.videomute' + tooltipPosition = { this.props.tooltipPosition } /> ); } }