Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: added notification preference ui #784

Merged
merged 1 commit into from
May 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/account-settings/AccountSettingsPage.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -819,12 +819,12 @@ class AccountSettingsPage extends React.Component {
</h1>
<div>
<div className="row">
<div className="col-md-3">
<div className="col-md-2">
<JumpNav
displayDemographicsLink={this.props.formValues.shouldDisplayDemographicsSection}
/>
</div>
<div className="col-md-9">
<div className="col-md-10">
{loading ? this.renderLoading() : null}
{loaded ? this.renderContent() : null}
{loadingError ? this.renderError() : null}
Expand Down
5 changes: 5 additions & 0 deletions src/account-settings/AccountSettingsPage.messages.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -565,6 +565,11 @@ const messages = defineMessages({
defaultMessage: 'No value set.',
description: 'The placeholder for an empty but uneditable field when there is no administrator',
},
'notification.preferences.notifications.label': {
id: 'notification.preferences.notifications.label',
defaultMessage: 'Notifications',
description: 'Label for Notifications',
},
});

export default messages;
23 changes: 21 additions & 2 deletions src/account-settings/JumpNav.jsx
Original file line number Diff line number Diff line change
@@ -1,20 +1,23 @@
import { getConfig } from '@edx/frontend-platform';
import { injectIntl, intlShape } from '@edx/frontend-platform/i18n';
import { breakpoints, useWindowSize } from '@edx/paragon';
import { breakpoints, useWindowSize, Icon } from '@edx/paragon';
import { OpenInNew } from '@edx/paragon/icons';
import classNames from 'classnames';
import PropTypes from 'prop-types';
import React from 'react';
import { NavHashLink } from 'react-router-hash-link';
import Scrollspy from 'react-scrollspy';
import { Link } from 'react-router-dom';
import messages from './AccountSettingsPage.messages';

const JumpNav = ({
intl,
displayDemographicsLink,
}) => {
const stickToTop = useWindowSize().width > breakpoints.small.minWidth;
const showNotificationMenu = false;
return (
<div className={classNames('jump-nav', { 'jump-nav-sm position-sticky pt-3': stickToTop })}>
<div className={classNames('jump-nav px-2.25', { 'jump-nav-sm position-sticky pt-3': stickToTop })}>
<Scrollspy
items={[
'basic-information',
Expand Down Expand Up @@ -67,6 +70,22 @@ const JumpNav = ({
</NavHashLink>
</li>
</Scrollspy>
{showNotificationMenu
&& (
<>
<hr />
<Scrollspy
className="list-unstyled"
>
<li>
<Link to="/notifications" target="_blank" rel="noopener noreferrer">
<span>{intl.formatMessage(messages['notification.preferences.notifications.label'])}</span>
<Icon className="d-inline-block align-bottom ml-1" src={OpenInNew} />
</Link>
</li>
</Scrollspy>
</>
)}
</div>
);
};
Expand Down
4 changes: 2 additions & 2 deletions src/account-settings/test/__snapshots__/JumpNav.test.jsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

exports[`JumpNav should not render Optional Information link 1`] = `
<div
className="jump-nav jump-nav-sm position-sticky pt-3"
className="jump-nav px-2.25 jump-nav-sm position-sticky pt-3"
>
<ul
className="list-unstyled"
Expand Down Expand Up @@ -80,7 +80,7 @@ exports[`JumpNav should not render Optional Information link 1`] = `

exports[`JumpNav should render Optional Information link 1`] = `
<div
className="jump-nav jump-nav-sm position-sticky pt-3"
className="jump-nav px-2.25 jump-nav-sm position-sticky pt-3"
>
<ul
className="list-unstyled"
Expand Down
2 changes: 2 additions & 0 deletions src/data/reducers.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@ import {
reducer as accountSettingsReducer,
storeName as accountSettingsStoreName,
} from '../account-settings';
import notificationPreferencesReducer from '../notification-preferences/data/reducers';

const createRootReducer = () => combineReducers({
[accountSettingsStoreName]: accountSettingsReducer,
notificationPreferences: notificationPreferencesReducer,
});
export default createRootReducer;
9 changes: 9 additions & 0 deletions src/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,11 @@ import messages from './i18n';

import './index.scss';
import Head from './head/Head';
import NotificationCourses from './notification-preferences/NotificationCourses';
import NotificationPreferences from './notification-preferences/NotificationPreferences';

subscribe(APP_READY, () => {
const allowNotificationRoutes = false;
ReactDOM.render(
<AppProvider store={configureStore()}>
<Head />
Expand All @@ -32,6 +35,12 @@ subscribe(APP_READY, () => {
<Header />
<main className="flex-grow-1">
<Switch>
{allowNotificationRoutes && (
<>
<Route path="/notifications/:courseId" component={NotificationPreferences} />
<Route path="/notifications" component={NotificationCourses} />
</>
)}
<Route path="/id-verification" component={IdVerificationPage} />
<Route exact path="/" component={AccountSettingsPage} />
<Route path="/notfound" component={NotFoundPage} />
Expand Down
24 changes: 24 additions & 0 deletions src/index.scss
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,27 @@ $fa-font-path: "~font-awesome/fonts";
font-weight: normal;
}
}

.notification-heading {
line-height: 36px;
font-weight: 700;
font-size: 32px;
}

.notification-course-title {
line-height: 28px;
font-weight: 700;
font-size: 18px;
}

.px-2\.25 {
padding-left: 0.625rem;
}

.notification-help-text {
font-size: 14px;
font-weight: 400;
line-height: 28px;
height: 28px;
color: #707070;
}
65 changes: 65 additions & 0 deletions src/notification-preferences/NotificationCourses.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import React, { useEffect } from 'react';
import { useDispatch, useSelector } from 'react-redux';
import { Link } from 'react-router-dom';
import { injectIntl, intlShape } from '@edx/frontend-platform/i18n';
import { Container, Icon, Spinner } from '@edx/paragon';
import { ArrowForwardIos } from '@edx/paragon/icons';
import { fetchCourseList } from './data/thunks';
import { courseListStatus, getCourseList } from './data/selectors';
import { IDLE_STATUS, LOADING_STATUS } from '../constants';
import { messages } from './messages';

const NotificationCourses = ({ intl }) => {
const dispatch = useDispatch();
const courseStatus = useSelector(courseListStatus());
const coursesList = useSelector(getCourseList());
useEffect(() => {
if (courseStatus === IDLE_STATUS || coursesList.length === 0) {
dispatch(fetchCourseList());
}
// eslint-disable-next-line react-hooks/exhaustive-deps
awais-ansari marked this conversation as resolved.
Show resolved Hide resolved
}, [courseStatus]);
if (courseStatus === LOADING_STATUS) {
return (
<div className="d-flex h-100">
<Spinner
variant="primary"
animation="border"
className="mx-auto my-auto"
style={{ width: '4rem', height: '4rem' }}
/>
</div>
);
}
Comment on lines +22 to +33
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can memo this like

const SpinnerComp = useMemo(() => {
if (courseStatus === LOADING_STATUS) {
return (


<Spinner
variant="primary"
animation="border"
className="mx-auto my-auto"
style={{ width: '4rem', height: '4rem' }}
/>

);
}
return null;
}, [courseStatus]);
if (SpinnerComp) { return SpinnerComp; }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this component is not being rendered multiple times, hence memoization is not required. Excessive memoization should be avoided in code

return (
<Container size="md">
<h2 className="notification-heading mt-6 mb-5.5">
{intl.formatMessage(messages.notificationHeading)}
</h2>
<div>
{
coursesList.map(course => (
<Link
to={`/notifications/${course.id}`}
>
<div className="mb-4 d-flex text-gray-700">
<span className="ml-0 mr-auto">
{course.name}
</span>
<span className="ml-auto mr-0">
<Icon src={ArrowForwardIos} />
</span>
</div>
</Link>
))
}
</div>
</Container>
);
};

NotificationCourses.propTypes = {
intl: intlShape.isRequired,
};

export default injectIntl(NotificationCourses);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

memo this
export default injectIntl(React.memo(NotificationCourses));

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NotificationCourses component is not taking any props (except intl). I don't think that React.memo will be helpful

74 changes: 74 additions & 0 deletions src/notification-preferences/NotificationPreferenceGroup.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
import React, { useCallback, useMemo, useState } from 'react';
import PropTypes from 'prop-types';
import { useDispatch, useSelector } from 'react-redux';
import { useIntl } from '@edx/frontend-platform/i18n';
import { Collapsible } from '@edx/paragon';
import { messages } from './messages';
import ToggleSwitch from './ToggleSwitch';
import {
getPreferenceGroup,
getSelectedCourse,
} from './data/selectors';
import NotificationPreferenceRow from './NotificationPreferenceRow';
import { updateGroupValue } from './data/actions';

const NotificationPreferenceGroup = ({ groupId }) => {
const dispatch = useDispatch();
const intl = useIntl();
const courseId = useSelector(getSelectedCourse());
const preferenceGroup = useSelector(getPreferenceGroup(groupId));
const [groupToggle, setGroupToggle] = useState(true);

const preferences = useMemo(() => (
preferenceGroup.map(preference => (
<NotificationPreferenceRow
key={preference.id}
groupId={groupId}
preferenceName={preference.id}
/>
))), [groupId, preferenceGroup]);

const onChangeGroupSettings = useCallback((checked) => {
setGroupToggle(checked);
dispatch(updateGroupValue(courseId, groupId, checked));
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [groupId]);
awais-ansari marked this conversation as resolved.
Show resolved Hide resolved
muhammadadeeltajamul marked this conversation as resolved.
Show resolved Hide resolved

if (!courseId) {
return null;
}
return (
<Collapsible.Advanced open={groupToggle}>
<Collapsible.Trigger>
<div className="d-flex">
<span className="ml-0 mr-auto">
{intl.formatMessage(messages.notificationGroupTitle, { key: groupId })}
</span>
<span className="ml-auto mr-0">
<ToggleSwitch value={groupToggle} onChange={onChangeGroupSettings} />
</span>
</div>
<hr />
</Collapsible.Trigger>
<Collapsible.Body>
<div className="d-flex flex-row notification-help-text">
<span className="col-8 px-0">{intl.formatMessage(messages.notificationHelpType)}</span>
<span className="d-flex col-4 px-0">
<span className="ml-0 mr-auto">{intl.formatMessage(messages.notificationHelpWeb)}</span>
<span className="mx-auto">{intl.formatMessage(messages.notificationHelpEmail)}</span>
<span className="ml-auto mr-0 pr-2.5">{intl.formatMessage(messages.notificationHelpPush)}</span>
</span>
</div>
<div className="mt-3 pb-5">
{ preferences }
</div>
</Collapsible.Body>
</Collapsible.Advanced>
);
};

NotificationPreferenceGroup.propTypes = {
groupId: PropTypes.string.isRequired,
};

export default React.memo(NotificationPreferenceGroup);
51 changes: 51 additions & 0 deletions src/notification-preferences/NotificationPreferenceRow.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import React, { useCallback } from 'react';
import PropTypes from 'prop-types';
import { useDispatch, useSelector } from 'react-redux';
import { useIntl } from '@edx/frontend-platform/i18n';
import { messages } from './messages';
import ToggleSwitch from './ToggleSwitch';
import { getPreferenceAttribute } from './data/selectors';
import { updatePreferenceValue } from './data/actions';

const NotificationPreferenceRow = ({ groupId, preferenceName }) => {
const dispatch = useDispatch();
const intl = useIntl();
const preference = useSelector(getPreferenceAttribute(groupId, preferenceName));
const onToggle = useCallback((checked, notificationChannel) => {
dispatch(updatePreferenceValue(groupId, preferenceName, notificationChannel, checked));
}, [dispatch, groupId, preferenceName]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need to pass dispatch here, instead pass updatePreferenceValue

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updatePreferenceValue is a function. I don't think we need to pass function in dependency list. Added dispatch to avoid disabling lint for this line

return (
<div className="d-flex flex-row mb-3">
<span className="col-8 px-0">
{intl.formatMessage(messages.notificationTitle, { text: preferenceName })}
</span>
<span className="d-flex col-4 px-0">
<span className="ml-0 mr-auto">
<ToggleSwitch
value={preference.web}
onChange={(checked) => onToggle(checked, 'web')}
/>
</span>
<span className="mx-auto">
<ToggleSwitch
value={preference.email}
onChange={(checked) => onToggle(checked, 'email')}
/>
</span>
<span className="ml-auto mr-0">
<ToggleSwitch
value={preference.push}
onChange={(checked) => onToggle(checked, 'push')}
/>
</span>
</span>
</div>
);
};

NotificationPreferenceRow.propTypes = {
groupId: PropTypes.string.isRequired,
preferenceName: PropTypes.string.isRequired,
};

export default React.memo(NotificationPreferenceRow);
Loading