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: add library component picker #1356

Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
cdfe3d6
feat: add library component picker
rpenido Oct 10, 2024
df9b518
feat: add api call
rpenido Oct 11, 2024
d8f3d7a
test: add tests
rpenido Oct 11, 2024
7ca8f22
test: more tests
rpenido Oct 11, 2024
45b2f01
test: try to fix flaky test
rpenido Oct 11, 2024
08a7e48
test: improve coverage
rpenido Oct 11, 2024
be2765d
fix: some navigation fixes
rpenido Oct 11, 2024
34f76e6
fix: more fixes
rpenido Oct 11, 2024
de1be9e
test: more coverage
rpenido Oct 11, 2024
81bcd53
refactor: use muteteAsync instead of hooks
rpenido Oct 14, 2024
d1d9c1d
fix: eslint
rpenido Oct 14, 2024
1ce80cb
fix: use routes
rpenido Oct 14, 2024
4f6a485
fix: add arrow icon
rpenido Oct 14, 2024
c569d3e
refactor: use muteteAsync instead of hooks
rpenido Oct 14, 2024
2a0c4ff
fix: make card clickable
rpenido Oct 14, 2024
dbb45b3
fix: add padding
rpenido Oct 14, 2024
6d43547
fix: add useCallback to handleSearch
rpenido Oct 14, 2024
d77a7db
Merge branch 'master' into rpenido/fal-3876-add-library-content-to-a-…
rpenido Oct 15, 2024
16e71fc
fix: eslint
rpenido Oct 15, 2024
3a4497b
fix: issues from merge
rpenido Oct 15, 2024
8431b67
fix: check readOnly on ManageCollections
rpenido Oct 15, 2024
72f7b85
fix: remove min-height style
rpenido Oct 16, 2024
ded2f07
fix: add jsdocs
rpenido Oct 16, 2024
a31836e
fix: set activeKey with navigation
rpenido Oct 16, 2024
fdc54e7
test: fix tests
rpenido Oct 16, 2024
f57efb4
test: fix test
rpenido Oct 16, 2024
9e82795
test: fix test
rpenido Oct 16, 2024
78d25fb
fix: add padding
rpenido Oct 16, 2024
5cc64cb
fix: remove add component when readOnly
rpenido Oct 16, 2024
9709133
chore: remove unused import
bradenmacdonald Oct 16, 2024
46babe0
test: revert change that broke flaky test
bradenmacdonald Oct 16, 2024
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/content-tags-drawer/ContentTagsDrawer.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ jest.mock('react-router-dom', () => ({
const renderDrawer = (contentId, drawerParams = {}) => (
render(
<ContentTagsDrawerSheetContext.Provider value={drawerParams}>
<ContentTagsDrawer canTagObject {...drawerParams} />
<ContentTagsDrawer {...drawerParams} />
</ContentTagsDrawerSheetContext.Provider>,
{ path, params: { contentId } },
)
Expand Down Expand Up @@ -256,7 +256,7 @@ describe('<ContentTagsDrawer />', () => {
])(
'should hide "$editButton" button on $variant variant if not allowed to tag object',
async ({ variant, editButton }) => {
renderDrawer(stagedTagsId, { variant, canTagObject: false });
renderDrawer(stagedTagsId, { variant, readOnly: true });
expect(await screen.findByText('Taxonomy 1')).toBeInTheDocument();

expect(screen.queryByRole('button', { name: editButton })).not.toBeInTheDocument();
Expand Down
40 changes: 21 additions & 19 deletions src/content-tags-drawer/ContentTagsDrawer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,10 @@ const ContentTagsDrawerTitle = () => {

interface ContentTagsDrawerVariantFooterProps {
onClose: () => void,
canTagObject: boolean,
readOnly: boolean,
}

const ContentTagsDrawerVariantFooter = ({ onClose, canTagObject }: ContentTagsDrawerVariantFooterProps) => {
const ContentTagsDrawerVariantFooter = ({ onClose, readOnly }: ContentTagsDrawerVariantFooterProps) => {
const intl = useIntl();
const {
commitGlobalStagedTagsStatus,
Expand Down Expand Up @@ -131,7 +131,7 @@ const ContentTagsDrawerVariantFooter = ({ onClose, canTagObject }: ContentTagsDr
? messages.tagsDrawerCancelButtonText
: messages.tagsDrawerCloseButtonText)}
</Button>
{canTagObject && (
{!readOnly && (
<Button
className="rounded-0"
onClick={isEditMode
Expand All @@ -157,7 +157,11 @@ const ContentTagsDrawerVariantFooter = ({ onClose, canTagObject }: ContentTagsDr
);
};

const ContentTagsComponentVariantFooter = ({ canTagObject }: { canTagObject: boolean }) => {
interface ContentTagsComponentVariantFooterProps {
readOnly?: boolean;
}

const ContentTagsComponentVariantFooter = ({ readOnly = false }: ContentTagsComponentVariantFooterProps) => {
const intl = useIntl();
const {
commitGlobalStagedTagsStatus,
Expand Down Expand Up @@ -198,16 +202,14 @@ const ContentTagsComponentVariantFooter = ({ canTagObject }: { canTagObject: boo
</div>
)}
</div>
) : (
canTagObject && (
<Button
variant="outline-primary"
onClick={toEditMode}
block
>
{intl.formatMessage(messages.manageTagsButton)}
</Button>
)
) : !readOnly && (
<Button
variant="outline-primary"
onClick={toEditMode}
block
>
{intl.formatMessage(messages.manageTagsButton)}
</Button>
)}
</div>
);
Expand All @@ -216,8 +218,8 @@ const ContentTagsComponentVariantFooter = ({ canTagObject }: { canTagObject: boo
interface ContentTagsDrawerProps {
id?: string;
onClose?: () => void;
canTagObject?: boolean;
variant?: 'drawer' | 'component';
readOnly?: boolean;
}

/**
Expand All @@ -232,8 +234,8 @@ interface ContentTagsDrawerProps {
const ContentTagsDrawer = ({
id,
onClose,
canTagObject = false,
variant = 'drawer',
readOnly = false,
}: ContentTagsDrawerProps) => {
const intl = useIntl();
// TODO: We can delete 'params' when the iframe is no longer used on edx-platform
Expand All @@ -244,7 +246,7 @@ const ContentTagsDrawer = ({
throw new Error('Error: contentId cannot be null.');
}

const context = useContentTagsDrawerContext(contentId, canTagObject);
const context = useContentTagsDrawerContext(contentId, !readOnly);
const { blockingSheet } = useContext(ContentTagsDrawerSheetContext);

const {
Expand Down Expand Up @@ -308,9 +310,9 @@ const ContentTagsDrawer = ({
if (isTaxonomyListLoaded && isContentTaxonomyTagsLoaded) {
switch (variant) {
case 'drawer':
return <ContentTagsDrawerVariantFooter onClose={onCloseDrawer} canTagObject={canTagObject} />;
return <ContentTagsDrawerVariantFooter onClose={onCloseDrawer} readOnly={readOnly} />;
case 'component':
return <ContentTagsComponentVariantFooter canTagObject={canTagObject} />;
return <ContentTagsComponentVariantFooter readOnly={readOnly} />;
default:
return null;
}
Expand Down
4 changes: 2 additions & 2 deletions src/content-tags-drawer/ContentTagsDrawerSheet.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const ContentTagsDrawerSheet = ({ id, onClose, showSheet }) => {

// ContentTagsDrawerSheet is only used when editing Courses/Course Units,
// so we assume it's ok to edit the object tags too.
const canTagObject = true;
const readOnly = false;

return (
<ContentTagsDrawerSheetContext.Provider value={context}>
Expand All @@ -27,7 +27,7 @@ const ContentTagsDrawerSheet = ({ id, onClose, showSheet }) => {
<ContentTagsDrawer
id={id}
onClose={onClose}
canTagObject={canTagObject}
readOnly={readOnly}
/>
</Sheet>
</ContentTagsDrawerSheetContext.Provider>
Expand Down
17 changes: 12 additions & 5 deletions src/course-outline/page-alerts/PageAlerts.test.jsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import React from 'react';
import { useSelector } from 'react-redux';
import { act, render, fireEvent } from '@testing-library/react';
import {
act,
render,
fireEvent,
waitFor,
} from '@testing-library/react';
import { IntlProvider } from '@edx/frontend-platform/i18n';
import { AppProvider } from '@edx/frontend-platform/react';
import { initializeMockApp, getConfig } from '@edx/frontend-platform';
Expand Down Expand Up @@ -84,7 +89,7 @@ describe('<PageAlerts />', () => {
});

it('renders discussion alerts', async () => {
const { queryByText } = renderComponent({
const { getByText, queryByText } = renderComponent({
...pageAlertsData,
discussionsSettings: {
providerType: 'openedx',
Expand All @@ -103,9 +108,11 @@ describe('<PageAlerts />', () => {
const discussionAlertDismissKey = `discussionAlertDismissed-${pageAlertsData.courseId}`;
expect(localStorage.getItem(discussionAlertDismissKey)).toBe('true');

const feedbackLink = queryByText(messages.discussionNotificationFeedback.defaultMessage);
expect(feedbackLink).toBeInTheDocument();
expect(feedbackLink).toHaveAttribute('href', 'some-feedback-url');
await waitFor(() => {
const feedbackLink = getByText(messages.discussionNotificationFeedback.defaultMessage);
expect(feedbackLink);
expect(feedbackLink).toHaveAttribute('href', 'some-feedback-url');
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was a flaky test

Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: you could just write const feedbackLink = await findByText(... instead of using await waitFor(() => {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here: 9e82795

});

it('renders deprecation warning alerts', async () => {
Expand Down
78 changes: 72 additions & 6 deletions src/generic/block-type-utils/index.scss
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
.component-style-default {
background-color: #005C9E;

.pgn__icon {
.pgn__icon:not(.btn-icon-before) {
color: white;
}

Expand All @@ -10,12 +10,23 @@
background-color: darken(#005C9E, 15%);
}
}

.btn {
background-color: lighten(#005C9E, 10%);
border: 0;

&:hover, &:active, &:focus {
background-color: lighten(#005C9E, 20%);
border: 1px solid $primary;
margin: -1px;
}
}
}

.component-style-html {
background-color: #9747FF;

.pgn__icon {
.pgn__icon:not(.btn-icon-before) {
color: white;
}

Expand All @@ -24,12 +35,23 @@
background-color: darken(#9747FF, 15%);
}
}

.btn {
background-color: lighten(#9747FF, 10%);
border: 0;

&:hover, &:active, &:focus {
background-color: lighten(#9747FF, 20%);
border: 1px solid $primary;
margin: -1px;
}
}
}

.component-style-collection {
background-color: #FFCD29;

.pgn__icon {
.pgn__icon:not(.btn-icon-before) {
color: black;
}

Expand All @@ -38,12 +60,23 @@
background-color: darken(#FFCD29, 15%);
}
}

.btn {
background-color: lighten(#FFCD29, 10%);
border: 0;

&:hover, &:active, &:focus {
background-color: lighten(#FFCD29, 20%);
border: 1px solid $primary;
margin: -1px;
}
}
}

.component-style-video {
background-color: #358F0A;

.pgn__icon {
.pgn__icon:not(.btn-icon-before) {
color: white;
}

Expand All @@ -52,12 +85,23 @@
background-color: darken(#358F0A, 15%);
}
}

.btn {
background-color: lighten(#358F0A, 10%);
border: 0;

&:hover, &:active, &:focus {
background-color: lighten(#358F0A, 20%);
border: 1px solid $primary;
margin: -1px;
}
}
}

.component-style-vertical {
background-color: #0B8E77;

.pgn__icon {
.pgn__icon:not(.btn-icon-before) {
color: white;
}

Expand All @@ -66,12 +110,23 @@
background-color: darken(#0B8E77, 15%);
}
}

.btn {
background-color: lighten(#0B8E77, 10%);
border: 0;

&:hover, &:active, &:focus {
background-color: lighten(#0B8E77, 20%);
border: 1px solid $primary;
margin: -1px;
}
}
}

.component-style-other {
background-color: #646464;

.pgn__icon {
.pgn__icon:not(.btn-icon-before) {
color: white;
}

Expand All @@ -80,4 +135,15 @@
background-color: darken(#646464, 15%);
}
}

.btn {
background-color: lighten(#646464, 10%);
border: 0;

&:hover, &:active, &:focus {
background-color: lighten(#646464, 20%);
border: 1px solid $primary;
margin: -1px;
}
}
}
3 changes: 2 additions & 1 deletion src/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { initializeHotjar } from '@edx/frontend-enterprise-hotjar';
import { logError } from '@edx/frontend-platform/logging';
import messages from './i18n';

import { CreateLibrary, LibraryLayout } from './library-authoring';
import { ComponentPicker, CreateLibrary, LibraryLayout } from './library-authoring';
import initializeStore from './store';
import CourseAuthoringRoutes from './CourseAuthoringRoutes';
import Head from './head/Head';
Expand Down Expand Up @@ -55,6 +55,7 @@ const App = () => {
<Route path="/libraries-v1" element={<StudioHome />} />
<Route path="/library/create" element={<CreateLibrary />} />
<Route path="/library/:libraryId/*" element={<LibraryLayout />} />
<Route path="/component-picker" element={<ComponentPicker />} />
<Route path="/course/:courseId/*" element={<CourseAuthoringRoutes />} />
<Route path="/course_rerun/:courseId" element={<CourseRerun />} />
{getConfig().ENABLE_ACCESSIBILITY_PAGE === 'true' && (
Expand Down
2 changes: 1 addition & 1 deletion src/library-authoring/LibraryAuthoringPage.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -519,7 +519,7 @@ describe('<LibraryAuthoringPage />', () => {
expect(showProbTypesSubmenuBtn).not.toBeNull();
fireEvent.click(showProbTypesSubmenuBtn!);

const validateSubmenu = async (submenuText : string) => {
const validateSubmenu = async (submenuText: string) => {
const submenu = screen.getByText(submenuText);
expect(submenu).toBeInTheDocument();
fireEvent.click(submenu);
Expand Down
Loading