From 584ec7c82e35093712210d4d0999b5a55c108c7c Mon Sep 17 00:00:00 2001 From: robertpin Date: Tue, 21 Sep 2021 20:30:24 +0300 Subject: [PATCH] fix(reactions) Reactions improvements (#9964) * Register shortcuts on mount * Changed icon for reactions menu * Enable reactions by default * Fix unreadCount bug When having unread messages and sending a reaction the unread count now shows the correct count * Fix overflow menu bottom color when reactions are enabled * Revert raise hand icon * Update raise hand functionality On desktop show raise button with arrow for reactions. Only show raise hand in the reactions menu on mobile * Fix lint error Add required prop to ToolboxButtonWithIcon * Legacy support for enable reactions If disableReactions is undefined treat it as true * Remove unnecessary code * Fix unread counter showing negative count * Fix unreadCount with reactions UnreadCount ignores all reactions messages * Fixed typo * Fix background color --- config.js | 4 +- css/_reactions-menu.scss | 2 +- react/features/base/config/configWhitelist.js | 2 +- .../dialog/components/native/BottomSheet.js | 6 +- .../base/dialog/components/native/styles.js | 7 ++ react/features/chat/actions.any.js | 3 + react/features/chat/functions.js | 20 ++++- react/features/chat/middleware.js | 18 ++-- react/features/chat/reducer.js | 1 + .../reactions/components/web/ReactionsMenu.js | 55 +++++++----- .../components/web/ReactionsMenuButton.js | 73 +++++++++++----- react/features/reactions/functions.any.js | 6 +- .../settings/components/web/SoundsTab.js | 10 +-- react/features/settings/functions.js | 3 +- .../toolbox/components/web/RaiseHandButton.js | 84 ------------------- .../toolbox/components/web/Toolbox.js | 68 ++------------- 16 files changed, 154 insertions(+), 208 deletions(-) delete mode 100644 react/features/toolbox/components/web/RaiseHandButton.js diff --git a/config.js b/config.js index 5790fbaa3..1695557e7 100644 --- a/config.js +++ b/config.js @@ -77,8 +77,8 @@ var config = { // Disables moderator indicators. // disableModeratorIndicator: false, - // Enables reactions feature. - // enableReactions: false, + // Disables the reactions feature. + // disableReactions: true, // Disables polls feature. // disablePolls: false, diff --git a/css/_reactions-menu.scss b/css/_reactions-menu.scss index 7c5ebfb18..6d40b008c 100644 --- a/css/_reactions-menu.scss +++ b/css/_reactions-menu.scss @@ -2,7 +2,7 @@ .reactions-menu { width: 280px; - background: #292929; + background: $menuBG; box-shadow: 0px 3px 16px rgba(0, 0, 0, 0.6), 0px 0px 4px 1px rgba(0, 0, 0, 0.25); border-radius: 3px; padding: 16px; diff --git a/react/features/base/config/configWhitelist.js b/react/features/base/config/configWhitelist.js index dfb1adf6e..fa65bed93 100644 --- a/react/features/base/config/configWhitelist.js +++ b/react/features/base/config/configWhitelist.js @@ -100,6 +100,7 @@ export default [ 'disableNS', 'disablePolls', 'disableProfile', + 'disableReactions', 'disableRecordAudioNotification', 'disableRemoteControl', 'disableRemoteMute', @@ -127,7 +128,6 @@ export default [ 'enableLayerSuspension', 'enableLipSync', 'enableOpusRed', - 'enableReactions', 'enableRemb', 'enableSaveLogs', 'enableScreenshotCapture', diff --git a/react/features/base/dialog/components/native/BottomSheet.js b/react/features/base/dialog/components/native/BottomSheet.js index e4a913894..8ec9a453e 100644 --- a/react/features/base/dialog/components/native/BottomSheet.js +++ b/react/features/base/dialog/components/native/BottomSheet.js @@ -145,6 +145,7 @@ class BottomSheet extends PureComponent { renderHeader ? _styles.sheetHeader : _styles.sheet, + renderFooter && _styles.sheetFooter, style, { maxHeight: _height - 100 @@ -154,7 +155,10 @@ class BottomSheet extends PureComponent { + style = { [ + renderFooter && _styles.sheet, + addScrollViewPadding && styles.scrollView + ] } > { this.props.children } { renderFooter && renderFooter() } diff --git a/react/features/base/dialog/components/native/styles.js b/react/features/base/dialog/components/native/styles.js index 6df2b4fc2..69bfbde5a 100644 --- a/react/features/base/dialog/components/native/styles.js +++ b/react/features/base/dialog/components/native/styles.js @@ -213,6 +213,13 @@ ColorSchemeRegistry.register('BottomSheet', { */ sheetHeader: { backgroundColor: BaseTheme.palette.ui02 + }, + + /** + * Bottom sheet's background color with footer. + */ + sheetFooter: { + backgroundColor: BaseTheme.palette.bottomSheet } }); diff --git a/react/features/chat/actions.any.js b/react/features/chat/actions.any.js index 17f6cbfa2..92059d1d8 100644 --- a/react/features/chat/actions.any.js +++ b/react/features/chat/actions.any.js @@ -22,6 +22,8 @@ import { * "error" or "local" or "remote". * @param {string} messageDetails.timestamp - A timestamp to display for when * the message was received. + * @param {string} messageDetails.isReaction - Whether or not the + * message is a reaction message. * @returns {{ * type: ADD_MESSAGE, * displayName: string, @@ -29,6 +31,7 @@ import { * message: string, * messageType: string, * timestamp: string, + * isReaction: boolean * }} */ export function addMessage(messageDetails: Object) { diff --git a/react/features/chat/functions.js b/react/features/chat/functions.js index 24d04a236..6ce421fb8 100644 --- a/react/features/chat/functions.js +++ b/react/features/chat/functions.js @@ -69,14 +69,30 @@ export function getUnreadCount(state: Object) { return 0; } + let reactionMessages = 0; + if (navigator.product === 'ReactNative') { // React native stores the messages in a reversed order. - return messages.indexOf(lastReadMessage); + const lastReadIndex = messages.indexOf(lastReadMessage); + + for (let i = 0; i < lastReadIndex; i++) { + if (messages[i].isReaction) { + reactionMessages++; + } + } + + return lastReadIndex - reactionMessages; } const lastReadIndex = messages.lastIndexOf(lastReadMessage); - return messagesCount - (lastReadIndex + 1); + for (let i = lastReadIndex + 1; i < messagesCount; i++) { + if (messages[i].isReaction) { + reactionMessages++; + } + } + + return messagesCount - (lastReadIndex + 1) - reactionMessages; } /** diff --git a/react/features/chat/middleware.js b/react/features/chat/middleware.js index 8e409147c..d5a8eb484 100644 --- a/react/features/chat/middleware.js +++ b/react/features/chat/middleware.js @@ -68,7 +68,12 @@ MiddlewareRegistry.register(store => next => action => { switch (action.type) { case ADD_MESSAGE: - unreadCount = action.hasRead ? 0 : getUnreadCount(getState()) + 1; + unreadCount = getUnreadCount(getState()); + if (action.isReaction) { + action.hasRead = false; + } else { + unreadCount = action.hasRead ? 0 : unreadCount + 1; + } isOpen = getState()['features/chat'].isOpen; if (typeof APP !== 'undefined') { @@ -171,7 +176,7 @@ MiddlewareRegistry.register(store => next => action => { message: action.message, privateMessage: false, timestamp: Date.now() - }, false); + }, false, true); } } @@ -270,7 +275,7 @@ function _addChatMsgListener(conference, store) { message: getReactionMessageFromBuffer(eventData.reactions), privateMessage: false, timestamp: eventData.timestamp - }, false); + }, false, true); } } }); @@ -304,11 +309,13 @@ function _handleChatError({ dispatch }, error) { * @param {Store} store - The Redux store. * @param {Object} message - The message object. * @param {boolean} shouldPlaySound - Whether or not to play the incoming message sound. + * @param {boolean} isReaction - Whether or not the message is a reaction message. * @returns {void} */ function _handleReceivedMessage({ dispatch, getState }, { id, message, privateMessage, timestamp }, - shouldPlaySound = true + shouldPlaySound = true, + isReaction = false ) { // Logic for all platforms: const state = getState(); @@ -337,7 +344,8 @@ function _handleReceivedMessage({ dispatch, getState }, message, privateMessage, recipient: getParticipantDisplayName(state, localParticipant.id), - timestamp: millisecondsTimestamp + timestamp: millisecondsTimestamp, + isReaction })); if (typeof APP !== 'undefined') { diff --git a/react/features/chat/reducer.js b/react/features/chat/reducer.js index 3390b5955..20a4f3d95 100644 --- a/react/features/chat/reducer.js +++ b/react/features/chat/reducer.js @@ -28,6 +28,7 @@ ReducerRegistry.register('features/chat', (state = DEFAULT_STATE, action) => { displayName: action.displayName, error: action.error, id: action.id, + isReaction: action.isReaction, messageType: action.messageType, message: action.message, privateMessage: action.privateMessage, diff --git a/react/features/reactions/components/web/ReactionsMenu.js b/react/features/reactions/components/web/ReactionsMenu.js index 1a7a6a38f..825686113 100644 --- a/react/features/reactions/components/web/ReactionsMenu.js +++ b/react/features/reactions/components/web/ReactionsMenu.js @@ -8,6 +8,7 @@ import { createToolbarEvent, sendAnalytics } from '../../../analytics'; +import { isMobileBrowser } from '../../../base/environment/utils'; import { translate } from '../../../base/i18n'; import { getLocalParticipant, participantUpdated } from '../../../base/participants'; import { connect } from '../../../base/redux'; @@ -21,34 +22,39 @@ import ReactionButton from './ReactionButton'; type Props = { /** - * Used for translation. + * Docks the toolbox */ - t: Function, + _dockToolbox: Function, /** - * Whether or not the local participant's hand is raised. + * Whether or not it's a mobile browser. */ - _raisedHand: boolean, + _isMobile: boolean, /** * The ID of the local participant. */ _localParticipantID: String, + /** + * Whether or not the local participant's hand is raised. + */ + _raisedHand: boolean, + /** * The Redux Dispatch function. */ dispatch: Function, - /** - * Docks the toolbox - */ - _dockToolbox: Function, - /** * Whether or not it's displayed in the overflow menu. */ - overflowMenu: boolean + overflowMenu: boolean, + + /** + * Used for translation. + */ + t: Function }; declare var APP: Object; @@ -177,25 +183,27 @@ class ReactionsMenu extends Component { * @inheritdoc */ render() { - const { _raisedHand, t, overflowMenu } = this.props; + const { _raisedHand, t, overflowMenu, _isMobile } = this.props; return (
{ this._getReactionButtons() }
-
- -
+ {_isMobile && ( +
+ +
+ )}
); } @@ -212,6 +220,7 @@ function mapStateToProps(state) { return { _localParticipantID: localParticipant.id, + _isMobile: isMobileBrowser(), _raisedHand: localParticipant.raisedHand }; } diff --git a/react/features/reactions/components/web/ReactionsMenuButton.js b/react/features/reactions/components/web/ReactionsMenuButton.js index dde67b921..442c5cef4 100644 --- a/react/features/reactions/components/web/ReactionsMenuButton.js +++ b/react/features/reactions/components/web/ReactionsMenuButton.js @@ -2,14 +2,16 @@ import React from 'react'; +import { isMobileBrowser } from '../../../base/environment/utils'; import { translate } from '../../../base/i18n'; -import { IconRaisedHand } from '../../../base/icons'; +import { IconArrowUp, IconRaisedHand } from '../../../base/icons'; import { getLocalParticipant } from '../../../base/participants'; import { connect } from '../../../base/redux'; +import { ToolboxButtonWithIcon } from '../../../base/toolbox/components'; import ToolbarButton from '../../../toolbox/components/web/ToolbarButton'; import { toggleReactionsMenuVisibility } from '../../actions.web'; import { type ReactionEmojiProps } from '../../constants'; -import { getReactionsQueue } from '../../functions.any'; +import { getReactionsQueue, isReactionsEnabled } from '../../functions.any'; import { getReactionsMenuVisibility } from '../../functions.web'; import ReactionEmoji from './ReactionEmoji'; @@ -18,34 +20,44 @@ import ReactionsMenuPopup from './ReactionsMenuPopup'; type Props = { /** - * Used for translation. + * Whether or not reactions are enabled. */ - t: Function, + _reactionsEnabled: Boolean, /** - * Whether or not the local participant's hand is raised. + * Redux dispatch function. */ - raisedHand: boolean, + dispatch: Function, /** - * Click handler for the reaction button. Toggles the reactions menu. + * Click handler for raise hand functionality. */ - onReactionsClick: Function, + handleClick: Function, /** * Whether or not the reactions menu is open. */ isOpen: boolean, + /** + * Whether or not it's a mobile browser. + */ + isMobile: boolean, + + /** + * Whether or not the local participant's hand is raised. + */ + raisedHand: boolean, + /** * The array of reactions to be displayed. */ reactionsQueue: Array, /** - * Redux dispatch function. + * Used for translation. */ - dispatch: Function + t: Function }; @@ -57,11 +69,14 @@ declare var APP: Object; * @returns {ReactElement} */ function ReactionsMenuButton({ - t, - raisedHand, + _reactionsEnabled, + dispatch, + handleClick, isOpen, + isMobile, + raisedHand, reactionsQueue, - dispatch + t }: Props) { /** @@ -73,16 +88,32 @@ function ReactionsMenuButton({ dispatch(toggleReactionsMenuVisibility()); } + const raiseHandButton = (); + return (
- + {!_reactionsEnabled || isMobile ? raiseHandButton + : ( + + {raiseHandButton} + + )} {reactionsQueue.map(({ reaction, uid }, index) => () { * @returns {boolean} */ export function isReactionsEnabled(state: Object) { - const { enableReactions } = state['features/base/config']; + const { disableReactions } = state['features/base/config']; if (navigator.product === 'ReactNative') { - return enableReactions && getFeatureFlag(state, REACTIONS_ENABLED, true); + return !disableReactions && getFeatureFlag(state, REACTIONS_ENABLED, true); } - return enableReactions; + return !disableReactions; } diff --git a/react/features/settings/components/web/SoundsTab.js b/react/features/settings/components/web/SoundsTab.js index 5b1bfe478..520a5b77d 100644 --- a/react/features/settings/components/web/SoundsTab.js +++ b/react/features/settings/components/web/SoundsTab.js @@ -15,6 +15,11 @@ declare var APP: Object; export type Props = { ...$Exact, + /** + * Whether or not the reactions feature is enabled. + */ + enableReactions: Boolean, + /** * Whether or not the sound for the incoming message should play. */ @@ -40,11 +45,6 @@ export type Props = { */ soundsReactions: Boolean, - /** - * Whether or not the reactions feature is enabled. - */ - enableReactions: Boolean, - /** * Invoked to obtain translated strings. */ diff --git a/react/features/settings/functions.js b/react/features/settings/functions.js index 1dfbeb758..cdd24b7b6 100644 --- a/react/features/settings/functions.js +++ b/react/features/settings/functions.js @@ -10,6 +10,7 @@ import { import { toState } from '../base/redux'; import { parseStandardURIString } from '../base/util'; import { isFollowMeActive } from '../follow-me'; +import { isReactionsEnabled } from '../reactions/functions.any'; import { SS_DEFAULT_FRAME_RATE, SS_SUPPORTED_FRAMERATES } from './constants'; @@ -174,7 +175,7 @@ export function getSoundsTabProps(stateful: Object | Function) { soundsTalkWhileMuted, soundsReactions } = state['features/base/settings']; - const { enableReactions } = state['features/base/config']; + const enableReactions = isReactionsEnabled(state); return { soundsIncomingMessage, diff --git a/react/features/toolbox/components/web/RaiseHandButton.js b/react/features/toolbox/components/web/RaiseHandButton.js deleted file mode 100644 index 8a0e4153c..000000000 --- a/react/features/toolbox/components/web/RaiseHandButton.js +++ /dev/null @@ -1,84 +0,0 @@ -// @flow - -import { translate } from '../../../base/i18n'; -import { IconRaisedHand } from '../../../base/icons'; -import { getLocalParticipant } from '../../../base/participants'; -import { connect } from '../../../base/redux'; -import { AbstractButton, type AbstractButtonProps } from '../../../base/toolbox/components'; - -type Props = AbstractButtonProps & { - - /** - * Whether or not the local participant's hand is raised. - */ - _raisedHand: boolean, -}; - -/** - * Implementation of a button for toggling raise hand functionality. - */ -class RaiseHandButton extends AbstractButton { - accessibilityLabel = 'toolbar.accessibilityLabel.raiseHand'; - icon = IconRaisedHand - label = 'toolbar.raiseYourHand'; - toggledLabel = 'toolbar.lowerYourHand' - - /** - * Retrieves tooltip dynamically. - */ - get tooltip() { - return this.props._raisedHand ? 'toolbar.lowerYourHand' : 'toolbar.raiseYourHand'; - } - - /** - * Required by linter due to AbstractButton overwritten prop being writable. - * - * @param {string} value - The value. - */ - set tooltip(value) { - return value; - } - - /** - * Handles clicking / pressing the button, and opens the appropriate dialog. - * - * @protected - * @returns {void} - */ - _handleClick() { - const { handleClick } = this.props; - - if (handleClick) { - handleClick(); - - return; - } - } - - /** - * Indicates whether this button is in toggled state or not. - * - * @override - * @protected - * @returns {boolean} - */ - _isToggled() { - return this.props._raisedHand; - } -} - -/** - * Function that maps parts of Redux state tree into component props. - * - * @param {Object} state - Redux state. - * @returns {Object} - */ -const mapStateToProps = state => { - const localParticipant = getLocalParticipant(state); - - return { - _raisedHand: localParticipant.raisedHand - }; -}; - -export default translate(connect(mapStateToProps)(RaiseHandButton)); diff --git a/react/features/toolbox/components/web/Toolbox.js b/react/features/toolbox/components/web/Toolbox.js index a58306c66..0d7ec6c73 100644 --- a/react/features/toolbox/components/web/Toolbox.js +++ b/react/features/toolbox/components/web/Toolbox.js @@ -17,7 +17,6 @@ import { translate } from '../../../base/i18n'; import JitsiMeetJS from '../../../base/lib-jitsi-meet'; import { getLocalParticipant, - getParticipantCount, haveParticipantWithScreenSharingFeature, raiseHand } from '../../../base/participants'; @@ -87,7 +86,6 @@ import AudioSettingsButton from './AudioSettingsButton'; import FullscreenButton from './FullscreenButton'; import OverflowMenuButton from './OverflowMenuButton'; import ProfileButton from './ProfileButton'; -import RaiseHandButton from './RaiseHandButton'; import Separator from './Separator'; import ShareDesktopButton from './ShareDesktopButton'; import ToggleCameraButton from './ToggleCameraButton'; @@ -180,11 +178,6 @@ type Props = { */ _overflowMenuVisible: boolean, - /** - * Number of participants in the conference. - */ - _participantCount: number, - /** * Whether or not the participants pane is open. */ @@ -254,16 +247,12 @@ type Props = { declare var APP: Object; -type State = { - reactionsShortcutsRegistered: boolean -}; - /** * Implements the conference toolbox on React/Web. * * @extends Component */ -class Toolbox extends Component { +class Toolbox extends Component { /** * Initializes a new {@code Toolbox} instance. * @@ -273,10 +262,6 @@ class Toolbox extends Component { constructor(props: Props) { super(props); - this.state = { - reactionsShortcutsRegistered: false - }; - // Bind event handlers so they are only bound once per instance. this._onMouseOut = this._onMouseOut.bind(this); this._onMouseOver = this._onMouseOver.bind(this); @@ -306,7 +291,7 @@ class Toolbox extends Component { * @returns {void} */ componentDidMount() { - const { _toolbarButtons, t, dispatch, _reactionsEnabled, _participantCount } = this.props; + const { _toolbarButtons, t, dispatch, _reactionsEnabled } = this.props; const KEYBOARD_SHORTCUTS = [ isToolbarButtonEnabled('videoquality', _toolbarButtons) && { character: 'A', @@ -355,7 +340,7 @@ class Toolbox extends Component { } }); - if (_reactionsEnabled && _participantCount > 1) { + if (_reactionsEnabled) { const REACTION_SHORTCUTS = Object.keys(REACTIONS).map(key => { const onShortcutSendReaction = () => { dispatch(addReactionToBuffer(key)); @@ -389,7 +374,7 @@ class Toolbox extends Component { * @inheritdoc */ componentDidUpdate(prevProps) { - const { _dialog, _reactionsEnabled, _participantCount, dispatch, t } = this.props; + const { _dialog, dispatch } = this.props; if (prevProps._overflowMenuVisible @@ -398,41 +383,6 @@ class Toolbox extends Component { this._onSetOverflowVisible(false); dispatch(setToolbarHovered(false)); } - - if (!this.state.reactionsShortcutsRegistered - && (prevProps._reactionsEnabled !== _reactionsEnabled - || prevProps._participantCount !== _participantCount)) { - if (_reactionsEnabled && _participantCount > 1) { - // eslint-disable-next-line react/no-did-update-set-state - this.setState({ - reactionsShortcutsRegistered: true - }); - const REACTION_SHORTCUTS = Object.keys(REACTIONS).map(key => { - const onShortcutSendReaction = () => { - dispatch(addReactionToBuffer(key)); - sendAnalytics(createShortcutEvent( - `reaction.${key}` - )); - }; - - return { - character: REACTIONS[key].shortcutChar, - exec: onShortcutSendReaction, - helpDescription: t(`toolbar.reaction${key.charAt(0).toUpperCase()}${key.slice(1)}`), - altKey: true - }; - }); - - REACTION_SHORTCUTS.forEach(shortcut => { - APP.keyboardshortcut.registerShortcut( - shortcut.character, - null, - shortcut.exec, - shortcut.helpDescription, - shortcut.altKey); - }); - } - } } /** @@ -445,7 +395,7 @@ class Toolbox extends Component { [ 'A', 'C', 'D', 'R', 'S' ].forEach(letter => APP.keyboardshortcut.unregisterShortcut(letter)); - if (this.props._reactionsEnabled && this.state.reactionsShortcutsRegistered) { + if (this.props._reactionsEnabled) { Object.keys(REACTIONS).map(key => REACTIONS[key].shortcutChar) .forEach(letter => APP.keyboardshortcut.unregisterShortcut(letter, true)); @@ -613,8 +563,7 @@ class Toolbox extends Component { const { _feedbackConfigured, _isMobile, - _screenSharing, - _reactionsEnabled + _screenSharing } = this.props; const microphone = { @@ -651,8 +600,8 @@ class Toolbox extends Component { const raisehand = { key: 'raisehand', - Content: _reactionsEnabled ? ReactionsMenuButton : RaiseHandButton, - handleClick: _reactionsEnabled ? null : this._onToolbarToggleRaiseHand, + Content: ReactionsMenuButton, + handleClick: this._onToolbarToggleRaiseHand, group: 2 }; @@ -1388,7 +1337,6 @@ function _mapStateToProps(state, ownProps) { _localParticipantID: localParticipant?.id, _localVideo: localVideo, _overflowMenuVisible: overflowMenuVisible, - _participantCount: getParticipantCount(state), _participantsPaneOpen: getParticipantsPaneOpen(state), _raisedHand: localParticipant?.raisedHand, _reactionsEnabled: isReactionsEnabled(state),