From 57ba702dda19481d8bf86b6600629758a04ff450 Mon Sep 17 00:00:00 2001 From: Ilya Daynatovich Date: Wed, 18 Jan 2017 14:21:30 +0200 Subject: [PATCH] Clean up routing logic --- react/features/app/actions.js | 57 ++++++++----------- react/features/app/functions.web.js | 30 +++++++--- .../unsupported-browser/components/Landing.js | 42 +++++++++++--- react/features/unsupported-browser/index.js | 2 - react/features/unsupported-browser/route.js | 8 --- 5 files changed, 81 insertions(+), 58 deletions(-) delete mode 100644 react/features/unsupported-browser/route.js diff --git a/react/features/app/actions.js b/react/features/app/actions.js index 2bd008916..538e79abc 100644 --- a/react/features/app/actions.js +++ b/react/features/app/actions.js @@ -1,16 +1,24 @@ import { setRoom } from '../base/conference'; import { getDomain, setDomain } from '../base/connection'; import { loadConfig, setConfig } from '../base/lib-jitsi-meet'; -import { Platform } from '../base/react'; import { APP_WILL_MOUNT, APP_WILL_UNMOUNT } from './actionTypes'; import { _getRoomAndDomainFromUrlString, _getRouteToRender, + areRoutesEqual, init } from './functions'; import './reducer'; +/** + * Variable saving current route of the app. Later it will be extracted as + * a property of the class with Router specific logic. + * + * @type {Object} + */ +let currentRoute = {}; + /** * Temporary solution. Should dispatch actions related to initial settings of * the app like setting log levels, reading the config parameters from query @@ -42,9 +50,7 @@ export function appNavigate(urlOrRoom) { // current conference and start a new one with the new room name or // domain. - if (room === 'mobile-app') { - return; - } else if (typeof domain === 'undefined' || oldDomain === domain) { + if (typeof domain === 'undefined' || oldDomain === domain) { // If both domain and room vars became undefined, that means we're // actually dealing with just room name and not with URL. dispatch( @@ -63,7 +69,15 @@ export function appNavigate(urlOrRoom) { loadConfig(`https://${domain}`) .then( config => configLoaded(/* err */ undefined, config), - err => configLoaded(err, /* config */ undefined)); + err => configLoaded(err, /* config */ undefined)) + .then(() => { + const link = typeof room === 'undefined' + && typeof domain === 'undefined' + ? urlOrRoom + : room; + + dispatch(_setRoomAndNavigate(link)); + }); } /** @@ -87,9 +101,6 @@ export function appNavigate(urlOrRoom) { return; } - // We set room name only here to prevent race conditions on app - // start to not make app re-render conference page for two times. - dispatch(setRoom(room)); dispatch(setConfig(config)); } }; @@ -127,20 +138,6 @@ export function appWillUnmount(app) { }; } -/** - * Navigates to route corresponding to current room name. - * - * @param {Object} state - Redux state. - * @private - * @returns {void} - */ -function _navigate(state) { - const app = state['features/app'].app; - const routeToRender = _getRouteToRender(state); - - app._navigate(routeToRender); -} - /** * Sets room and navigates to new route if needed. * @@ -150,21 +147,15 @@ function _navigate(state) { */ function _setRoomAndNavigate(newRoom) { return (dispatch, getState) => { - const oldRoom = getState()['features/base/conference'].room; - dispatch(setRoom(newRoom)); const state = getState(); - const { room } = state['features/base/conference']; - const { landingIsShown } = state['features/unsupported-browser']; + const { app } = state['features/app']; + const newRoute = _getRouteToRender(state); - // If the user agent is a mobile browser and landing hasn't been shown - // yet, we should recheck which component to render. - const OS = Platform.OS; - - if (((OS === 'android' || OS === 'ios') && !landingIsShown) - || room !== oldRoom) { - _navigate(state); + if (!areRoutesEqual(newRoute, currentRoute)) { + currentRoute = newRoute; + app._navigate(newRoute); } }; } diff --git a/react/features/app/functions.web.js b/react/features/app/functions.web.js index 0bb4ed5b8..e0ed703b0 100644 --- a/react/features/app/functions.web.js +++ b/react/features/app/functions.web.js @@ -35,17 +35,33 @@ export function _getRouteToRender(stateOrGetState) { // If landing was shown, there is no need to show it again. const { landingIsShown } = state['features/unsupported-browser']; - let component; + const { room } = state['features/base/conference']; + const component = isRoomValid(room) ? Conference : WelcomePage; + + // We're using spread operator here to create copy of the route registered + // in registry. If we overwrite some of its properties (like 'component') + // they will stay unchanged in the registry. + const route = { ...RouteRegistry.getRouteByComponent(component) }; if ((OS === 'android' || OS === 'ios') && !landingIsShown) { - component = Landing; - } else { - const { room } = state['features/base/conference']; - - component = isRoomValid(room) ? Conference : WelcomePage; + route.component = Landing; } - return RouteRegistry.getRouteByComponent(component); + return route; +} + +/** + * Method checking whether route objects are equal by value. Returns true if + * and only if key values of the first object are equal to key values of + * the second one. + * + * @param {Object} newRoute - New route object to be compared. + * @param {Object} oldRoute - Old route object to be compared. + * @returns {boolean} + */ +export function areRoutesEqual(newRoute, oldRoute) { + return Object.keys(newRoute) + .every(key => newRoute[key] === oldRoute[key]); } /** diff --git a/react/features/unsupported-browser/components/Landing.js b/react/features/unsupported-browser/components/Landing.js index fe1aa2afc..ed8573366 100644 --- a/react/features/unsupported-browser/components/Landing.js +++ b/react/features/unsupported-browser/components/Landing.js @@ -1,9 +1,9 @@ import React, { Component } from 'react'; import { connect } from 'react-redux'; -import { Link } from 'react-router'; import { Platform } from '../../base/react'; +import { appNavigate } from '../../app'; import { landingIsShown } from '../actions'; /** @@ -21,6 +21,20 @@ const URLS = { * @class Landing */ class Landing extends Component { + + /** + * Constructor of Landing component. + * + * @param {Object} props - The read-only React Component props with which + * the new instance is to be initialized. + */ + constructor(props) { + super(props); + + // Bind methods + this._onClickJoin = this._onClickJoin.bind(this); + } + /** * Landing component's property types. * @@ -48,7 +62,7 @@ class Landing extends Component { componentWillMount() { const { room } = this.props; let btnText; - let link = '/'; + let link = ''; if (room) { btnText = 'Join the conversation'; @@ -63,13 +77,25 @@ class Landing extends Component { }); } + /** + * Navigates to the next state of the app. + * + * @returns {void} + * @private + */ + _onClickJoin() { + const { link } = this.state; + + this.props.dispatch(appNavigate(link)); + } + /** * Renders landing component. * * @returns {ReactElement} */ render() { - const { btnText, link } = this.state; + const { btnText } = this.state; const primaryButtonClasses = 'landing__button landing__button_primary'; return ( @@ -92,11 +118,11 @@ class Landing extends Component {
then

- - - + ); diff --git a/react/features/unsupported-browser/index.js b/react/features/unsupported-browser/index.js index a0296874a..3c46ed49d 100644 --- a/react/features/unsupported-browser/index.js +++ b/react/features/unsupported-browser/index.js @@ -1,4 +1,2 @@ -import './route'; - export * from './actions'; export * from './components'; diff --git a/react/features/unsupported-browser/route.js b/react/features/unsupported-browser/route.js deleted file mode 100644 index e38da3a02..000000000 --- a/react/features/unsupported-browser/route.js +++ /dev/null @@ -1,8 +0,0 @@ -import { RouteRegistry } from '../base/navigator'; - -import { Landing } from './components'; - -RouteRegistry.register({ - component: Landing, - path: '/mobile-app' -});