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.

Sign up for GitHub

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: adding a new plugin for the enterprise modal on dashboard #565

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
+59 −427
Open

feat: adding a new plugin for the enterprise modal on dashboard #565

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
46a02dc
feat: adding new plugin slot for an enterprise modal
kiram15 Feb 12, 2025
91b2e77
fix: renaming slot
kiram15 Feb 12, 2025
d2dc28e
fix: pr requests
kiram15 Feb 13, 2025
9f57363
fix: PR requests
kiram15 Feb 18, 2025
528fb79
fix: PR requests
kiram15 Feb 24, 2025
6f3fd02
Merge branch 'master' into kiram15/ENT-9853
kiram15 Feb 26, 2025
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
7 changes: 4 additions & 3 deletions src/containers/Dashboard/__snapshots__/index.test.jsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Dashboard snapshots courses loaded, show select session modal, no available dashboards snapshot 1`] = `
exports[`Dashboard snapshots courses loaded, show select session modal snapshot 1`] = `
<div
className="d-flex flex-column p-2 pt-0"
id="dashboard-container"
Expand All @@ -11,6 +11,7 @@ exports[`Dashboard snapshots courses loaded, show select session modal, no avail
test-page-title
</h1>
<Fragment>
<DashboardModalSlot />
<SelectSessionModal />
</Fragment>
<div
Expand Down Expand Up @@ -43,7 +44,7 @@ exports[`Dashboard snapshots courses still loading snapshot 1`] = `
</div>
`;

exports[`Dashboard snapshots there are no courses, there ARE available dashboards snapshot 1`] = `
exports[`Dashboard snapshots there are no courses snapshot 1`] = `
<div
className="d-flex flex-column p-2 pt-0"
id="dashboard-container"
Expand All @@ -54,7 +55,7 @@ exports[`Dashboard snapshots there are no courses, there ARE available dashboard
test-page-title
</h1>
<Fragment>
<EnterpriseDashboardModal />
<DashboardModalSlot />
</Fragment>
<div
data-testid="dashboard-content"
Expand Down
5 changes: 2 additions & 3 deletions src/containers/Dashboard/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ import React from 'react';

import { reduxHooks } from 'hooks';
import { RequestKeys } from 'data/constants/requests';
import EnterpriseDashboardModal from 'containers/EnterpriseDashboardModal';
import SelectSessionModal from 'containers/SelectSessionModal';
import CoursesPanel from 'containers/CoursesPanel';
import DashboardModalSlot from 'plugin-slots/DashboardModalSlot';

import LoadingView from './LoadingView';
import DashboardLayout from './DashboardLayout';
Expand All @@ -15,7 +15,6 @@ export const Dashboard = () => {
hooks.useInitializeDashboard();
const { pageTitle } = hooks.useDashboardMessages();
const hasCourses = reduxHooks.useHasCourses();
const hasAvailableDashboards = reduxHooks.useHasAvailableDashboards();
const initIsPending = reduxHooks.useRequestIsPending(RequestKeys.initialize);
const showSelectSessionModal = reduxHooks.useShowSelectSessionModal();

Expand All @@ -24,7 +23,7 @@ export const Dashboard = () => {
<h1 className="sr-only">{pageTitle}</h1>
{!initIsPending && (
<>
{hasAvailableDashboards && <EnterpriseDashboardModal />}
<DashboardModalSlot />
{(hasCourses && showSelectSessionModal) && <SelectSessionModal />}
Comment on lines +26 to 27
Copy link
Member

Choose a reason for hiding this comment

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

[consideration/thought question] Would it make sense to co-locate SelectSessionModal as the default modal within DashboardModalSlot? It could be a bit confusing to have default modals outside of DashboardModalSlot (e.g., when should a default dashboard modal be put in this modal-specific slot vs. outside the modal-specific slot?).

If intentional to keep default dashboard modals out of the DashboardModalSlot, perhaps DashboardModalSlot is renamed to DashboardCustomModalSlot to be explicit that its for custom modals, not default modals like SelectSessionModal.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was thinking it'd make sense to have the SelectSessionModal as the default content as well, and it received some agreement during the most recent Frontend Working Group meeting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because we have a customer waiting upon this change, I think this would be a good idea to accomplish as a fast-follow

</>
)}
Expand Down
21 changes: 3 additions & 18 deletions src/containers/Dashboard/index.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { shallow } from '@edx/react-unit-test-utils';

import { reduxHooks } from 'hooks';

import EnterpriseDashboardModal from 'containers/EnterpriseDashboardModal';
import SelectSessionModal from 'containers/SelectSessionModal';
import CoursesPanel from 'containers/CoursesPanel';

Expand All @@ -14,13 +13,12 @@ import Dashboard from '.';
jest.mock('hooks', () => ({
reduxHooks: {
useHasCourses: jest.fn(),
useHasAvailableDashboards: jest.fn(),
useShowSelectSessionModal: jest.fn(),
useRequestIsPending: jest.fn(),
},
}));

jest.mock('containers/EnterpriseDashboardModal', () => 'EnterpriseDashboardModal');
jest.mock('plugin-slots/DashboardModalSlot', () => 'DashboardModalSlot');
jest.mock('containers/CoursesPanel', () => 'CoursesPanel');
jest.mock('./LoadingView', () => 'LoadingView');
jest.mock('./DashboardLayout', () => 'DashboardLayout');
Expand All @@ -38,12 +36,10 @@ describe('Dashboard', () => {
});
const createWrapper = ({
hasCourses,
hasAvailableDashboards,
initIsPending,
showSelectSessionModal,
}) => {
reduxHooks.useHasCourses.mockReturnValueOnce(hasCourses);
reduxHooks.useHasAvailableDashboards.mockReturnValueOnce(hasAvailableDashboards);
reduxHooks.useRequestIsPending.mockReturnValueOnce(initIsPending);
reduxHooks.useShowSelectSessionModal.mockReturnValueOnce(showSelectSessionModal);
return shallow(<Dashboard />);
Expand Down Expand Up @@ -71,7 +67,6 @@ describe('Dashboard', () => {
const testView = ({
props,
content: [contentName, contentEl],
showEnterpriseModal,
showSelectSessionModal,
}) => {
beforeEach(() => { wrapper = createWrapper(props); });
Expand All @@ -80,10 +75,6 @@ describe('Dashboard', () => {
it(`renders ${contentName}`, () => {
testContent(contentEl);
});
it(`${renderString(showEnterpriseModal)} dashbaord modal`, () => {
expect(wrapper.instance.findByType(EnterpriseDashboardModal).length)
.toEqual(showEnterpriseModal ? 1 : 0);
});
it(`${renderString(showSelectSessionModal)} select session modal`, () => {
expect(wrapper.instance.findByType(SelectSessionModal).length).toEqual(showSelectSessionModal ? 1 : 0);
});
Expand All @@ -92,44 +83,38 @@ describe('Dashboard', () => {
testView({
props: {
hasCourses: false,
hasAvailableDashboards: false,
initIsPending: true,
showSelectSessionModal: false,
},
content: ['LoadingView', <LoadingView />],
showEnterpriseModal: false,
showSelectSessionModal: false,
});
});

describe('courses loaded, show select session modal, no available dashboards', () => {
describe('courses loaded, show select session modal', () => {
testView({
props: {
hasCourses: true,
hasAvailableDashboards: false,
initIsPending: false,
showSelectSessionModal: true,
},
content: ['LoadedView', (
<DashboardLayout><CoursesPanel /></DashboardLayout>
)],
showEnterpriseModal: false,
showSelectSessionModal: true,
});
});

describe('there are no courses, there ARE available dashboards', () => {
describe('there are no courses', () => {
testView({
props: {
hasCourses: false,
hasAvailableDashboards: true,
initIsPending: false,
showSelectSessionModal: false,
},
content: ['Dashboard layout with no courses sidebar and content', (
<DashboardLayout><CoursesPanel /></DashboardLayout>
)],
showEnterpriseModal: true,
showSelectSessionModal: false,
});
});
Expand Down
42 changes: 0 additions & 42 deletions src/containers/EnterpriseDashboardModal/__snapshots__/index.test.jsx.snap
View file
Open in desktop

This file was deleted.

48 changes: 0 additions & 48 deletions src/containers/EnterpriseDashboardModal/hooks.js
View file
Open in desktop

This file was deleted.

75 changes: 0 additions & 75 deletions src/containers/EnterpriseDashboardModal/hooks.test.js
View file
Open in desktop

This file was deleted.

60 changes: 0 additions & 60 deletions src/containers/EnterpriseDashboardModal/index.jsx
View file
Open in desktop

This file was deleted.

Loading