From c8f139cced2458ef42326d9ea0d88e4bee8a55ea Mon Sep 17 00:00:00 2001 From: Alf Eaton <75253002+aeaton-overleaf@users.noreply.github.com> Date: Fri, 5 Mar 2021 13:00:21 +0000 Subject: [PATCH] Merge pull request #3707 from overleaf/ae-refactor-word-count-modal Refactor "Word Count" modal GitOrigin-RevId: 00561b5b3f8f161238321c440ecde67cd42ece1c --- .../components/word-count-modal-content.js | 29 ++-- .../components/word-count-modal.js | 38 ++---- .../js/features/word-count-modal/utils/api.js | 10 ++ .../word-count-modal-content.stories.js | 45 ++++++ .../stories/word-count-modal.stories.js | 129 +++++++++--------- services/web/package.json | 1 + .../components/word-count-modal.test.js | 86 +++++++----- 7 files changed, 207 insertions(+), 131 deletions(-) create mode 100644 services/web/frontend/js/features/word-count-modal/utils/api.js create mode 100644 services/web/frontend/stories/word-count-modal-content.stories.js 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 7f7c36dd31..e3cf73c93c 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 @@ -3,12 +3,25 @@ import { Row, Col, Modal, Grid, Alert, Button } from 'react-bootstrap' import PropTypes from 'prop-types' import { useTranslation } from 'react-i18next' import Icon from '../../../shared/components/icon' +import AccessibleModal from '../../../shared/components/accessible-modal' -function WordCountModalContent({ data, error, handleHide, loading }) { +export default function WordCountModalContent({ + animation = true, + show, + data, + error, + handleHide, + loading +}) { const { t } = useTranslation() return ( - <> + {t('word_count')} @@ -16,7 +29,7 @@ function WordCountModalContent({ data, error, handleHide, loading }) { {loading && !error && (
-   {t('loading')}… +   {t('loading')}…
)} @@ -70,11 +83,13 @@ function WordCountModalContent({ data, error, handleHide, loading }) { - + ) } WordCountModalContent.propTypes = { + animation: PropTypes.bool, + show: PropTypes.bool.isRequired, handleHide: PropTypes.func.isRequired, loading: PropTypes.bool.isRequired, error: PropTypes.bool, @@ -86,9 +101,3 @@ WordCountModalContent.propTypes = { textWords: PropTypes.number }) } - -function Loading() { - return -} - -export default WordCountModalContent 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 dac9291de4..573ea3d284 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,7 +1,8 @@ import React, { useCallback, useEffect, useState } from 'react' -import { Modal } from 'react-bootstrap' import PropTypes from 'prop-types' +import AbortController from 'abort-controller' import WordCountModalContent from './word-count-modal-content' +import { fetchWordCount } from '../utils/api' function WordCountModal({ clsiServerId, handleHide, projectId, show }) { const [loading, setLoading] = useState(true) @@ -21,25 +22,17 @@ function WordCountModal({ clsiServerId, handleHide, projectId, show }) { const _abortController = new AbortController() setAbortController(_abortController) - let query = '' - if (clsiServerId) { - query = `?clsiserverid=${clsiServerId}` - } - - fetch(`/project/${projectId}/wordcount${query}`, { + fetchWordCount(projectId, clsiServerId, { signal: _abortController.signal }) - .then(async response => { - if (response.ok) { - const { texcount } = await response.json() - setData(texcount) - } else { + .then(data => { + setData(data.texcount) + }) + .catch(error => { + if (error.cause?.name !== 'AbortError') { setError(true) } }) - .catch(() => { - setError(true) - }) .finally(() => { setLoading(false) }) @@ -55,14 +48,13 @@ function WordCountModal({ clsiServerId, handleHide, projectId, show }) { }, [abortController, handleHide]) return ( - - - + ) } diff --git a/services/web/frontend/js/features/word-count-modal/utils/api.js b/services/web/frontend/js/features/word-count-modal/utils/api.js new file mode 100644 index 0000000000..54818f0338 --- /dev/null +++ b/services/web/frontend/js/features/word-count-modal/utils/api.js @@ -0,0 +1,10 @@ +import { getJSON } from '../../../infrastructure/fetch-json' + +export function fetchWordCount(projectId, clsiServerId, options) { + let query = '' + if (clsiServerId) { + query = `?clsiserverid=${clsiServerId}` + } + + return getJSON(`/project/${projectId}/wordcount${query}`, options) +} diff --git a/services/web/frontend/stories/word-count-modal-content.stories.js b/services/web/frontend/stories/word-count-modal-content.stories.js new file mode 100644 index 0000000000..8d4cfc5f8c --- /dev/null +++ b/services/web/frontend/stories/word-count-modal-content.stories.js @@ -0,0 +1,45 @@ +import React from 'react' + +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: 'Word Count Modal / 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 20b6781cdc..703a830477 100644 --- a/services/web/frontend/stories/word-count-modal.stories.js +++ b/services/web/frontend/stories/word-count-modal.stories.js @@ -1,77 +1,76 @@ import React from 'react' +import fetchMock from 'fetch-mock' +import PropTypes from 'prop-types' -import WordCountModalContent from '../js/features/word-count-modal/components/word-count-modal-content' +import WordCountModal from '../js/features/word-count-modal/components/word-count-modal' -// NOTE: WordCountModalContent is wrapped in modal classes, without modal behaviours +export const Interactive = ({ + mockResponse = 200, + mockResponseDelay = 500, + ...args +}) => { + fetchMock.restore().get( + 'express:/project/:projectId/wordcount', + () => { + switch (mockResponse) { + case 400: + return { status: 400, body: 'The project id is not valid' } -export const Loading = args => ( -
-
- -
-
-) -Loading.args = { - loading: true, - error: false + case 200: + return { + texcount: { + headers: 4, + mathDisplay: 40, + mathInline: 400, + textWords: 4000 + } + } + + default: + return mockResponse + } + }, + { delay: mockResponseDelay } + ) + + return } - -export const LoadingError = args => ( -
-
- -
-
-) -LoadingError.args = { - loading: false, - error: true -} - -export const Loaded = args => ( -
-
- -
-
-) -Loaded.args = { - loading: false, - error: false, - data: { - headers: 4, - mathDisplay: 40, - mathInline: 400, - textWords: 4000 - } -} - -export const Messages = args => ( -
-
- -
-
-) -Messages.args = { - loading: false, - error: false, - data: { - 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'), - headers: 4, - mathDisplay: 40, - mathInline: 400, - textWords: 4000 - } +Interactive.propTypes = { + mockResponse: PropTypes.number, + mockResponseDelay: PropTypes.number } export default { title: 'Word Count Modal', - component: WordCountModalContent, + component: WordCountModal, + args: { + clsiServerId: 'server-id', + projectId: 'project-id', + show: true + }, argTypes: { - handleHide: { action: 'handleHide' } + 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/package.json b/services/web/package.json index 56caa3ddb7..81ee45a749 100644 --- a/services/web/package.json +++ b/services/web/package.json @@ -165,6 +165,7 @@ "@storybook/react": "^6.1.10", "@testing-library/dom": "^7.29.4", "@testing-library/react": "^11.2.3", + "abort-controller": "^3.0.0", "acorn": "^7.1.1", "acorn-walk": "^7.1.1", "angular-mocks": "~1.8.0", 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 e0b8095b1e..1a658b8db5 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,29 +1,43 @@ import React from 'react' -import { render, screen } from '@testing-library/react' -import WordCountModalContent from '../../../../../frontend/js/features/word-count-modal/components/word-count-modal-content' +import { render, screen, cleanup } from '@testing-library/react' +import WordCountModal from '../../../../../frontend/js/features/word-count-modal/components/word-count-modal' import { expect } from 'chai' +import sinon from 'sinon' +import fetchMock from 'fetch-mock' -const handleHide = () => { - // closed -} +describe('', function() { + afterEach(function() { + fetchMock.reset() + cleanup() + }) + + const modalProps = { + projectId: 'project-1', + clsiServerId: 'clsi-server-1', + show: true, + handleHide: sinon.stub() + } -describe('', function() { it('renders the translated modal title', async function() { - render() + render() await screen.findByText('Word Count') - - expect(screen.queryByText(/Loading/)).to.not.exist }) it('renders a loading message when loading', async function() { - render() + fetchMock.get('express:/project/:projectId/wordcount', () => { + return { status: 200, body: { texcount: {} } } + }) - await screen.findByText('Loading') + render() + + await screen.findByText('Loading…') }) it('renders an error message and hides loading message on error', async function() { - render() + fetchMock.get('express:/project/:projectId/wordcount', 500) + + render() await screen.findByText('Sorry, something went wrong') @@ -31,32 +45,38 @@ describe('', function() { }) it('displays messages', async function() { - render( - - ) + fetchMock.get('express:/project/:projectId/wordcount', () => { + return { + status: 200, + body: { + texcount: { + messages: 'This is a test' + } + } + } + }) + + render() await screen.findByText('This is a test') }) it('displays counts data', async function() { - render( - - ) + fetchMock.get('express:/project/:projectId/wordcount', () => { + return { + status: 200, + body: { + texcount: { + textWords: 100, + mathDisplay: 200, + mathInline: 300, + headers: 400 + } + } + } + }) + + render() await screen.findByText((content, element) => element.textContent.trim().match(/^Total Words\s*:\s*100$/)