From f14095ecfc2e8f5ba9fcf57613a1dbba8f68233a Mon Sep 17 00:00:00 2001 From: hristoterezov Date: Mon, 16 Apr 2018 13:44:08 +0100 Subject: [PATCH] feat(deep_linking): add analytics In order to be able to add analytics to the deep-linking pages the lib-jitsi-meet initialization has been moved so it happens earlier. The introduced `initPromise` will eventually disappear, once conference is migrated into React and / or support for Temasys is dropped. At that stage, it can be turned into a sync function which all platforms share. --- conference.js | 51 +++------------- react/features/analytics/AnalyticsEvents.js | 19 ++++++ react/features/base/connection/actions.web.js | 30 +++++----- .../base/lib-jitsi-meet/actionTypes.js | 10 ++++ react/features/base/lib-jitsi-meet/actions.js | 28 +++++---- .../base/lib-jitsi-meet/middleware.js | 53 ++++++++++++++++- react/features/base/lib-jitsi-meet/reducer.js | 10 +++- react/features/base/logging/middleware.js | 6 +- .../components/DeepLinkingDesktopPage.js | 10 ++++ .../components/DeepLinkingMobilePage.js | 59 ++++++++++++++++++- .../deep-linking/components/NoMobileApp.js | 12 ++++ .../welcome/components/WelcomePage.web.js | 15 +---- 12 files changed, 215 insertions(+), 88 deletions(-) diff --git a/conference.js b/conference.js index 2bf68c48a..48c353877 100644 --- a/conference.js +++ b/conference.js @@ -21,7 +21,6 @@ import { createSelectParticipantFailedEvent, createStreamSwitchDelayEvent, createTrackMutedEvent, - initAnalytics, sendAnalytics } from './react/features/analytics'; import { @@ -49,7 +48,6 @@ import { } from './react/features/base/conference'; import { updateDeviceList } from './react/features/base/devices'; import { - isAnalyticsEnabled, isFatalJitsiConnectionError, JitsiConferenceErrors, JitsiConferenceEvents, @@ -685,53 +683,20 @@ export default { /** * Open new connection and join to the conference. * @param {object} options - * @param {string} roomName name of the conference + * @param {string} roomName - The name of the conference. * @returns {Promise} */ init(options) { this.roomName = options.roomName; - // attaches global error handler, if there is already one, respect it - if (JitsiMeetJS.getGlobalOnErrorHandler) { - const oldOnErrorHandler = window.onerror; - - // eslint-disable-next-line max-params - window.onerror = (message, source, lineno, colno, error) => { - JitsiMeetJS.getGlobalOnErrorHandler( - message, source, lineno, colno, error); - - if (oldOnErrorHandler) { - oldOnErrorHandler(message, source, lineno, colno, error); - } - }; - - const oldOnUnhandledRejection = window.onunhandledrejection; - - window.onunhandledrejection = function(event) { - JitsiMeetJS.getGlobalOnErrorHandler( - null, null, null, null, event.reason); - - if (oldOnUnhandledRejection) { - oldOnUnhandledRejection(event); - } - }; - } - return ( - JitsiMeetJS.init({ - enableAnalyticsLogging: isAnalyticsEnabled(APP.store), - ...config - }).then(() => { - initAnalytics(APP.store); - - return this.createInitialLocalTracksAndConnect( - options.roomName, { - startAudioOnly: config.startAudioOnly, - startScreenSharing: config.startScreenSharing, - startWithAudioMuted: config.startWithAudioMuted, - startWithVideoMuted: config.startWithVideoMuted - }); - }) + this.createInitialLocalTracksAndConnect( + options.roomName, { + startAudioOnly: config.startAudioOnly, + startScreenSharing: config.startScreenSharing, + startWithAudioMuted: config.startWithAudioMuted, + startWithVideoMuted: config.startWithVideoMuted + }) .then(([ tracks, con ]) => { tracks.forEach(track => { if ((track.isAudioTrack() && this.isLocalAudioMuted()) diff --git a/react/features/analytics/AnalyticsEvents.js b/react/features/analytics/AnalyticsEvents.js index f0bfc96bf..41bdd7576 100644 --- a/react/features/analytics/AnalyticsEvents.js +++ b/react/features/analytics/AnalyticsEvents.js @@ -97,6 +97,25 @@ export function createAudioOnlyChangedEvent(enabled) { }; } +/** + * Creates an event for an action on the deep linking page. + * + * @param {string} action - The action that the event represents. + * @param {string} actionSubject - The subject that was acted upon. + * @param {boolean} attributes - Additional attributes to attach to the event. + * @returns {Object} The event in a format suitable for sending via + * sendAnalytics. + */ +export function createDeepLinkingPageEvent( + action, actionSubject, attributes = {}) { + return { + action, + actionSubject, + source: 'deepLinkingPage', + attributes + }; +} + /** * Creates an event which indicates that a device was changed. * diff --git a/react/features/base/connection/actions.web.js b/react/features/base/connection/actions.web.js index 69c65946f..01a1b45d9 100644 --- a/react/features/base/connection/actions.web.js +++ b/react/features/base/connection/actions.web.js @@ -30,24 +30,26 @@ export function connect() { // XXX Lib-jitsi-meet does not accept uppercase letters. const room = state['features/base/conference'].room.toLowerCase(); + const { initPromise } = state['features/base/lib-jitsi-meet']; // XXX For web based version we use conference initialization logic // from the old app (at the moment of writing). - return APP.conference.init({ roomName: room }) - .catch(error => { - APP.API.notifyConferenceLeft(APP.conference.roomName); - logger.error(error); + return initPromise.then(() => APP.conference.init({ + roomName: room + })).catch(error => { + APP.API.notifyConferenceLeft(APP.conference.roomName); + logger.error(error); - // TODO The following are in fact Errors raised by - // JitsiMeetJS.init() which should be taken care of in - // features/base/lib-jitsi-meet but we are not there yet on the - // Web at the time of this writing. - switch (error.name) { - case WEBRTC_NOT_READY: - case WEBRTC_NOT_SUPPORTED: - dispatch(libInitError(error)); - } - }); + // TODO The following are in fact Errors raised by + // JitsiMeetJS.init() which should be taken care of in + // features/base/lib-jitsi-meet but we are not there yet on the + // Web at the time of this writing. + switch (error.name) { + case WEBRTC_NOT_READY: + case WEBRTC_NOT_SUPPORTED: + dispatch(libInitError(error)); + } + }); }; } diff --git a/react/features/base/lib-jitsi-meet/actionTypes.js b/react/features/base/lib-jitsi-meet/actionTypes.js index 25f9a44f5..39a0cca35 100644 --- a/react/features/base/lib-jitsi-meet/actionTypes.js +++ b/react/features/base/lib-jitsi-meet/actionTypes.js @@ -27,6 +27,16 @@ export const LIB_DID_INIT = Symbol('LIB_DID_INIT'); */ export const LIB_INIT_ERROR = Symbol('LIB_INIT_ERROR'); +/** + * Action to dispatch the promise returned by JitsiMeetJS.init. + * + * { + * type: LIB_INIT_PROMISE_CREATED, + * initPromise: Promise + * } + */ +export const LIB_INIT_PROMISE_CREATED = Symbol('LIB_INIT_PROMISE_CREATED'); + /** * The type of Redux action which signals that {@link JitsiMeetJS} will be * disposed. diff --git a/react/features/base/lib-jitsi-meet/actions.js b/react/features/base/lib-jitsi-meet/actions.js index 132bda2d6..21a27a189 100644 --- a/react/features/base/lib-jitsi-meet/actions.js +++ b/react/features/base/lib-jitsi-meet/actions.js @@ -7,6 +7,7 @@ import { LIB_DID_DISPOSE, LIB_DID_INIT, LIB_INIT_ERROR, + LIB_INIT_PROMISE_CREATED, LIB_WILL_DISPOSE, LIB_WILL_INIT, SET_WEBRTC_READY @@ -44,22 +45,27 @@ export function initLib() { throw new Error('Cannot init lib-jitsi-meet without config'); } - // FIXME Until the logic of conference.js is rewritten into the React - // app we, JitsiMeetJS.init is to not be used for the React app. - if (typeof APP !== 'undefined') { - return Promise.resolve(); - } - dispatch({ type: LIB_WILL_INIT }); + const initPromise = JitsiMeetJS.init({ + enableAnalyticsLogging: isAnalyticsEnabled(getState), + ...config + }); + + dispatch({ + type: LIB_INIT_PROMISE_CREATED, + initPromise + }); + return ( - JitsiMeetJS.init({ - enableAnalyticsLogging: isAnalyticsEnabled(getState), - ...config - }) + initPromise .then(() => dispatch({ type: LIB_DID_INIT })) .catch(error => { - dispatch(libInitError(error)); + // TODO: See the comment in the connect action in + // base/connection/actions.web.js. + if (typeof APP === 'undefined') { + dispatch(libInitError(error)); + } // TODO Handle LIB_INIT_ERROR error somewhere instead. console.error('lib-jitsi-meet failed to init:', error); diff --git a/react/features/base/lib-jitsi-meet/middleware.js b/react/features/base/lib-jitsi-meet/middleware.js index f6f0b5b08..aa360c703 100644 --- a/react/features/base/lib-jitsi-meet/middleware.js +++ b/react/features/base/lib-jitsi-meet/middleware.js @@ -5,10 +5,13 @@ import { setLoggingConfig } from '../logging'; import { PARTICIPANT_LEFT } from '../participants'; import { MiddlewareRegistry } from '../redux'; +import JitsiMeetJS from './_'; import { disposeLib, initLib, setWebRTCReady } from './actions'; -import { LIB_DID_INIT, LIB_INIT_ERROR } from './actionTypes'; +import { LIB_DID_INIT, LIB_INIT_ERROR, LIB_WILL_INIT } from './actionTypes'; import { WEBRTC_NOT_READY, WEBRTC_NOT_SUPPORTED } from './constants'; +declare var APP: Object; + /** * Middleware that captures PARTICIPANT_LEFT action for a local participant * (which signalizes that we finally left the app) and disposes lib-jitsi-meet. @@ -21,8 +24,21 @@ import { WEBRTC_NOT_READY, WEBRTC_NOT_SUPPORTED } from './constants'; */ MiddlewareRegistry.register(store => next => action => { switch (action.type) { + case LIB_WILL_INIT: + // Moved from conference.js init method. It appears the error handlers + // are not used for mobile. + if (typeof APP !== 'undefined') { + _setErrorHandlers(); + } + break; case LIB_DID_INIT: - store.dispatch(setWebRTCReady(true)); + // FIXME: The web version doesn't need this action during initialization + // because it is still using the old logic from conference.js. We still + // have to reactify the old logic from conference.js and then maybe + // we'll need this action for web too. + if (typeof APP === 'undefined') { + store.dispatch(setWebRTCReady(true)); + } break; case LIB_INIT_ERROR: @@ -119,3 +135,36 @@ function _setConfig({ dispatch, getState }, next, action) { return result; } + +/** + * Attaches our custom error handlers to the window object. + * + * @returns {void} + */ +function _setErrorHandlers() { + // attaches global error handler, if there is already one, respect it + if (JitsiMeetJS.getGlobalOnErrorHandler) { + const oldOnErrorHandler = window.onerror; + + // eslint-disable-next-line max-params + window.onerror = (message, source, lineno, colno, error) => { + JitsiMeetJS.getGlobalOnErrorHandler( + message, source, lineno, colno, error); + + if (oldOnErrorHandler) { + oldOnErrorHandler(message, source, lineno, colno, error); + } + }; + + const oldOnUnhandledRejection = window.onunhandledrejection; + + window.onunhandledrejection = function(event) { + JitsiMeetJS.getGlobalOnErrorHandler( + null, null, null, null, event.reason); + + if (oldOnUnhandledRejection) { + oldOnUnhandledRejection(event); + } + }; + } +} diff --git a/react/features/base/lib-jitsi-meet/reducer.js b/react/features/base/lib-jitsi-meet/reducer.js index 5825d620d..184b99593 100644 --- a/react/features/base/lib-jitsi-meet/reducer.js +++ b/react/features/base/lib-jitsi-meet/reducer.js @@ -6,6 +6,7 @@ import { LIB_DID_DISPOSE, LIB_DID_INIT, LIB_INIT_ERROR, + LIB_INIT_PROMISE_CREATED, SET_WEBRTC_READY } from './actionTypes'; @@ -34,7 +35,14 @@ ReducerRegistry.register( return { ...state, initError: action.error, - initialized: false + initialized: false, + initPromise: undefined + }; + + case LIB_INIT_PROMISE_CREATED: + return { + ...state, + initPromise: action.initPromise }; case SET_WEBRTC_READY: diff --git a/react/features/base/logging/middleware.js b/react/features/base/logging/middleware.js index c16bead1e..032817383 100644 --- a/react/features/base/logging/middleware.js +++ b/react/features/base/logging/middleware.js @@ -114,7 +114,11 @@ function _initLogging(loggingConfig, isTestingEnabled) { * specified {@code action}. */ function _libWillInit({ getState }, next, action) { - _setLogLevels(JitsiMeetJS, getState()['features/base/logging'].config); + // Adding the if in order to preserve the logic for web after enabling + // LIB_WILL_INIT action for web in initLib action. + if (typeof APP === 'undefined') { + _setLogLevels(JitsiMeetJS, getState()['features/base/logging'].config); + } return next(action); } diff --git a/react/features/deep-linking/components/DeepLinkingDesktopPage.js b/react/features/deep-linking/components/DeepLinkingDesktopPage.js index 4d222b8dc..3e79fc339 100644 --- a/react/features/deep-linking/components/DeepLinkingDesktopPage.js +++ b/react/features/deep-linking/components/DeepLinkingDesktopPage.js @@ -5,6 +5,7 @@ import { AtlasKitThemeProvider } from '@atlaskit/theme'; import React, { Component } from 'react'; import { connect } from 'react-redux'; +import { createDeepLinkingPageEvent, sendAnalytics } from '../../analytics'; import { translate } from '../../base/i18n'; import { @@ -60,6 +61,9 @@ class DeepLinkingDesktopPage

extends Component

{ */ componentDidMount() { this._openDesktopApp(); + sendAnalytics( + createDeepLinkingPageEvent( + 'displayed', 'DeepLinkingDesktop', { isMobileBrowser: false })); } /** @@ -147,6 +151,9 @@ class DeepLinkingDesktopPage

extends Component

{ * @returns {void} */ _onTryAgain() { + sendAnalytics( + createDeepLinkingPageEvent( + 'clicked', 'tryAgainButton', { isMobileBrowser: false })); this._openDesktopApp(); } @@ -158,6 +165,9 @@ class DeepLinkingDesktopPage

extends Component

{ * @returns {void} */ _onLaunchWeb() { + sendAnalytics( + createDeepLinkingPageEvent( + 'clicked', 'launchWebButton', { isMobileBrowser: false })); this.props.dispatch(openWebApp()); } } diff --git a/react/features/deep-linking/components/DeepLinkingMobilePage.js b/react/features/deep-linking/components/DeepLinkingMobilePage.js index ffbea5172..64a585315 100644 --- a/react/features/deep-linking/components/DeepLinkingMobilePage.js +++ b/react/features/deep-linking/components/DeepLinkingMobilePage.js @@ -4,6 +4,7 @@ import PropTypes from 'prop-types'; import React, { Component } from 'react'; import { connect } from 'react-redux'; +import { createDeepLinkingPageEvent, sendAnalytics } from '../../analytics'; import { translate, translateToHTML } from '../../base/i18n'; import { HideNotificationBarStyle, Platform } from '../../base/react'; import { DialInSummary } from '../../invite'; @@ -63,6 +64,20 @@ class DeepLinkingMobilePage extends Component<*, *> { t: PropTypes.func }; + /** + * Initializes a new {@code DeepLinkingMobilePage} instance. + * + * @param {Object} props - The read-only React {@code Component} props with + * which the new instance is to be initialized. + */ + constructor(props) { + super(props); + + // Bind event handlers so they are only bound once per instance. + this._onDownloadApp = this._onDownloadApp.bind(this); + this._onOpenApp = this._onOpenApp.bind(this); + } + /** * Initializes the text and URL of the `Start a conference` / `Join the * conversation` button which takes the user to the mobile app. @@ -75,6 +90,17 @@ class DeepLinkingMobilePage extends Component<*, *> { }); } + /** + * Implements the Component's componentDidMount method. + * + * @inheritdoc + */ + componentDidMount() { + sendAnalytics( + createDeepLinkingPageEvent( + 'displayed', 'DeepLinkingMobile', { isMobileBrowser: true })); + } + /** * Implements React's {@link Component#render()}. * @@ -110,14 +136,17 @@ class DeepLinkingMobilePage extends Component<*, *> { { app: NATIVE_APP_NAME }) }

- + + href = { this.state.joinURL } + onClick = { this._onOpenApp }> {/* */} @@ -131,6 +160,32 @@ class DeepLinkingMobilePage extends Component<*, *> { ); } + + _onDownloadApp: () => {}; + + /** + * Handles download app button clicks. + * + * @returns {void} + */ + _onDownloadApp() { + sendAnalytics( + createDeepLinkingPageEvent( + 'clicked', 'downloadAppButton', { isMobileBrowser: true })); + } + + _onOpenApp: () => {}; + + /** + * Handles open app button clicks. + * + * @returns {void} + */ + _onOpenApp() { + sendAnalytics( + createDeepLinkingPageEvent( + 'clicked', 'openAppButton', { isMobileBrowser: true })); + } } /** diff --git a/react/features/deep-linking/components/NoMobileApp.js b/react/features/deep-linking/components/NoMobileApp.js index f5e7f223a..eab689a22 100644 --- a/react/features/deep-linking/components/NoMobileApp.js +++ b/react/features/deep-linking/components/NoMobileApp.js @@ -2,6 +2,7 @@ import React, { Component } from 'react'; +import { createDeepLinkingPageEvent, sendAnalytics } from '../../analytics'; import { HideNotificationBarStyle } from '../../base/react'; declare var interfaceConfig: Object; @@ -12,6 +13,17 @@ declare var interfaceConfig: Object; * @class NoMobileApp */ export default class NoMobileApp extends Component<*> { + /** + * Implements the Component's componentDidMount method. + * + * @inheritdoc + */ + componentDidMount() { + sendAnalytics( + createDeepLinkingPageEvent( + 'displayed', 'noMobileApp', { isMobileBrowser: true })); + } + /** * Renders the component. * diff --git a/react/features/welcome/components/WelcomePage.web.js b/react/features/welcome/components/WelcomePage.web.js index a84576c91..716911b90 100644 --- a/react/features/welcome/components/WelcomePage.web.js +++ b/react/features/welcome/components/WelcomePage.web.js @@ -1,4 +1,4 @@ -/* global APP, config, interfaceConfig, JitsiMeetJS */ +/* global interfaceConfig */ import Button from '@atlaskit/button'; import { FieldTextStateless } from '@atlaskit/field-text'; @@ -6,9 +6,7 @@ import { AtlasKitThemeProvider } from '@atlaskit/theme'; import React from 'react'; import { connect } from 'react-redux'; -import { initAnalytics } from '../../analytics'; import { translate } from '../../base/i18n'; -import { isAnalyticsEnabled } from '../../base/lib-jitsi-meet'; import { HideNotificationBarStyle, Watermarks } from '../../base/react'; import { AbstractWelcomePage, _mapStateToProps } from './AbstractWelcomePage'; @@ -71,17 +69,6 @@ class WelcomePage extends AbstractWelcomePage { componentDidMount() { document.body.classList.add('welcome-page'); - // FIXME: This is not the best place for this logic. Ideally we should - // use features/base/lib-jitsi-meet#initLib() action for this use case. - // But currently lib-jitsi-meet#initLib()'s logic works for mobile only - // (on web it ends up with infinite loop over initLib). - JitsiMeetJS.init({ - enableAnalyticsLogging: isAnalyticsEnabled(APP.store), - ...config - }).then(() => { - initAnalytics(APP.store); - }); - if (this.state.generateRoomnames) { this._updateRoomname(); }