From 1d55af6e75279ce2707ea5a8ae7ccc4915a60322 Mon Sep 17 00:00:00 2001 From: Alf Eaton Date: Tue, 14 Sep 2021 09:54:08 +0100 Subject: [PATCH] Refactor WordCountModalController (#4747) GitOrigin-RevId: d32d84a96743cd104f7d5fcd6ec66fc2c0b61c45 --- .../app/views/project/editor/left-menu.pug | 2 - .../components/word-count-modal-content.js | 39 ++----- .../components/word-count-modal.js | 58 +++------- .../word-count-modal-controller.js | 42 +++---- .../word-count-modal/hooks/use-word-count.js | 26 +++++ .../word-count-modal-content.stories.js | 43 ------- .../stories/word-count-modal.stories.js | 105 ++++++++---------- .../components/word-count-modal.test.js | 45 ++++++-- .../frontend/helpers/render-with-context.js | 3 +- 9 files changed, 157 insertions(+), 206 deletions(-) create mode 100644 services/web/frontend/js/features/word-count-modal/hooks/use-word-count.js delete mode 100644 services/web/frontend/stories/word-count-modal-content.stories.js diff --git a/services/web/app/views/project/editor/left-menu.pug b/services/web/app/views/project/editor/left-menu.pug index bb42f5ce9c..3ba85746ac 100644 --- a/services/web/app/views/project/editor/left-menu.pug +++ b/services/web/app/views/project/editor/left-menu.pug @@ -60,9 +60,7 @@ aside#left-menu.full-size( span.link-disabled    #{translate("word_count")} word-count-modal( - clsi-server-id="clsiServerId" handle-hide="handleHide" - project-id="projectId" show="show" ) diff --git a/services/web/frontend/js/features/word-count-modal/components/word-count-modal-content.js b/services/web/frontend/js/features/word-count-modal/components/word-count-modal-content.js index 3441c7c311..f5fe891173 100644 --- a/services/web/frontend/js/features/word-count-modal/components/word-count-modal-content.js +++ b/services/web/frontend/js/features/word-count-modal/components/word-count-modal-content.js @@ -1,26 +1,20 @@ -import { Row, Col, Modal, Grid, Alert, Button } from 'react-bootstrap' import PropTypes from 'prop-types' import { useTranslation } from 'react-i18next' +import { Alert, Button, Modal, Row, Col, Grid } from 'react-bootstrap' +import { useIdeContext } from '../../../shared/context/ide-context' +import { useProjectContext } from '../../../shared/context/project-context' +import { useWordCount } from '../hooks/use-word-count' import Icon from '../../../shared/components/icon' -import AccessibleModal from '../../../shared/components/accessible-modal' -export default function WordCountModalContent({ - animation = true, - show, - data, - error, - handleHide, - loading, -}) { +// NOTE: this component is only mounted when the modal is open +export default function WordCountModalContent({ handleHide }) { + const { _id: projectId } = useProjectContext() + const { clsiServerId } = useIdeContext() const { t } = useTranslation() + const { data, error, loading } = useWordCount(projectId, clsiServerId) return ( - + <> {t('word_count')} @@ -82,21 +76,10 @@ export default function WordCountModalContent({ - + ) } WordCountModalContent.propTypes = { - animation: PropTypes.bool, - show: PropTypes.bool.isRequired, handleHide: PropTypes.func.isRequired, - loading: PropTypes.bool.isRequired, - error: PropTypes.bool, - data: PropTypes.shape({ - messages: PropTypes.string, - headers: PropTypes.number, - mathDisplay: PropTypes.number, - mathInline: PropTypes.number, - textWords: PropTypes.number, - }), } diff --git a/services/web/frontend/js/features/word-count-modal/components/word-count-modal.js b/services/web/frontend/js/features/word-count-modal/components/word-count-modal.js index b3f636afd5..daed9eb7a2 100644 --- a/services/web/frontend/js/features/word-count-modal/components/word-count-modal.js +++ b/services/web/frontend/js/features/word-count-modal/components/word-count-modal.js @@ -1,52 +1,28 @@ -import { useEffect, useState } from 'react' +import React from 'react' import PropTypes from 'prop-types' import WordCountModalContent from './word-count-modal-content' -import { fetchWordCount } from '../utils/api' - -function WordCountModal({ clsiServerId, handleHide, projectId, show }) { - const [loading, setLoading] = useState(true) - const [error, setError] = useState(false) - const [data, setData] = useState() - - useEffect(() => { - if (!show) { - return - } - - setData(undefined) - setError(false) - setLoading(true) - - fetchWordCount(projectId, clsiServerId) - .then(data => { - setData(data.texcount) - }) - .catch(error => { - if (error.cause?.name !== 'AbortError') { - setError(true) - } - }) - .finally(() => { - setLoading(false) - }) - }, [show, projectId, clsiServerId]) +import AccessibleModal from '../../../shared/components/accessible-modal' +import withErrorBoundary from '../../../infrastructure/error-boundary' +const WordCountModal = React.memo(function WordCountModal({ + show, + handleHide, +}) { return ( - + onHide={handleHide} + id="clone-project-modal" + > + + ) -} +}) WordCountModal.propTypes = { - clsiServerId: PropTypes.string, + show: PropTypes.bool, handleHide: PropTypes.func.isRequired, - projectId: PropTypes.string.isRequired, - show: PropTypes.bool.isRequired, } -export default WordCountModal +export default withErrorBoundary(WordCountModal) diff --git a/services/web/frontend/js/features/word-count-modal/controllers/word-count-modal-controller.js b/services/web/frontend/js/features/word-count-modal/controllers/word-count-modal-controller.js index c1820ec5ce..84fcfd5dcd 100644 --- a/services/web/frontend/js/features/word-count-modal/controllers/word-count-modal-controller.js +++ b/services/web/frontend/js/features/word-count-modal/controllers/word-count-modal-controller.js @@ -1,28 +1,28 @@ import App from '../../../base' import { react2angular } from 'react2angular' - import WordCountModal from '../components/word-count-modal' +import { rootContext } from '../../../shared/context/root-context' -App.component('wordCountModal', react2angular(WordCountModal)) +export default App.controller('WordCountModalController', function ($scope) { + $scope.show = false -export default App.controller( - 'WordCountModalController', - function ($scope, ide) { - $scope.show = false - $scope.projectId = ide.project_id - - $scope.handleHide = () => { - $scope.$applyAsync(() => { - $scope.show = false - }) - } - - $scope.openWordCountModal = () => { - $scope.$applyAsync(() => { - $scope.clsiServerId = ide.clsiServerId - $scope.projectId = ide.project_id - $scope.show = true - }) - } + $scope.handleHide = () => { + $scope.$applyAsync(() => { + $scope.show = false + }) } + + $scope.openWordCountModal = () => { + $scope.$applyAsync(() => { + $scope.show = true + }) + } +}) + +App.component( + 'wordCountModal', + react2angular( + rootContext.use(WordCountModal), + Object.keys(WordCountModal.propTypes) + ) ) diff --git a/services/web/frontend/js/features/word-count-modal/hooks/use-word-count.js b/services/web/frontend/js/features/word-count-modal/hooks/use-word-count.js new file mode 100644 index 0000000000..51d6cba8f4 --- /dev/null +++ b/services/web/frontend/js/features/word-count-modal/hooks/use-word-count.js @@ -0,0 +1,26 @@ +import useAbortController from '../../../shared/hooks/use-abort-controller' +import { fetchWordCount } from '../utils/api' +import { useEffect, useState } from 'react' + +export function useWordCount(projectId, clsiServerId) { + const [loading, setLoading] = useState(true) + const [error, setError] = useState(false) + const [data, setData] = useState() + + const { signal } = useAbortController() + + useEffect(() => { + fetchWordCount(projectId, clsiServerId, { signal }) + .then(data => { + setData(data.texcount) + }) + .catch(() => { + setError(true) + }) + .finally(() => { + setLoading(false) + }) + }, [signal, clsiServerId, projectId]) + + return { data, error, loading } +} diff --git a/services/web/frontend/stories/word-count-modal-content.stories.js b/services/web/frontend/stories/word-count-modal-content.stories.js deleted file mode 100644 index 6170ffe85c..0000000000 --- a/services/web/frontend/stories/word-count-modal-content.stories.js +++ /dev/null @@ -1,43 +0,0 @@ -import WordCountModalContent from '../js/features/word-count-modal/components/word-count-modal-content' - -export const Basic = args => { - const data = { - headers: 4, - mathDisplay: 40, - mathInline: 400, - textWords: 4000, - } - - return -} - -export const Loading = args => { - return -} - -export const LoadingError = args => { - return -} - -export const Messages = args => { - const messages = [ - 'Lorem ipsum dolor sit amet.', - 'Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.', - ].join('\n') - - return -} - -export default { - title: 'Modals / Word Count / Content', - component: WordCountModalContent, - args: { - animation: false, - show: true, - error: false, - loading: false, - }, - argTypes: { - handleHide: { action: 'hide' }, - }, -} diff --git a/services/web/frontend/stories/word-count-modal.stories.js b/services/web/frontend/stories/word-count-modal.stories.js index 982b52421b..c48c8cbc72 100644 --- a/services/web/frontend/stories/word-count-modal.stories.js +++ b/services/web/frontend/stories/word-count-modal.stories.js @@ -1,77 +1,64 @@ -import PropTypes from 'prop-types' - -import WordCountModal from '../js/features/word-count-modal/components/word-count-modal' import useFetchMock from './hooks/use-fetch-mock' +import { withContextRoot } from './utils/with-context-root' +import WordCountModal from '../js/features/word-count-modal/components/word-count-modal' -export const Interactive = ({ - mockResponse = 200, - mockResponseDelay = 500, - ...args -}) => { +const counts = { + headers: 4, + mathDisplay: 40, + mathInline: 400, + textWords: 4000, +} + +const messages = [ + 'Lorem ipsum dolor sit amet.', + 'Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.', +].join('\n') + +const project = { + _id: 'project-id', + name: 'A Project', +} + +export const WordCount = args => { useFetchMock(fetchMock => { fetchMock.get( 'express:/project/:projectId/wordcount', - () => { - switch (mockResponse) { - case 400: - return { status: 400, body: 'The project id is not valid' } - - case 200: - return { - texcount: { - headers: 4, - mathDisplay: 40, - mathInline: 400, - textWords: 4000, - }, - } - - default: - return mockResponse - } - }, - { delay: mockResponseDelay } + { status: 200, body: { texcount: counts } }, + { delay: 500 } ) }) - return + return withContextRoot(, { project }) } -Interactive.propTypes = { - mockResponse: PropTypes.number, - mockResponseDelay: PropTypes.number, + +export const WordCountWithMessages = args => { + useFetchMock(fetchMock => { + fetchMock.get( + 'express:/project/:projectId/wordcount', + { status: 200, body: { texcount: { ...counts, messages } } }, + { delay: 500 } + ) + }) + + return withContextRoot(, { project }) +} + +export const ErrorResponse = args => { + useFetchMock(fetchMock => { + fetchMock.get( + 'express:/project/:projectId/wordcount', + { status: 500 }, + { delay: 500 } + ) + }) + + return withContextRoot(, { project }) } export default { title: 'Modals / Word Count', component: WordCountModal, args: { - clsiServerId: 'server-id', - projectId: 'project-id', show: true, }, - argTypes: { - handleHide: { action: 'handleHide' }, - mockResponse: { - name: 'Mock Response Status', - type: { name: 'number', required: false }, - description: 'The status code that should be returned by the mock server', - defaultValue: 200, - control: { - type: 'radio', - options: [200, 500, 400], - }, - }, - mockResponseDelay: { - name: 'Mock Response Delay', - type: { name: 'number', required: false }, - description: 'The delay before returning a response from the mock server', - defaultValue: 500, - control: { - type: 'range', - min: 0, - max: 2500, - step: 250, - }, - }, - }, } diff --git a/services/web/test/frontend/features/word-count-modal/components/word-count-modal.test.js b/services/web/test/frontend/features/word-count-modal/components/word-count-modal.test.js index c34f6933f2..0a2c7df641 100644 --- a/services/web/test/frontend/features/word-count-modal/components/word-count-modal.test.js +++ b/services/web/test/frontend/features/word-count-modal/components/word-count-modal.test.js @@ -1,24 +1,27 @@ -import { render, screen, cleanup } from '@testing-library/react' -import WordCountModal from '../../../../../frontend/js/features/word-count-modal/components/word-count-modal' +import { screen } from '@testing-library/react' import { expect } from 'chai' import sinon from 'sinon' import fetchMock from 'fetch-mock' +import { renderWithEditorContext } from '../../../helpers/render-with-context' +import WordCountModal from '../../../../../frontend/js/features/word-count-modal/components/word-count-modal' describe('', function () { afterEach(function () { fetchMock.reset() - cleanup() }) - const modalProps = { + const contextProps = { projectId: 'project-1', clsiServerId: 'clsi-server-1', - show: true, - handleHide: sinon.stub(), } it('renders the translated modal title', async function () { - render() + const handleHide = sinon.stub() + + renderWithEditorContext( + , + contextProps + ) await screen.findByText('Word Count') }) @@ -28,7 +31,12 @@ describe('', function () { return { status: 200, body: { texcount: { messages: 'This is a test' } } } }) - render() + const handleHide = sinon.stub() + + renderWithEditorContext( + , + contextProps + ) await screen.findByText('Loading…') @@ -38,7 +46,12 @@ describe('', function () { it('renders an error message and hides loading message on error', async function () { fetchMock.get('express:/project/:projectId/wordcount', 500) - render() + const handleHide = sinon.stub() + + renderWithEditorContext( + , + contextProps + ) await screen.findByText('Sorry, something went wrong') @@ -57,7 +70,12 @@ describe('', function () { } }) - render() + const handleHide = sinon.stub() + + renderWithEditorContext( + , + contextProps + ) await screen.findByText('This is a test') }) @@ -77,7 +95,12 @@ describe('', function () { } }) - render() + const handleHide = sinon.stub() + + renderWithEditorContext( + , + contextProps + ) await screen.findByText((content, element) => element.textContent.trim().match(/^Total Words\s*:\s*100$/) diff --git a/services/web/test/frontend/helpers/render-with-context.js b/services/web/test/frontend/helpers/render-with-context.js index a98ec7cb93..f4ab9b71cf 100644 --- a/services/web/test/frontend/helpers/render-with-context.js +++ b/services/web/test/frontend/helpers/render-with-context.js @@ -20,6 +20,7 @@ export function EditorProviders({ removeListener: sinon.stub(), }, isRestrictedTokenMember = false, + clsiServerId = '1234', scope, children, }) { @@ -51,7 +52,7 @@ export function EditorProviders({ ...scope, } - window._ide = { $scope, socket } + window._ide = { $scope, socket, clsiServerId } return (