diff --git a/spec/javascripts/jsx/external_apps/routerSpec.js b/spec/javascripts/jsx/external_apps/routerSpec.js deleted file mode 100644 index 63e6564dbc74f..0000000000000 --- a/spec/javascripts/jsx/external_apps/routerSpec.js +++ /dev/null @@ -1,53 +0,0 @@ -/* - * Copyright (C) 2016 - present Instructure, Inc. - * - * This file is part of Canvas. - * - * Canvas is free software: you can redistribute it and/or modify it under - * the terms of the GNU Affero General Public License as published by the Free - * Software Foundation, version 3 of the License. - * - * Canvas is distributed in the hope that it will be useful, but WITHOUT ANY - * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR - * A PARTICULAR PURPOSE. See the GNU Affero General Public License for more - * details. - * - * You should have received a copy of the GNU Affero General Public License along - * with this program. If not, see . - */ - -import regularizePathname from 'ui/features/external_apps/react/lib/regularizePathname.js' - -QUnit.module('External Apps Client-side Router', { - before() { - window.ENV = window.ENV || {} - window.ENV.TESTING_PATH = '/settings/something' - } -}) - -test('regularizePathname removes trailing slash', () => { - const fakeCtx = { - pathname: '/app/something/else/' - } - - // No op for next(). - const fakeNext = () => {} - - regularizePathname(fakeCtx, fakeNext) - - equal(fakeCtx.pathname, '/app/something/else', 'trailing slash is gone') -}) - -test('regularizePathname removes url hash fragment', () => { - const fakeCtx = { - hash: 'blah-ha-ba', - pathname: '/app/something/else/#blah-ha-ba' - } - - // No op for next(). - const fakeNext = () => {} - - regularizePathname(fakeCtx, fakeNext) - - equal(fakeCtx.pathname, '/app/something/else', 'url hash fragment is gone') -}) diff --git a/ui/features/external_apps/react/components/AppList.js b/ui/features/external_apps/react/components/AppList.js index 56523e7d567a2..9824c45593c2b 100644 --- a/ui/features/external_apps/react/components/AppList.js +++ b/ui/features/external_apps/react/components/AppList.js @@ -18,6 +18,7 @@ import I18n from 'i18n!external_tools' import React from 'react' +import PropTypes from 'prop-types' import store from '../lib/AppCenterStore' import extStore from '../lib/ExternalAppsStore' import AppTile from './AppTile' @@ -27,6 +28,10 @@ import ManageAppListButton from './ManageAppListButton' import splitAssetString from '@canvas/util/splitAssetString' export default class AppList extends React.Component { + static propTypes = { + baseUrl: PropTypes.string.isRequired + } + state = store.getState() onChange = () => { @@ -67,7 +72,7 @@ export default class AppList extends React.Component { } else { return store .filteredApps() - .map(app => ) + .map(app => ) } } @@ -79,7 +84,7 @@ export default class AppList extends React.Component {
{this.manageAppListButton()} {I18n.t('View App Configurations')} diff --git a/ui/features/external_apps/react/components/AppTile.js b/ui/features/external_apps/react/components/AppTile.js index 83a605a0c6b2d..67da6f0e4878f 100644 --- a/ui/features/external_apps/react/components/AppTile.js +++ b/ui/features/external_apps/react/components/AppTile.js @@ -19,11 +19,17 @@ import I18n from 'i18n!external_tools' import $ from 'jquery' import React from 'react' +import PropTypes from 'prop-types' import page from 'page' export default class extends React.Component { static displayName = 'AppTile' + static propTypes = { + app: PropTypes.object, + baseUrl: PropTypes.string.isRequired + } + state = { isHidingDetails: true } @@ -53,7 +59,7 @@ export default class extends React.Component { handleClick = e => { e.preventDefault() - page(`${this.props.pathname}/app/${this.props.app.short_name}`) + page(`${this.props.baseUrl}/app/${this.props.app.short_name}`) } installedRibbon = () => { diff --git a/ui/features/external_apps/react/components/__tests__/AppList.test.js b/ui/features/external_apps/react/components/__tests__/AppList.test.js index d6f89c573ab91..ed7286b5e5576 100644 --- a/ui/features/external_apps/react/components/__tests__/AppList.test.js +++ b/ui/features/external_apps/react/components/__tests__/AppList.test.js @@ -127,35 +127,47 @@ describe('AppList', () => { $.ajax = originalAJAX }) + function renderAppList() { + return render() + } + it('renders loading indicator', () => { store.setState({isLoading: true}) - const {getByTestId} = render() + const {getByTestId} = renderAppList() expect(getByTestId(/Spinner/i)).toBeVisible() }) it('does not apply focus to filter text input on render', () => { - const {getByText} = render() + const {getByText} = renderAppList() expect(getByText('Filter by name')).not.toHaveFocus() }) it('renders manage app list button if context type is account', () => { - const {getByText} = render() + const stuff = renderAppList() + const {getByText} = stuff expect(getByText('Manage App List')).toBeVisible() }) it('does not render manage app list button if context type is not account', () => { window.ENV = {context_asset_string: 'course_1'} - const {queryByText} = render() + const {queryByText} = renderAppList() expect(queryByText('Manage App List')).not.toBeInTheDocument() }) it('renders view app configurations link', () => { - const {getByText} = render() + const {getByText} = renderAppList() expect(getByText('View App Configurations')).toBeVisible() }) + it('uses baseUrl for the app configurations link', () => { + const {getByText} = renderAppList() + expect(getByText('View App Configurations').closest('a').getAttribute('href')).toEqual( + '/the/base/url/configurations' + ) + }) + it('renders app filters', () => { - const {getByText} = render() + const {getByText} = renderAppList() expect(getByText('All')).toBeVisible() expect(getByText('Not Installed')).toBeVisible() expect(getByText('Installed')).toBeVisible() diff --git a/ui/features/external_apps/react/lib/regularizePathname.js b/ui/features/external_apps/react/lib/regularizePathname.js deleted file mode 100644 index 8695a516a9aac..0000000000000 --- a/ui/features/external_apps/react/lib/regularizePathname.js +++ /dev/null @@ -1,29 +0,0 @@ -/* - * Copyright (C) 2016 - present Instructure, Inc. - * - * This file is part of Canvas. - * - * Canvas is free software: you can redistribute it and/or modify it under - * the terms of the GNU Affero General Public License as published by the Free - * Software Foundation, version 3 of the License. - * - * Canvas is distributed in the hope that it will be useful, but WITHOUT ANY - * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR - * A PARTICULAR PURPOSE. See the GNU Affero General Public License for more - * details. - * - * You should have received a copy of the GNU Affero General Public License along - * with this program. If not, see . - */ - -/** - * This makes the pathname always leave off a potential trailing - * slash. - */ -const regularizePathname = (ctx, next) => { - ctx.originalPathname = ctx.pathname - ctx.pathname = ctx.pathname.replace('#' + ctx.hash, '').replace(/\/$/, '') - next() -} - -export default regularizePathname diff --git a/ui/features/external_apps/react/router.js b/ui/features/external_apps/react/router.js index 09483def23fe6..170f40b41c7da 100644 --- a/ui/features/external_apps/react/router.js +++ b/ui/features/external_apps/react/router.js @@ -24,7 +24,6 @@ import AppList from './components/AppList' import AppDetails from './components/AppDetails' import Configurations from './components/Configurations' import AppCenterStore from './lib/AppCenterStore' -import regularizePathname from './lib/regularizePathname' const currentPath = window.location.pathname const re = /(.*\/settings|.*\/details)/ @@ -32,46 +31,39 @@ const matches = re.exec(currentPath) const baseUrl = matches[0] let targetNodeToRenderIn = null -let alreadyRendered = false /** * Route Handlers */ -const renderAppList = ctx => { +const renderAppList = _ctx => { if (!window.ENV.APP_CENTER.enabled) { page.redirect('/configurations') } else { ReactDOM.render( - + , targetNodeToRenderIn ) - alreadyRendered = true } } const renderAppDetails = ctx => { ReactDOM.render( - + , targetNodeToRenderIn ) } -const renderConfigurations = ctx => { +const renderConfigurations = _ctx => { // router.start is only called when loading the Apps tab // so we don't want to try anything here that hasn't happened. if (targetNodeToRenderIn) { ReactDOM.render( - + , targetNodeToRenderIn ) @@ -82,7 +74,6 @@ const renderConfigurations = ctx => { * Route Configuration */ page.base(baseUrl) -page('*', regularizePathname) page('/', renderAppList) page('/app/:shortName', renderAppDetails) page('/configurations', renderConfigurations) @@ -95,6 +86,5 @@ export default { stop() { // we may not be the only thing using page on this page. page.stop() - }, - regularizePathname + } }