From 2bf71cce1062d1bb624c80267ef2791389693c86 Mon Sep 17 00:00:00 2001 From: sundasnoreen12 Date: Wed, 12 Jun 2024 00:57:27 -0700 Subject: [PATCH 1/5] feat: remove app level toggles --- src/index.scss | 4 +++ .../NotificationPreferenceApp.jsx | 34 +++++-------------- .../NotificationPreferenceColumn.jsx | 7 ++-- .../NotificationPreferences.jsx | 30 +++++++++++++++- .../NotificationTypes.jsx | 4 +-- src/notification-preferences/data/actions.js | 9 ----- src/notification-preferences/data/service.js | 10 ------ src/notification-preferences/data/thunks.js | 16 --------- src/notification-preferences/messages.js | 2 +- 9 files changed, 47 insertions(+), 69 deletions(-) diff --git a/src/index.scss b/src/index.scss index 03ea9d5..1518f56 100755 --- a/src/index.scss +++ b/src/index.scss @@ -113,6 +113,10 @@ $fa-font-path: "~font-awesome/fonts"; border-right: 0px !important; } + .email-channel { + width: 250px !important; + } + .dropdown-item:active, .dropdown-item:focus, .btn-tertiary:not(:disabled):not(.disabled).active { diff --git a/src/notification-preferences/NotificationPreferenceApp.jsx b/src/notification-preferences/NotificationPreferenceApp.jsx index 94d0f43..6a954e7 100644 --- a/src/notification-preferences/NotificationPreferenceApp.jsx +++ b/src/notification-preferences/NotificationPreferenceApp.jsx @@ -1,41 +1,33 @@ -import React, { useCallback } from 'react'; +import React from 'react'; import PropTypes from 'prop-types'; import classNames from 'classnames'; -import { useDispatch, useSelector } from 'react-redux'; +import { useSelector } from 'react-redux'; import { Collapsible } from '@openedx/paragon'; import { useIntl } from '@edx/frontend-platform/i18n'; - import messages from './messages'; import { useIsOnMobile } from '../hooks'; -import ToggleSwitch from './ToggleSwitch'; -import { LOADING_STATUS } from '../constants'; import NotificationTypes from './NotificationTypes'; -import { notificationChannels } from './data/utils'; -import { updateAppPreferenceToggle } from './data/thunks'; +import { notificationChannels, shouldHideAppPreferences } from './data/utils'; import NotificationPreferenceColumn from './NotificationPreferenceColumn'; -import { selectPreferenceAppToggleValue, selectSelectedCourseId, selectUpdatePreferencesStatus } from './data/selectors'; +import { selectPreferenceAppToggleValue, selectSelectedCourseId, selectPreferencesOfApp } from './data/selectors'; const NotificationPreferenceApp = ({ appId }) => { - const dispatch = useDispatch(); const intl = useIntl(); const courseId = useSelector(selectSelectedCourseId()); const appToggle = useSelector(selectPreferenceAppToggleValue(appId)); - const updatePreferencesStatus = useSelector(selectUpdatePreferencesStatus()); + const appPreferences = useSelector(selectPreferencesOfApp(appId)); const mobileView = useIsOnMobile(); const NOTIFICATION_CHANNELS = notificationChannels(); - - const onChangeAppSettings = useCallback((event) => { - dispatch(updateAppPreferenceToggle(courseId, appId, event.target.checked)); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [appId]); + const hideAppPreferences = shouldHideAppPreferences(appPreferences, appId) || false; if (!courseId) { return null; } return ( + !hideAppPreferences && ( { {intl.formatMessage(messages.notificationAppTitle, { key: appId })} - - - - {!mobileView &&
} -
+
{!mobileView && (
@@ -71,6 +54,7 @@ const NotificationPreferenceApp = ({ appId }) => { {mobileView &&
} + ) ); }; diff --git a/src/notification-preferences/NotificationPreferenceColumn.jsx b/src/notification-preferences/NotificationPreferenceColumn.jsx index e5a7038..2ed2630 100644 --- a/src/notification-preferences/NotificationPreferenceColumn.jsx +++ b/src/notification-preferences/NotificationPreferenceColumn.jsx @@ -62,7 +62,6 @@ const NotificationPreferenceColumn = ({ appId, channel, appPreference }) => { className={classNames( 'd-flex align-items-center justify-content-center mb-2 h-4.5 column-padding', { - 'pr-0': channel === NOTIFICATION_CHANNELS[NOTIFICATION_CHANNELS.length - 1], 'pl-0': channel === 'web' && mobileView, }, )} @@ -71,7 +70,7 @@ const NotificationPreferenceColumn = ({ appId, channel, appPreference }) => { name={channel} value={preference[channel]} onChange={(event) => onToggle(event, preference.id)} - disabled={nonEditable?.[preference.id]?.includes(channel) || updatePreferencesStatus === LOADING_STATUS} + disabled={updatePreferencesStatus === LOADING_STATUS} id={`${preference.id}-${channel}`} className="my-1" /> @@ -89,7 +88,7 @@ const NotificationPreferenceColumn = ({ appId, channel, appPreference }) => { return (
- {!hideAppPreferences && ( + {!hideAppPreferences && mobileView && ( { onClick={onChannelToggle} className={classNames('mb-3 header-label column-padding', { 'pr-0': channel === NOTIFICATION_CHANNELS[NOTIFICATION_CHANNELS.length - 1], - 'pl-0': channel === 'web' && mobileView, + 'pl-0': channel === 'web', })} > {intl.formatMessage(messages.notificationChannel, { text: channel })} diff --git a/src/notification-preferences/NotificationPreferences.jsx b/src/notification-preferences/NotificationPreferences.jsx index e78d53d..314c0cb 100644 --- a/src/notification-preferences/NotificationPreferences.jsx +++ b/src/notification-preferences/NotificationPreferences.jsx @@ -2,13 +2,15 @@ import React, { useEffect, useMemo } from 'react'; import { Link, useParams } from 'react-router-dom'; import { useDispatch, useSelector } from 'react-redux'; +import classNames from 'classnames'; import { ArrowBack } from '@openedx/paragon/icons'; import { useIntl } from '@edx/frontend-platform/i18n'; import { - Container, Hyperlink, Icon, Spinner, + Container, Hyperlink, Icon, Spinner, NavItem, } from '@openedx/paragon'; +import { useIsOnMobile } from '../hooks'; import messages from './messages'; import { NotFoundPage } from '../account-settings'; import NotificationPreferenceApp from './NotificationPreferenceApp'; @@ -19,6 +21,7 @@ import { import { selectCourse, selectCourseList, selectCourseListStatus, selectNotificationPreferencesStatus, selectPreferenceAppsId, } from './data/selectors'; +import { notificationChannels } from './data/utils'; const NotificationPreferences = () => { const { courseId } = useParams(); @@ -29,6 +32,8 @@ const NotificationPreferences = () => { const course = useSelector(selectCourse(courseId)); const notificationStatus = useSelector(selectNotificationPreferencesStatus()); const preferenceAppsIds = useSelector(selectPreferenceAppsId()); + const mobileView = useIsOnMobile(); + const NOTIFICATION_CHANNELS = notificationChannels(); const isLoading = notificationStatus === LOADING_STATUS || courseStatus === LOADING_STATUS; const preferencesList = useMemo(() => ( @@ -77,6 +82,29 @@ const NotificationPreferences = () => { {course?.name}
+ {!mobileView && ( +
+
+ {Object.values(NOTIFICATION_CHANNELS).map((channel) => ( +
+ + {intl.formatMessage(messages.notificationChannel, { text: channel })} + +
+ ))} +
+
+ )} {preferencesList} {isLoading && (
diff --git a/src/notification-preferences/NotificationTypes.jsx b/src/notification-preferences/NotificationTypes.jsx index 628538d..7ddd626 100644 --- a/src/notification-preferences/NotificationTypes.jsx +++ b/src/notification-preferences/NotificationTypes.jsx @@ -9,7 +9,7 @@ import { Icon, OverlayTrigger, Tooltip } from '@openedx/paragon'; import messages from './messages'; import { useIsOnMobile } from '../hooks'; -import { notificationChannels, shouldHideAppPreferences } from './data/utils'; +import { notificationChannels } from './data/utils'; import { selectPreferencesOfApp } from './data/selectors'; import NotificationPreferenceColumn from './NotificationPreferenceColumn'; @@ -19,11 +19,9 @@ const NotificationTypes = ({ appId }) => { const preferences = useSelector(selectPreferencesOfApp(appId)); const mobileView = useIsOnMobile(); const NOTIFICATION_CHANNELS = notificationChannels(); - const hideAppPreferences = shouldHideAppPreferences(preferences, appId) || false; return (
- {!mobileView && !hideAppPreferences && {intl.formatMessage(messages.typeLabel)}} {preferences.map(preference => ( (preference?.coreNotificationTypes?.length > 0 || preference.id !== 'core') && ( <> diff --git a/src/notification-preferences/data/actions.js b/src/notification-preferences/data/actions.js index e59cdc9..a8e5420 100644 --- a/src/notification-preferences/data/actions.js +++ b/src/notification-preferences/data/actions.js @@ -47,12 +47,3 @@ export const updatePreferenceValue = (appId, preferenceName, notificationChannel value, }) ); - -export const updateAppToggle = (courseId, appId, value) => dispatch => ( - dispatch({ - type: Actions.UPDATE_APP_PREFERENCE, - courseId, - appId, - value, - }) -); diff --git a/src/notification-preferences/data/service.js b/src/notification-preferences/data/service.js index 50a1e04..810e9db 100644 --- a/src/notification-preferences/data/service.js +++ b/src/notification-preferences/data/service.js @@ -15,16 +15,6 @@ export const getCourseList = async (page, pageSize) => { return data; }; -export const patchAppPreferenceToggle = async (courseId, appId, value) => { - const patchData = snakeCaseObject({ - notificationApp: appId, - value, - }); - const url = `${getConfig().LMS_BASE_URL}/api/notifications/configurations/${courseId}`; - const { data } = await getAuthenticatedHttpClient().patch(url, patchData); - return data; -}; - export const patchPreferenceToggle = async ( courseId, notificationApp, diff --git a/src/notification-preferences/data/thunks.js b/src/notification-preferences/data/thunks.js index 8671b08..3f5706b 100644 --- a/src/notification-preferences/data/thunks.js +++ b/src/notification-preferences/data/thunks.js @@ -7,14 +7,12 @@ import { fetchNotificationPreferenceFailed, fetchNotificationPreferenceFetching, fetchNotificationPreferenceSuccess, - updateAppToggle, updatePreferenceValue, updateSelectedCourse, } from './actions'; import { getCourseList, getCourseNotificationPreferences, - patchAppPreferenceToggle, patchChannelPreferenceToggle, patchPreferenceToggle, } from './service'; @@ -103,20 +101,6 @@ export const fetchCourseNotificationPreferences = (courseId) => ( } ); -export const updateAppPreferenceToggle = (courseId, appId, value) => ( - async (dispatch) => { - try { - dispatch(updateAppToggle(courseId, appId, value)); - const data = await patchAppPreferenceToggle(courseId, appId, value); - const normalizedData = normalizePreferences(camelCaseObject(data)); - dispatch(fetchNotificationPreferenceSuccess(courseId, normalizedData)); - } catch (errors) { - dispatch(updateAppToggle(courseId, appId, !value)); - dispatch(fetchNotificationPreferenceFailed()); - } - } -); - export const updatePreferenceToggle = ( courseId, notificationApp, diff --git a/src/notification-preferences/messages.js b/src/notification-preferences/messages.js index 45ef84c..9c9611b 100644 --- a/src/notification-preferences/messages.js +++ b/src/notification-preferences/messages.js @@ -22,7 +22,7 @@ const messages = defineMessages({ id: 'notification.preference.title', defaultMessage: `{ text, select, - core {Core notifications} + core {Activity notifications} newDiscussionPost {New discussion posts} newQuestionPost {New question posts} contentReported {Reported content} From 5954b3122f95a6c05930080c9dc351bd8f9d21cc Mon Sep 17 00:00:00 2001 From: sundasnoreen12 Date: Wed, 12 Jun 2024 00:59:39 -0700 Subject: [PATCH 2/5] fix: remove extra check --- src/notification-preferences/NotificationPreferences.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/notification-preferences/NotificationPreferences.jsx b/src/notification-preferences/NotificationPreferences.jsx index 314c0cb..ec20a18 100644 --- a/src/notification-preferences/NotificationPreferences.jsx +++ b/src/notification-preferences/NotificationPreferences.jsx @@ -93,7 +93,7 @@ const NotificationPreferences = () => { role="button" className={classNames('mb-3 header-label column-padding', { 'pr-0': channel === NOTIFICATION_CHANNELS[NOTIFICATION_CHANNELS.length - 1], - 'mr-2': channel === 'web' && !mobileView, + 'mr-2': channel === 'web', 'email-channel ': channel === 'email', })} From f5e2fa74484b6b64c0e5b1aee650ff8effd25919 Mon Sep 17 00:00:00 2001 From: sundasnoreen12 Date: Wed, 12 Jun 2024 13:53:34 +0500 Subject: [PATCH 3/5] test: removed test cases --- .../NotificationPreferences.test.jsx | 43 ------------------- 1 file changed, 43 deletions(-) diff --git a/src/notification-preferences/NotificationPreferences.test.jsx b/src/notification-preferences/NotificationPreferences.test.jsx index ad1c73b..b3a2908 100644 --- a/src/notification-preferences/NotificationPreferences.test.jsx +++ b/src/notification-preferences/NotificationPreferences.test.jsx @@ -154,13 +154,6 @@ describe('Notification Preferences', () => { expect(screen.queryAllByTestId('notification-preference')).toHaveLength(4); }); - it('update group on click', async () => { - const wrapper = await render(notificationPreferences(store)); - const element = wrapper.container.querySelector('#discussion-app-toggle'); - await fireEvent.click(element); - expect(mockDispatch).toHaveBeenCalled(); - }); - it('update preference on click', async () => { const wrapper = await render(notificationPreferences(store)); const element = wrapper.container.querySelector('#core-web'); @@ -174,40 +167,4 @@ describe('Notification Preferences', () => { await render(notificationPreferences(store)); expect(screen.queryByTestId('not-found-page')).toBeInTheDocument(); }); - - it('updates all preferences in the column on web channel click', async () => { - store = setupStore(updateChannelPreferences(true)); - const wrapper = render(notificationPreferences(store)); - - const getChannelSwitch = (id) => screen.queryByTestId(`${id}-web`); - const notificationTypes = ['newComment', 'newAssignment']; - - const verifyState = (toggleState) => { - notificationTypes.forEach((notificationType) => { - if (toggleState) { - expect(getChannelSwitch(notificationType)).toBeChecked(); - } else { - expect(getChannelSwitch(notificationType)).not.toBeChecked(); - } - }); - }; - - verifyState(true); - expect(getChannelSwitch('core')).toBeChecked(); - - const discussionApp = screen.queryByTestId('discussion-app'); - const webChannel = within(discussionApp).queryByText('Web'); - - await act(async () => { - await fireEvent.click(webChannel); - }); - - store = setupStore(updateChannelPreferences(false)); - wrapper.rerender(notificationPreferences(store)); - - await waitFor(() => { - verifyState(false); - expect(getChannelSwitch('core')).toBeChecked(); - }); - }); }); From 4502dbe4139090e85059c288e15d5e3640480baa Mon Sep 17 00:00:00 2001 From: sundasnoreen12 Date: Wed, 12 Jun 2024 13:56:37 +0500 Subject: [PATCH 4/5] fix: removed role button to remove cursor pointer --- .../NotificationPreferences.jsx | 1 - .../NotificationPreferences.test.jsx | 18 +----------------- 2 files changed, 1 insertion(+), 18 deletions(-) diff --git a/src/notification-preferences/NotificationPreferences.jsx b/src/notification-preferences/NotificationPreferences.jsx index ec20a18..da83f35 100644 --- a/src/notification-preferences/NotificationPreferences.jsx +++ b/src/notification-preferences/NotificationPreferences.jsx @@ -90,7 +90,6 @@ const NotificationPreferences = () => { ({ - preferences: [ - { - id: 'core', appId: 'discussion', web: true, coreNotificationTypes: ['new_comment'], - }, - { - id: 'newComment', appId: 'discussion', web: toggleVal, coreNotificationTypes: [], - }, - { - id: 'newAssignment', appId: 'coursework', web: toggleVal, coreNotificationTypes: [], - }, - ], -}); - const setupStore = (override = {}) => { const storeState = defaultState; storeState.courses = { From 5aacaf44a3510205164aafee736e959bd8a450b2 Mon Sep 17 00:00:00 2001 From: sundasnoreen12 Date: Fri, 14 Jun 2024 12:17:47 +0500 Subject: [PATCH 5/5] refactor: rename selector --- src/notification-preferences/NotificationPreferenceApp.jsx | 4 ++-- src/notification-preferences/NotificationPreferenceColumn.jsx | 4 ++-- src/notification-preferences/NotificationTypes.jsx | 4 ++-- src/notification-preferences/data/selectors.js | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/notification-preferences/NotificationPreferenceApp.jsx b/src/notification-preferences/NotificationPreferenceApp.jsx index 6a954e7..2d6b9a1 100644 --- a/src/notification-preferences/NotificationPreferenceApp.jsx +++ b/src/notification-preferences/NotificationPreferenceApp.jsx @@ -11,13 +11,13 @@ import { useIsOnMobile } from '../hooks'; import NotificationTypes from './NotificationTypes'; import { notificationChannels, shouldHideAppPreferences } from './data/utils'; import NotificationPreferenceColumn from './NotificationPreferenceColumn'; -import { selectPreferenceAppToggleValue, selectSelectedCourseId, selectPreferencesOfApp } from './data/selectors'; +import { selectPreferenceAppToggleValue, selectSelectedCourseId, selectAppPreferences } from './data/selectors'; const NotificationPreferenceApp = ({ appId }) => { const intl = useIntl(); const courseId = useSelector(selectSelectedCourseId()); const appToggle = useSelector(selectPreferenceAppToggleValue(appId)); - const appPreferences = useSelector(selectPreferencesOfApp(appId)); + const appPreferences = useSelector(selectAppPreferences(appId)); const mobileView = useIsOnMobile(); const NOTIFICATION_CHANNELS = notificationChannels(); const hideAppPreferences = shouldHideAppPreferences(appPreferences, appId) || false; diff --git a/src/notification-preferences/NotificationPreferenceColumn.jsx b/src/notification-preferences/NotificationPreferenceColumn.jsx index 2ed2630..8d27059 100644 --- a/src/notification-preferences/NotificationPreferenceColumn.jsx +++ b/src/notification-preferences/NotificationPreferenceColumn.jsx @@ -14,7 +14,7 @@ import EmailCadences from './EmailCadences'; import { LOADING_STATUS } from '../constants'; import { updateChannelPreferenceToggle, updatePreferenceToggle } from './data/thunks'; import { - selectNonEditablePreferences, selectPreferencesOfApp, selectSelectedCourseId, selectUpdatePreferencesStatus, + selectNonEditablePreferences, selectAppPreferences, selectSelectedCourseId, selectUpdatePreferencesStatus, } from './data/selectors'; import { notificationChannels, shouldHideAppPreferences } from './data/utils'; @@ -22,7 +22,7 @@ const NotificationPreferenceColumn = ({ appId, channel, appPreference }) => { const dispatch = useDispatch(); const intl = useIntl(); const courseId = useSelector(selectSelectedCourseId()); - const appPreferences = useSelector(selectPreferencesOfApp(appId)); + const appPreferences = useSelector(selectAppPreferences(appId)); const nonEditable = useSelector(selectNonEditablePreferences(appId)); const updatePreferencesStatus = useSelector(selectUpdatePreferencesStatus()); const mobileView = useIsOnMobile(); diff --git a/src/notification-preferences/NotificationTypes.jsx b/src/notification-preferences/NotificationTypes.jsx index 7ddd626..0d5fc98 100644 --- a/src/notification-preferences/NotificationTypes.jsx +++ b/src/notification-preferences/NotificationTypes.jsx @@ -11,12 +11,12 @@ import messages from './messages'; import { useIsOnMobile } from '../hooks'; import { notificationChannels } from './data/utils'; -import { selectPreferencesOfApp } from './data/selectors'; +import { selectAppPreferences } from './data/selectors'; import NotificationPreferenceColumn from './NotificationPreferenceColumn'; const NotificationTypes = ({ appId }) => { const intl = useIntl(); - const preferences = useSelector(selectPreferencesOfApp(appId)); + const preferences = useSelector(selectAppPreferences(appId)); const mobileView = useIsOnMobile(); const NOTIFICATION_CHANNELS = notificationChannels(); diff --git a/src/notification-preferences/data/selectors.js b/src/notification-preferences/data/selectors.js index ffbc5d9..f5c5cb8 100644 --- a/src/notification-preferences/data/selectors.js +++ b/src/notification-preferences/data/selectors.js @@ -28,7 +28,7 @@ export const selectPreferenceAppsId = () => state => ( state.notificationPreferences.preferences.apps.map(app => app.id) ); -export const selectPreferencesOfApp = appId => state => ( +export const selectAppPreferences = appId => state => ( selectPreferences()(state).filter(preference => ( preference.appId === appId ))