From e76a2b0d631a69785564061c088af43688f8e3fb Mon Sep 17 00:00:00 2001 From: SaurabhSharma-884 Date: Fri, 15 Dec 2023 16:05:31 +0530 Subject: [PATCH] [MI-3842]: Review fixes frontend phase 1 --- webapp/__mocks__/svg.js | 2 + webapp/jest.config.ts | 1 + webapp/src/App.tsx | 9 +- .../components/Dialog/Dialog.component.tsx | 57 ++++++------- webapp/src/components/Dialog/Dialog.test.tsx | 10 ++- .../Dialog/__snapshots__/Dialog.test.tsx.snap | 33 +++----- .../Snackbar/Snackbar.component.tsx | 28 +++---- .../enforceConnectedAccountModal.tsx | 14 ++-- webapp/src/containers/Rhs/Rhs.container.tsx | 3 +- .../Rhs/__snapshots__/Rhs.test.tsx.snap | 84 +------------------ .../ConnectAccount.container.tsx | 2 +- .../ConnectedAccount.container.tsx | 44 +++++----- .../ConnectedAccount.test.tsx | 9 +- .../ConnectedAccount.test.tsx.snap | 84 +------------------ .../LinkedChannels.container.tsx | 9 +- webapp/src/hooks/useAlert.ts | 6 +- .../src/hooks/useApiRequestCompletionState.ts | 26 +++--- webapp/src/hooks/useDialog.tsx | 32 ------- webapp/src/hooks/usePluginApi.ts | 20 +++-- webapp/src/index.tsx | 5 +- webapp/src/reducers/dialog/index.test.ts | 32 ------- webapp/src/reducers/dialog/index.ts | 37 -------- webapp/src/reducers/index.ts | 2 - webapp/src/selectors/index.tsx | 14 ++-- webapp/src/types/common/store.d.ts | 4 - webapp/src/types/mattermost-webapp/index.d.ts | 3 +- 26 files changed, 150 insertions(+), 420 deletions(-) create mode 100644 webapp/__mocks__/svg.js delete mode 100644 webapp/src/hooks/useDialog.tsx delete mode 100644 webapp/src/reducers/dialog/index.test.ts delete mode 100644 webapp/src/reducers/dialog/index.ts diff --git a/webapp/__mocks__/svg.js b/webapp/__mocks__/svg.js new file mode 100644 index 000000000..ffe2050a0 --- /dev/null +++ b/webapp/__mocks__/svg.js @@ -0,0 +1,2 @@ +export default 'SvgrURL'; +export const ReactComponent = 'div'; diff --git a/webapp/jest.config.ts b/webapp/jest.config.ts index 37de23099..119011a81 100644 --- a/webapp/jest.config.ts +++ b/webapp/jest.config.ts @@ -43,6 +43,7 @@ const config: Config = { // A map from regular expressions to module names or to arrays of module names that allow to stub out resources with a single module moduleNameMapper: { + '\\.svg$': '/__mocks__/svg.js', '^.+\\.(jpg|jpeg|png|gif|eot|otf|webp|svg|ttf|woff|woff2|mp4|webm|wav|mp3|m4a|aac|oga)$': 'identity-obj-proxy', '^.+\\.(css|less|scss)$': 'identity-obj-proxy', '^src/(.*)': '/src/$1', diff --git a/webapp/src/App.tsx b/webapp/src/App.tsx index 7c4a3fbda..790ba3e8a 100644 --- a/webapp/src/App.tsx +++ b/webapp/src/App.tsx @@ -1,4 +1,4 @@ -import React, {useEffect} from 'react'; +import React, {useEffect, useMemo} from 'react'; import {Action, Store} from 'redux'; import {useDispatch} from 'react-redux'; @@ -28,6 +28,7 @@ import 'styles/main.scss'; const App = ({registry, store}:{registry: PluginRegistry, store: Store>>}): JSX.Element => { const dispatch = useDispatch(); const {makeApiRequestWithCompletionStatus, getApiState} = usePluginApi(); + useEffect(() => { const linkedChannelsParams: SearchLinkedChannelParams = {page: defaultPage, per_page: defaultPerPage}; @@ -36,13 +37,13 @@ const App = ({registry, store}:{registry: PluginRegistry, store: Store getApiState(pluginApiServiceConfigs.needsConnect.apiServiceName), [getApiState]); useEffect(() => { dispatch(setIsRhsLoading(isLoading)); }, [isLoading]); - const {data: configData} = getApiState(pluginApiServiceConfigs.getConfig.apiServiceName); + const {data: configData} = useMemo(() => getApiState(pluginApiServiceConfigs.getConfig.apiServiceName), [getApiState]); useApiRequestCompletionState({ serviceName: pluginApiServiceConfigs.needsConnect.apiServiceName, @@ -98,4 +99,4 @@ const App = ({registry, store}:{registry: PluginRegistry, store: Store; }; -export default App; +export default App; \ No newline at end of file diff --git a/webapp/src/components/Dialog/Dialog.component.tsx b/webapp/src/components/Dialog/Dialog.component.tsx index f20b935f0..fff6e54e9 100644 --- a/webapp/src/components/Dialog/Dialog.component.tsx +++ b/webapp/src/components/Dialog/Dialog.component.tsx @@ -1,35 +1,30 @@ import React from 'react'; -import {useDispatch} from 'react-redux'; import {Dialog as MMDialog, LinearProgress, DialogProps} from '@brightscout/mattermost-ui-library'; -import usePluginApi from 'hooks/usePluginApi'; -import {getDialogState} from 'selectors'; -import {closeDialog} from 'reducers/dialog'; - -export const Dialog = ({onCloseHandler, onSubmitHandler}: Pick) => { - const dispatch = useDispatch(); - const {state} = usePluginApi(); - const {show, title, description, destructive, primaryButtonText, secondaryButtonText, isLoading} = getDialogState(state); - - const handleClose = () => dispatch(closeDialog()); - - return ( - { - handleClose(); - onCloseHandler(); - }} - onSubmitHandler={onSubmitHandler} - className='disconnect-dialog' - title={title} - > - {isLoading && } - - ); -}; +export const Dialog = ({ + show, + title, + destructive, + primaryButtonText, + secondaryButtonText, + onSubmitHandler, + onCloseHandler, + children, + isLoading, + className, +}: DialogProps & {isLoading?: boolean, children: React.ReactNode}) => ( + onCloseHandler()} + onSubmitHandler={onSubmitHandler} + className={className} + title={title} + > + {isLoading && } + {children} + +); diff --git a/webapp/src/components/Dialog/Dialog.test.tsx b/webapp/src/components/Dialog/Dialog.test.tsx index 392eff3ea..9e8521046 100644 --- a/webapp/src/components/Dialog/Dialog.test.tsx +++ b/webapp/src/components/Dialog/Dialog.test.tsx @@ -9,15 +9,20 @@ const onCloseHandler = jest.fn(); const onSubmitHandler = jest.fn(); const dialogProps = { + show: true, + destructive: true, + primaryButtonText: 'Try Again', + secondaryButtonText: 'Cancel', onCloseHandler, onSubmitHandler, + title: 'Unlink Channel', }; let tree: RenderResult; describe('Dialog component', () => { beforeEach(() => { - tree = render(); + tree = render({'Are you sure you want to disconnect your Microsoft Teams Account?'}); }); it('Should render correctly', () => { @@ -25,9 +30,8 @@ describe('Dialog component', () => { }); it('Should close the dialog on clicking close button', () => { - expect(tree.getAllByRole('button').length).toEqual(2); + userEvent.click(tree.getByText('Cancel')); - userEvent.click(tree.getAllByRole('button')[1]); expect(onCloseHandler).toHaveBeenCalledTimes(1); }); }); diff --git a/webapp/src/components/Dialog/__snapshots__/Dialog.test.tsx.snap b/webapp/src/components/Dialog/__snapshots__/Dialog.test.tsx.snap index 243386d60..e8f29029a 100644 --- a/webapp/src/components/Dialog/__snapshots__/Dialog.test.tsx.snap +++ b/webapp/src/components/Dialog/__snapshots__/Dialog.test.tsx.snap @@ -15,7 +15,7 @@ exports[`Dialog component Should render correctly 1`] = ` />
- -
-
-
-
-
+ Are you sure you want to disconnect your Microsoft Teams Account?
+ -
= { + error: 'bg-error', + default: 'bg-default', + success: 'bg-success', +}; + +const snackbarIconMap: Record = { + error: 'warning', + default: 'tick', + success: 'tick', +}; + export const Snackbar = () => { const dispatch = useDispatch(); const {state} = usePluginApi(); const timeId = useRef(0); const {isOpen, message, severity} = getSnackbarState(state); - const handleClose = () => dispatch(closeAlert()); + const handleClose = useCallback(() => dispatch(closeAlert()), []); useEffect(() => { if (isOpen) { @@ -34,18 +46,6 @@ export const Snackbar = () => { }; }, [isOpen]); - const snackbarColorMap: Record = { - error: 'bg-error', - default: 'bg-default', - success: 'bg-success', - }; - - const snackbarIconMap: Record = { - error: 'warning', - default: 'tick', - success: 'tick', - }; - return (
{ @@ -26,15 +26,15 @@ export default function EnforceConnectedAccountModal() { makeApiRequestWithCompletionStatus(Constants.pluginApiServiceConfigs.needsConnect.apiServiceName); }, []); - const {data: needsConnectData} = getApiState(Constants.pluginApiServiceConfigs.needsConnect.apiServiceName); - const {data: connectData} = getApiState(Constants.pluginApiServiceConfigs.connect.apiServiceName); + const {data: needsConnectData} = useMemo(() => getApiState(Constants.pluginApiServiceConfigs.needsConnect.apiServiceName), [getApiState]); + const {data: connectData} = useMemo(() => getApiState(Constants.pluginApiServiceConfigs.connect.apiServiceName), [getApiState]); useApiRequestCompletionState({ serviceName: Constants.pluginApiServiceConfigs.needsConnect.apiServiceName, handleSuccess: () => { if (needsConnectData) { const data = needsConnectData as NeedsConnectData; - if (!isInterval) { + if (!isInterval.current) { setOpen(data.needsConnect); setCanSkip(data.canSkip); } else if (!data.needsConnect) { @@ -62,12 +62,12 @@ export default function EnforceConnectedAccountModal() { useEffect(() => { let interval: any = 0; if (connecting) { - setIsInterval(true); + isInterval.current = true; interval = setInterval(checkConnected, 1000); } return () => { if (interval) { - setIsInterval(false); + isInterval.current = false; clearInterval(interval); } }; diff --git a/webapp/src/containers/Rhs/Rhs.container.tsx b/webapp/src/containers/Rhs/Rhs.container.tsx index c5b77bf91..41d762038 100644 --- a/webapp/src/containers/Rhs/Rhs.container.tsx +++ b/webapp/src/containers/Rhs/Rhs.container.tsx @@ -1,4 +1,4 @@ -import React, {useCallback, useMemo} from 'react'; +import React, {memo, useCallback, useMemo} from 'react'; import {Spinner} from '@brightscout/mattermost-ui-library'; @@ -14,7 +14,6 @@ import {ConnectAccount} from './views/ConnectAccount'; import {LinkedChannels} from './views/LinkedChannels'; import {ConnectedAccount} from './views/ConnectedAccount'; -// TODO: update component later export const Rhs = () => { const {state, getApiState} = usePluginApi(); const {connected} = getConnectedState(state); diff --git a/webapp/src/containers/Rhs/__snapshots__/Rhs.test.tsx.snap b/webapp/src/containers/Rhs/__snapshots__/Rhs.test.tsx.snap index cf9e7c2ae..8f9dc5d1d 100644 --- a/webapp/src/containers/Rhs/__snapshots__/Rhs.test.tsx.snap +++ b/webapp/src/containers/Rhs/__snapshots__/Rhs.test.tsx.snap @@ -3,13 +3,8 @@ exports[`RHS view Should render correctly 1`] = ` { "asFragment": [Function], - "baseElement": - -
- ); diff --git a/webapp/src/containers/Rhs/views/ConnectedAccount/ConnectedAccount.test.tsx b/webapp/src/containers/Rhs/views/ConnectedAccount/ConnectedAccount.test.tsx index 7da803f2a..aa0ed2d26 100644 --- a/webapp/src/containers/Rhs/views/ConnectedAccount/ConnectedAccount.test.tsx +++ b/webapp/src/containers/Rhs/views/ConnectedAccount/ConnectedAccount.test.tsx @@ -31,16 +31,9 @@ describe('Connected Account View', () => { it('Should display the name of connected user correctly', () => { const usernameContainer = tree.getByText('Connected as'); - const username = tree.getByText(getConnectedState(mockTestState as ReduxState).username); + const username = tree.getByText(getConnectedState(mockTestState['plugins-com.mattermost.msteams-sync'] as ReduxState).username); expect(usernameContainer).toContainElement(username); expect(username).toBeVisible(); }); - - it('Should dispatch action on clicking disconnect', async () => { - const disconnectButton = tree.getByText('Disconnect'); - - await userEvent.click(disconnectButton); - expect(mockDispatch).toHaveBeenCalledTimes(1); - }); }); diff --git a/webapp/src/containers/Rhs/views/ConnectedAccount/__snapshots__/ConnectedAccount.test.tsx.snap b/webapp/src/containers/Rhs/views/ConnectedAccount/__snapshots__/ConnectedAccount.test.tsx.snap index 3412d0def..9051bbd82 100644 --- a/webapp/src/containers/Rhs/views/ConnectedAccount/__snapshots__/ConnectedAccount.test.tsx.snap +++ b/webapp/src/containers/Rhs/views/ConnectedAccount/__snapshots__/ConnectedAccount.test.tsx.snap @@ -3,13 +3,8 @@ exports[`Connected Account View Should render correctly 1`] = ` { "asFragment": [Function], - "baseElement": - -
-