Skip to content

Commit

Permalink
refactor: fixed UI and logic related issue
Browse files Browse the repository at this point in the history
  • Loading branch information
sundasnoreen12 committed Jan 2, 2024
1 parent 8b36507 commit c949cd5
Show file tree
Hide file tree
Showing 10 changed files with 79 additions and 67 deletions.
2 changes: 1 addition & 1 deletion src/courseware/course/new-sidebar/Sidebar.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const Sidebar = () => {
const SidebarToRender = SIDEBARS[currentSidebar].Sidebar;

Check warning on line 20 in src/courseware/course/new-sidebar/Sidebar.jsx

View check run for this annotation

Codecov / codecov/patch

src/courseware/course/new-sidebar/Sidebar.jsx#L20

Added line #L20 was not covered by tests

return (

Check warning on line 22 in src/courseware/course/new-sidebar/Sidebar.jsx

View check run for this annotation

Codecov / codecov/patch

src/courseware/course/new-sidebar/Sidebar.jsx#L22

Added line #L22 was not covered by tests
<div className={classNames('vh-100 d-flex flex-column', { 'bg-white fixed-top': shouldDisplayFullScreen })}>
<div className={classNames('d-flex flex-column', { 'bg-white fixed-top': shouldDisplayFullScreen })}>
{shouldDisplayFullScreen && (
<div

Check warning on line 25 in src/courseware/course/new-sidebar/Sidebar.jsx

View check run for this annotation

Codecov / codecov/patch

src/courseware/course/new-sidebar/Sidebar.jsx#L25

Added line #L25 was not covered by tests
className="pt-2 pb-2.5 border-bottom border-light-400 d-flex align-items-center ml-2"
Expand Down
31 changes: 19 additions & 12 deletions src/courseware/course/new-sidebar/SidebarContextProvider.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,37 +3,42 @@ import PropTypes from 'prop-types';
import React, {
useEffect, useState, useMemo, useCallback,
} from 'react';
import { useIntl } from '@edx/frontend-platform/i18n';
import isEmpty from 'lodash/isEmpty';
import { SIDEBARS } from './sidebars';
import { getLocalStorage, setLocalStorage } from '../../../data/localStorage';
import SidebarContext from './SidebarContext';
import { useModel } from '../../../generic/model-store';
import messages from './messages';

const SidebarProvider = ({
courseId,
unitId,
children,
}) => {
const intl = useIntl();
const shouldDisplayFullScreen = useWindowSize().width < breakpoints.large.minWidth;
const shouldDisplaySidebarOpen = useWindowSize().width > breakpoints.medium.minWidth;
const query = new URLSearchParams(window.location.search);

Check warning on line 22 in src/courseware/course/new-sidebar/SidebarContextProvider.jsx

View check run for this annotation

Codecov / codecov/patch

src/courseware/course/new-sidebar/SidebarContextProvider.jsx#L18-L22

Added lines #L18 - L22 were not covered by tests
const initialSidebar = (shouldDisplaySidebarOpen || query.get('sidebar') === 'true')
? SIDEBARS.DISCUSSIONS_NOTIFICATIONS.ID : null;
const [currentSidebar, setCurrentSidebar] = useState(initialSidebar);
const [notificationStatus, setNotificationStatus] = useState(getLocalStorage(`notificationStatus.${courseId}`));
const [hideDiscussionbar, setHideDiscussionbar] = useState(false);
const [hideNotificationbar, setHideNotificationbar] = useState(false);
const [upgradeNotificationCurrentState, setUpgradeNotificationCurrentState] = useState(

Check warning on line 29 in src/courseware/course/new-sidebar/SidebarContextProvider.jsx

View check run for this annotation

Codecov / codecov/patch

src/courseware/course/new-sidebar/SidebarContextProvider.jsx#L24-L29

Added lines #L24 - L29 were not covered by tests
getLocalStorage(`upgradeNotificationCurrentState.${courseId}`),
);
const topic = useModel('discussionTopics', unitId);
const { verifiedMode } = useModel('courseHomeMeta', courseId);

Check warning on line 33 in src/courseware/course/new-sidebar/SidebarContextProvider.jsx

View check run for this annotation

Codecov / codecov/patch

src/courseware/course/new-sidebar/SidebarContextProvider.jsx#L32-L33

Added lines #L32 - L33 were not covered by tests
const isDiscussionbarAvailable = !topic?.id || !topic?.enabledInContext;
const isDiscussionbarAvailable = !(!topic?.id || !topic?.enabledInContext);
const isNotificationbarAvailable = !isEmpty(verifiedMode);

Check warning on line 35 in src/courseware/course/new-sidebar/SidebarContextProvider.jsx

View check run for this annotation

Codecov / codecov/patch

src/courseware/course/new-sidebar/SidebarContextProvider.jsx#L35

Added line #L35 was not covered by tests
const [hideDiscussionbar, setHideDiscussionbar] = useState(isDiscussionbarAvailable);
const [hideNotificationbar, setHideNotificationbar] = useState(isNotificationbarAvailable);

useEffect(() => {
setHideDiscussionbar(!isDiscussionbarAvailable);
setHideNotificationbar(!isNotificationbarAvailable);
setCurrentSidebar(SIDEBARS.DISCUSSIONS_NOTIFICATIONS.ID);

Check warning on line 40 in src/courseware/course/new-sidebar/SidebarContextProvider.jsx

View check run for this annotation

Codecov / codecov/patch

src/courseware/course/new-sidebar/SidebarContextProvider.jsx#L37-L40

Added lines #L37 - L40 were not covered by tests
}, [unitId]);
}, [unitId, topic]);

const onNotificationSeen = useCallback(() => {
setNotificationStatus('inactive');
Expand All @@ -46,17 +51,19 @@ const SidebarProvider = ({
}
}, [hideDiscussionbar, hideNotificationbar]);

const toggleSidebar = useCallback((sidebarId, tabId) => {
if (tabId === Discussions) {
setHideDiscussionbar(true);
} else if (tabId === Notifications) {
setHideNotificationbar(true);
const toggleSidebar = useCallback((sidebarId, widgetId) => {

Check warning on line 54 in src/courseware/course/new-sidebar/SidebarContextProvider.jsx

View check run for this annotation

Codecov / codecov/patch

src/courseware/course/new-sidebar/SidebarContextProvider.jsx#L54

Added line #L54 was not covered by tests
if (widgetId) {
if (widgetId === intl.formatMessage(messages.discussionsTitle)) {
setHideDiscussionbar(true);

Check warning on line 57 in src/courseware/course/new-sidebar/SidebarContextProvider.jsx

View check run for this annotation

Codecov / codecov/patch

src/courseware/course/new-sidebar/SidebarContextProvider.jsx#L57

Added line #L57 was not covered by tests
} else if (widgetId === intl.formatMessage(messages.notificationTitle)) {
setHideNotificationbar(true);

Check warning on line 59 in src/courseware/course/new-sidebar/SidebarContextProvider.jsx

View check run for this annotation

Codecov / codecov/patch

src/courseware/course/new-sidebar/SidebarContextProvider.jsx#L59

Added line #L59 was not covered by tests
}
} else {

Check warning on line 61 in src/courseware/course/new-sidebar/SidebarContextProvider.jsx

View check run for this annotation

Codecov / codecov/patch

src/courseware/course/new-sidebar/SidebarContextProvider.jsx#L61

Added line #L61 was not covered by tests
setCurrentSidebar(prevSidebar => (sidebarId === prevSidebar ? null : sidebarId));
if (isDiscussionbarAvailable) { setHideDiscussionbar(false); }
if (isNotificationbarAvailable) { setHideNotificationbar(false); }
setHideDiscussionbar(!isDiscussionbarAvailable);
setHideNotificationbar(!isNotificationbarAvailable);

Check warning on line 64 in src/courseware/course/new-sidebar/SidebarContextProvider.jsx

View check run for this annotation

Codecov / codecov/patch

src/courseware/course/new-sidebar/SidebarContextProvider.jsx#L63-L64

Added lines #L63 - L64 were not covered by tests
}
}, [isNotificationbarAvailable, isDiscussionbarAvailable]);
}, [isDiscussionbarAvailable, isNotificationbarAvailable]);

const contextValue = useMemo(() => ({

Check warning on line 68 in src/courseware/course/new-sidebar/SidebarContextProvider.jsx

View check run for this annotation

Codecov / codecov/patch

src/courseware/course/new-sidebar/SidebarContextProvider.jsx#L68

Added line #L68 was not covered by tests
toggleSidebar,
Expand Down
8 changes: 6 additions & 2 deletions src/courseware/course/new-sidebar/common/SidebarBase.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const SidebarBase = ({
showTitleBar,
width,
allowFullHeight,
showBorder,
}) => {
const intl = useIntl();

Check warning on line 22 in src/courseware/course/new-sidebar/common/SidebarBase.jsx

View check run for this annotation

Codecov / codecov/patch

src/courseware/course/new-sidebar/common/SidebarBase.jsx#L21-L22

Added lines #L21 - L22 were not covered by tests
const {
Expand All @@ -28,7 +29,7 @@ const SidebarBase = ({
const receiveMessage = useCallback(({ data }) => {
const { type } = data;

Check warning on line 30 in src/courseware/course/new-sidebar/common/SidebarBase.jsx

View check run for this annotation

Codecov / codecov/patch

src/courseware/course/new-sidebar/common/SidebarBase.jsx#L29-L30

Added lines #L29 - L30 were not covered by tests
if (type === 'learning.events.sidebar.close') {
toggleSidebar(sidebarId, 'Discussions');
toggleSidebar(sidebarId, intl.formatMessage(messages.discussionsTitle));

Check warning on line 32 in src/courseware/course/new-sidebar/common/SidebarBase.jsx

View check run for this annotation

Codecov / codecov/patch

src/courseware/course/new-sidebar/common/SidebarBase.jsx#L32

Added line #L32 was not covered by tests
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [toggleSidebar]);
Expand All @@ -37,9 +38,10 @@ const SidebarBase = ({

return (

Check warning on line 39 in src/courseware/course/new-sidebar/common/SidebarBase.jsx

View check run for this annotation

Codecov / codecov/patch

src/courseware/course/new-sidebar/common/SidebarBase.jsx#L39

Added line #L39 was not covered by tests
<section
className={classNames('ml-0 ml-lg-4 border border-light-400 rounded-sm h-auto align-top', {
className={classNames('ml-0 ml-lg-4 h-auto align-top', {
'min-vh-100': !shouldDisplayFullScreen && allowFullHeight,
'd-none': currentSidebar !== sidebarId,
'border border-light-400 rounded-sm': showBorder,
}, className)}
data-testid={`sidebar-${sidebarId}`}
style={{ width: shouldDisplayFullScreen ? '100%' : width }}
Expand Down Expand Up @@ -77,13 +79,15 @@ SidebarBase.propTypes = {
showTitleBar: PropTypes.bool,
width: PropTypes.string,
allowFullHeight: PropTypes.bool,
showBorder: PropTypes.bool,
};

SidebarBase.defaultProps = {
width: '31rem',
allowFullHeight: false,
showTitleBar: true,
className: '',
showBorder: true,
};

export default SidebarBase;
Original file line number Diff line number Diff line change
@@ -1,23 +1,26 @@
import React from 'react';
import React, { useContext } from 'react';
import { useIntl } from '@edx/frontend-platform/i18n';
import SidebarBase from '../../common/SidebarBase';
import NotificationTray from './notifications/NotificationsWidget';
import DiscussionsSidebar from './discussions/DiscussionsWidget';
import messages from '../../messages';
import { ID } from './DiscussionsNotificationsTrigger';
import SidebarContext from '../../SidebarContext';

const DiscussionsNotificationsSidebar = () => {
const intl = useIntl();
const { hideNotificationbar } = useContext(SidebarContext);

Check warning on line 12 in src/courseware/course/new-sidebar/sidebars/discussions-notifications/DiscussionsNotificationsSidebar.jsx

View check run for this annotation

Codecov / codecov/patch

src/courseware/course/new-sidebar/sidebars/discussions-notifications/DiscussionsNotificationsSidebar.jsx#L11-L12

Added lines #L11 - L12 were not covered by tests

return (

Check warning on line 14 in src/courseware/course/new-sidebar/sidebars/discussions-notifications/DiscussionsNotificationsSidebar.jsx

View check run for this annotation

Codecov / codecov/patch

src/courseware/course/new-sidebar/sidebars/discussions-notifications/DiscussionsNotificationsSidebar.jsx#L14

Added line #L14 was not covered by tests
<SidebarBase
ariaLabel={intl.formatMessage(messages.discussionNotificationTray)}
sidebarId={ID}
className="flex-fill"
className="d-flex flex-column flex-fill"
showTitleBar={false}
showBorder={false}
>
<NotificationTray />
<div className="my-1.5" />
{!hideNotificationbar && <div className="my-1.5" />}
<DiscussionsSidebar />
</SidebarBase>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ const DiscussionsWidget = () => {
return (
<iframe
src={`${discussionsUrl}?inContextSidebar`}
className="d-flex w-100 h-100 border-0 flex-fill"
className="d-flex w-100 vh-100 flex-fill border border-light-400 rounded-sm"
title={intl.formatMessage(messages.discussionsTitle)}
allow="clipboard-write"
loading="lazy"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,16 @@ import MockAdapter from 'axios-mock-adapter';
import React from 'react';
import {
initializeMockApp, initializeTestStore, render, screen,
} from '../../../../../setupTest';
import { executeThunk } from '../../../../../utils';
import { buildTopicsFromUnits } from '../../../../data/__factories__/discussionTopics.factory';
import { getCourseDiscussionTopics } from '../../../../data/thunks';
import SidebarContext from '../../SidebarContext';
import DiscussionsSidebar from './DiscussionsSidebar';
} from '../../../../../../setupTest';
import { executeThunk } from '../../../../../../utils';
import { buildTopicsFromUnits } from '../../../../../data/__factories__/discussionTopics.factory';
import { getCourseDiscussionTopics } from '../../../../../data/thunks';
import SidebarContext from '../../../SidebarContext';
import DiscussionsWidget from './DiscussionsWidget';

initializeMockApp();

describe('Discussions Trigger', () => {
describe('DiscussionsWidget', () => {
let axiosMock;
let mockData;
let courseId;
Expand Down Expand Up @@ -52,7 +52,7 @@ describe('Discussions Trigger', () => {
function renderWithProvider(testData = {}) {
const { container } = render(
<SidebarContext.Provider value={{ ...mockData, ...testData }}>
<DiscussionsSidebar />
<DiscussionsWidget />
</SidebarContext.Provider>,
);
return container;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,25 +39,26 @@ const NotificationsWidget = () => {
if (hideNotificationbar || !isNotificationbarAvailable) { return null; }

return (
<UpgradeNotification
offer={offer}
verifiedMode={verifiedMode}
accessExpiration={accessExpiration}
contentTypeGatingEnabled={contentTypeGatingEnabled}
marketingUrl={marketingUrl}
upsellPageName="in_course"
userTimezone={userTimezone}
shouldDisplayBorder={false}
timeOffsetMillis={timeOffsetMillis}
courseId={courseId}
org={org}
upgradeNotificationCurrentState={upgradeNotificationCurrentState}
setupgradeNotificationCurrentState={setUpgradeNotificationCurrentState}
showRemoveIcon
sidebarId="qqw"
toggleSidebar={toggleSidebar}
tabId={intl.formatMessage(messages.notificationTitle)}
/>
<div className="border border-light-400 rounded-sm">
<UpgradeNotification
offer={offer}
verifiedMode={verifiedMode}
accessExpiration={accessExpiration}
contentTypeGatingEnabled={contentTypeGatingEnabled}
marketingUrl={marketingUrl}
upsellPageName="in_course"
userTimezone={userTimezone}
shouldDisplayBorder={false}
timeOffsetMillis={timeOffsetMillis}
courseId={courseId}
org={org}
upgradeNotificationCurrentState={upgradeNotificationCurrentState}
setupgradeNotificationCurrentState={setUpgradeNotificationCurrentState}
showRemoveIcon
toggleSidebar={toggleSidebar}
widgetId={intl.formatMessage(messages.notificationTitle)}
/>
</div>
);
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,18 @@ import { breakpoints } from '@edx/paragon';
import MockAdapter from 'axios-mock-adapter';
import React from 'react';
import { Factory } from 'rosie';
import { initializeMockApp, render, screen } from '../../../../../setupTest';
import initializeStore from '../../../../../store';
import { appendBrowserTimezoneToUrl, executeThunk } from '../../../../../utils';
import { initializeMockApp, render, screen } from '../../../../../../setupTest';
import initializeStore from '../../../../../../store';
import { appendBrowserTimezoneToUrl, executeThunk } from '../../../../../../utils';

import { fetchCourse } from '../../../../data';
import SidebarContext from '../../SidebarContext';
import NotificationTray from './NotificationsSidebar';
import { fetchCourse } from '../../../../../data';
import SidebarContext from '../../../SidebarContext';
import NotificationsWidget from './NotificationsWidget';

initializeMockApp();
jest.mock('@edx/frontend-platform/analytics');

describe('NotificationsSidebar', () => {
describe('NotificationsWidget', () => {
let axiosMock;
let store;
const ID = 'NEWSIDEBAR';
Expand Down Expand Up @@ -57,7 +57,7 @@ describe('NotificationsSidebar', () => {
isNotificationbarAvailable: true,
}}
>
<NotificationTray />
<NotificationsWidget />
</SidebarContext.Provider>,
);
const UpgradeNotification = document.querySelector('.upgrade-notification');
Expand All @@ -81,7 +81,7 @@ describe('NotificationsSidebar', () => {
isNotificationbarAvailable: false,
}}
>
<NotificationTray />
<NotificationsWidget />
</SidebarContext.Provider>,
);
expect(screen.queryByText('Notifications'))
Expand All @@ -101,7 +101,7 @@ describe('NotificationsSidebar', () => {
isNotificationbarAvailable: true,
}}
>
<NotificationTray />
<NotificationsWidget />
</SidebarContext.Provider>,
);
expect(onNotificationSeen).toHaveBeenCalledTimes(0);
Expand Down
7 changes: 3 additions & 4 deletions src/courseware/course/sequence/Sequence.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
sendTrackEvent,
sendTrackingLogEvent,
} from '@edx/frontend-platform/analytics';
import { injectIntl, intlShape } from '@edx/frontend-platform/i18n';
import { useIntl } from '@edx/frontend-platform/i18n';
import { useSelector } from 'react-redux';
import SequenceExamWrapper from '@edx/frontend-lib-special-exams';
import { breakpoints, useWindowSize } from '@edx/paragon';
Expand All @@ -35,8 +35,8 @@ const Sequence = ({
unitNavigationHandler,
nextSequenceHandler,
previousSequenceHandler,
intl,
}) => {
const intl = useIntl();
const course = useModel('coursewareMeta', courseId);
const {
isStaff,
Expand Down Expand Up @@ -227,12 +227,11 @@ Sequence.propTypes = {
unitNavigationHandler: PropTypes.func.isRequired,
nextSequenceHandler: PropTypes.func.isRequired,
previousSequenceHandler: PropTypes.func.isRequired,
intl: intlShape.isRequired,
};

Sequence.defaultProps = {
sequenceId: null,
unitId: null,
};

export default injectIntl(Sequence);
export default Sequence;
12 changes: 5 additions & 7 deletions src/generic/upgrade-notification/UpgradeNotification.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
SupportMissionBullet,
} from '../upsell-bullets/UpsellBullets';
import messages from '../messages';
import { ID } from '../../courseware/course/new-sidebar/sidebars/discussions-notifications/DiscussionsNotificationsTrigger';

const UpsellNoFBECardContent = () => (
<ul className="fa-ul upgrade-notification-ul pt-0">
Expand Down Expand Up @@ -289,8 +290,7 @@ const UpgradeNotification = ({
verifiedMode,
showRemoveIcon,
toggleSidebar,
sidebarId,
tabId,
widgetId,
}) => {
const intl = useIntl();
const dateNow = Date.now();
Expand Down Expand Up @@ -505,7 +505,7 @@ const UpgradeNotification = ({
src={Close}
size="sm"
iconAs={Icon}
onClick={() => toggleSidebar(sidebarId, tabId)}
onClick={() => toggleSidebar(ID, widgetId)}

Check warning on line 508 in src/generic/upgrade-notification/UpgradeNotification.jsx

View check run for this annotation

Codecov / codecov/patch

src/generic/upgrade-notification/UpgradeNotification.jsx#L508

Added line #L508 was not covered by tests
className="icon-hover"
alt={intl.formatMessage(messages.close)}
/>
Expand Down Expand Up @@ -553,8 +553,7 @@ UpgradeNotification.propTypes = {
upgradeUrl: PropTypes.string.isRequired,
}),
showRemoveIcon: PropTypes.bool,
sidebarId: PropTypes.string,
tabId: PropTypes.string,
widgetId: PropTypes.string,
};

UpgradeNotification.defaultProps = {
Expand All @@ -569,8 +568,7 @@ UpgradeNotification.defaultProps = {
verifiedMode: null,
showRemoveIcon: false,
toggleSidebar: null,
sidebarId: null,
tabId: null,
widgetId: null,
};

export default injectIntl(UpgradeNotification);

0 comments on commit c949cd5

Please sign in to comment.