diff --git a/localdev/configs/ocw-course-site-config.yml b/localdev/configs/ocw-course-site-config.yml index 842a87475..b8959c32f 100644 --- a/localdev/configs/ocw-course-site-config.yml +++ b/localdev/configs/ocw-course-site-config.yml @@ -14,7 +14,11 @@ collections: - label: Body name: body widget: markdown - attach: resource + link: + - resource + - page + embed: + - resource - category: Content folder: content/video_galleries diff --git a/localdev/configs/ocw-www.yml b/localdev/configs/ocw-www.yml index 393112a66..fb8a26ab0 100644 --- a/localdev/configs/ocw-www.yml +++ b/localdev/configs/ocw-www.yml @@ -14,6 +14,9 @@ collections: - label: Body name: body widget: markdown + link: + - course_collections + - resource_collections - category: Content folder: content/resource_collections diff --git a/static/js/components/widgets/MarkdownEditor.test.tsx b/static/js/components/widgets/MarkdownEditor.test.tsx index 71909b991..d592ec3f0 100644 --- a/static/js/components/widgets/MarkdownEditor.test.tsx +++ b/static/js/components/widgets/MarkdownEditor.test.tsx @@ -21,7 +21,8 @@ jest.mock("@ckeditor/ckeditor5-react", () => ({ CKEditor: () =>
})) -const render = (props = {}) => shallow() +const render = (props = {}) => + shallow() describe("MarkdownEditor", () => { let sandbox: SinonSandbox @@ -49,7 +50,8 @@ describe("MarkdownEditor", () => { const wrapper = render({ minimal, value, - attach: "attach" + embed: ["resource"], + link: ["page"] }) const ckWrapper = wrapper.find("CKEditor") expect(ckWrapper.prop("editor")).toBe(ClassicEditor) @@ -61,17 +63,19 @@ describe("MarkdownEditor", () => { }) }) - it("should render ResourceEmbedField if attach is provided", () => { - const wrapper = render({ attach: "foo" }) - const diaglogWrapper = wrapper.find("ResourcePickerDialog") - expect(diaglogWrapper.length).toBe(1) - }) - - it("should not render ResourceEmbedField if attach is missing", () => { - const wrapper = render() - const diaglogWrapper = wrapper.find("ResourcePickerDialog") - expect(diaglogWrapper.length).toBe(0) - }) + it.each([ + { link: [], embed: [], shouldExist: false }, + { link: ["page"], embed: [], shouldExist: true }, + { link: [], embed: ["resource"], shouldExist: true }, + { link: ["page"], embed: ["resource"], shouldExist: true } + ])( + "should render ResourcePickerDialog iff link or embed are nonempty", + ({ link, embed, shouldExist }) => { + const wrapper = render({ link, embed }) + const resourcePicker = wrapper.find("ResourcePickerDialog") + expect(resourcePicker.exists()).toBe(shouldExist) + } + ) it("should render resources with using EmbeddedResource", () => { const wrapper = render() @@ -88,30 +92,43 @@ describe("MarkdownEditor", () => { ).toEqual("resource-uuid") }) - // - ;[true, false].forEach(hasAttach => { - it(`${ - hasAttach ? "should" : "shouldn't" - } have an add resource button since attach ${ - hasAttach ? "was" : "wasn't" - } set`, () => { - const wrapper = render(hasAttach ? { attach: "resource" } : {}) + it.each([ + { embed: [], hasTool: false }, + { embed: ["resource"], hasTool: true } + ])( + 'should show "add resource" iff embed is nonempty. Case: $embed', + ({ embed, hasTool }) => { + const wrapper = render({ embed }) const editorConfig = wrapper.find("CKEditor").prop("config") // @ts-ignore expect(editorConfig.toolbar.items.includes(ADD_RESOURCE_EMBED)).toBe( - hasAttach + hasTool ) + } + ) + + it.each([ + { link: [], hasTool: false }, + { link: ["page"], hasTool: true } + ])( + 'should show "add link" iff link is nonempty. Case: $link', + ({ link, hasTool }) => { + const wrapper = render({ link }) + const editorConfig = wrapper.find("CKEditor").prop("config") // @ts-ignore expect(editorConfig.toolbar.items.includes(ADD_RESOURCE_LINK)).toBe( - hasAttach + hasTool ) - }) - }) + } + ) // ;[RESOURCE_EMBED, RESOURCE_LINK].forEach(resourceNodeType => { it(`should open the resource picker for ${resourceNodeType}`, () => { - const wrapper = render({ attach: "resource" }) + const wrapper = render({ + embed: ["resource"], + link: ["resource", "page"] + }) const editor = wrapper.find("CKEditor").prop("config") // @ts-ignore editor[CKEDITOR_RESOURCE_UTILS].openResourcePicker(resourceNodeType) diff --git a/static/js/components/widgets/MarkdownEditor.tsx b/static/js/components/widgets/MarkdownEditor.tsx index f0b42a672..66ac08b63 100644 --- a/static/js/components/widgets/MarkdownEditor.tsx +++ b/static/js/components/widgets/MarkdownEditor.tsx @@ -26,7 +26,8 @@ export interface Props { onChange?: (event: { target: { value: string; name: string } }) => void children?: React.ReactNode minimal?: boolean - attach?: string + embed: string[] + link: string[] } type RenderQueueEntry = [string, HTMLElement] @@ -37,7 +38,7 @@ type RenderQueueEntry = [string, HTMLElement] * pass minimal: true to get a minimal version. */ export default function MarkdownEditor(props: Props): JSX.Element { - const { attach, value, name, onChange, minimal } = props + const { link, embed, value, name, onChange, minimal } = props const editor = useRef() const setEditorRef = useCallback(editorInstance => { @@ -85,8 +86,6 @@ export default function MarkdownEditor(props: Props): JSX.Element { [setResourcePickerMode, setIsResourcePickerOpen] ) - const hasAttach = attach && attach.length > 0 - const editorConfig = useMemo(() => { if (minimal) { return MinimalEditorConfig @@ -102,15 +101,19 @@ export default function MarkdownEditor(props: Props): JSX.Element { }, toolbar: { ...FullEditorConfig.toolbar, - items: FullEditorConfig.toolbar.items.filter(item => - hasAttach ? - true : - item !== ADD_RESOURCE_LINK && item !== ADD_RESOURCE_EMBED - ) + items: FullEditorConfig.toolbar.items.filter(item => { + if (item === ADD_RESOURCE_LINK) { + return link.length > 0 + } + if (item === ADD_RESOURCE_EMBED) { + return embed.length > 0 + } + return true + }) } } } - }, [minimal, renderResource, openResourcePicker, hasAttach]) + }, [minimal, renderResource, openResourcePicker, link, embed]) const onChangeCB = useCallback( (_event: any, editor: any) => { @@ -145,14 +148,15 @@ export default function MarkdownEditor(props: Props): JSX.Element { onReady={setEditorRef} onChange={onChangeCB} /> - {hasAttach ? ( + {(link.length > 0 || embed.length > 0) && ( - ) : null} + )} {renderQueue.map(([uuid, el], idx) => ( ))} diff --git a/static/js/components/widgets/ResourcePickerDialog.test.tsx b/static/js/components/widgets/ResourcePickerDialog.test.tsx index 3580733f2..063f6ca7d 100644 --- a/static/js/components/widgets/ResourcePickerDialog.test.tsx +++ b/static/js/components/widgets/ResourcePickerDialog.test.tsx @@ -1,6 +1,6 @@ import React from "react" import { act } from "react-dom/test-utils" -import { TabPane, NavLink, Dropdown, DropdownItem } from "reactstrap" +import { TabPane, NavLink } from "reactstrap" import { ReactWrapper } from "enzyme" import ResourcePickerDialog, { TabIds } from "./ResourcePickerDialog" @@ -16,6 +16,7 @@ import { RESOURCE_LINK } from "../../lib/ckeditor/plugins/constants" import { ResourceType } from "../../constants" +import Dialog from "../Dialog" jest.mock("../../hooks/state") @@ -57,10 +58,11 @@ describe("ResourcePickerDialog", () => { useDebouncedState.mockReturnValue(["", setStub]) render = helper.configureRenderer(ResourcePickerDialog, { - mode: RESOURCE_EMBED, - isOpen: true, - closeDialog: closeDialogStub, - insertEmbed: insertEmbedStub + mode: RESOURCE_EMBED, + contentNames: ["resource", "page"], + isOpen: true, + closeDialog: closeDialogStub, + insertEmbed: insertEmbedStub }) }) @@ -68,44 +70,45 @@ describe("ResourcePickerDialog", () => { helper.cleanup() }) - it("should render 3 tabs when embedding", async () => { - const { wrapper } = await render({ mode: RESOURCE_EMBED }) - expect(wrapper.find(TabPane).map(pane => pane.prop("tabId"))).toEqual([ - TabIds.Documents, - TabIds.Videos, - TabIds.Images - ]) - }) - - it("should render 6 tabs when linking", async () => { - const { wrapper } = await render({ mode: RESOURCE_LINK }) - expect(wrapper.find(TabPane).map(pane => pane.prop("tabId"))).toEqual([ - TabIds.Documents, - TabIds.Videos, - TabIds.Images, - TabIds.Pages, - TabIds.CourseCollections, - TabIds.ResourceCollections - ]) - }) + it.each([ + { + contentNames: ["resource"], + expectedTabs: [TabIds.Documents, TabIds.Videos, TabIds.Images] + }, + { contentNames: ["page"], expectedTabs: [TabIds.Pages] }, + { + contentNames: ["course_collections"], + expectedTabs: [TabIds.CourseCollections] + }, + { + contentNames: ["resource_collections"], + expectedTabs: [TabIds.ResourceCollections] + }, + { + contentNames: ["resource", "page", "course_collections"], + expectedTabs: [ + TabIds.Documents, + TabIds.Videos, + TabIds.Images, + TabIds.Pages, + TabIds.CourseCollections + ] + } + ])( + "should render tabs based on contentNames. Case: $contentNames", + async ({ contentNames, expectedTabs }) => { + const { wrapper } = await render({ contentNames, expectedTabs }) + expect(wrapper.find(TabPane).map(pane => pane.prop("tabId"))).toEqual( + expectedTabs + ) + } + ) test("TabIds values are unique", () => { const uniqueTabIds = new Set(Object.values(TabIds)) expect(Object.values(TabIds).length).toBe(uniqueTabIds.size) }) - it.each([ - { mode: RESOURCE_LINK, dropdownExists: true, should: "should" }, - { mode: RESOURCE_EMBED, dropdownExists: false, should: "should not" } - ])( - 'when in mode "$mode", $should show the "more" dropdown', - async ({ mode, dropdownExists }) => { - const { wrapper } = await render({ mode }) - const dropdown = wrapper.find(Dropdown) - expect(dropdown.exists()).toBe(dropdownExists) - } - ) - it("should pass some basic props down to the dialog", async () => { const { wrapper } = await render() const dialog = wrapper.find("Dialog") @@ -113,65 +116,37 @@ describe("ResourcePickerDialog", () => { expect(dialog.prop("wrapClassName")).toBe("resource-picker-dialog") }) - it("should allow focusing and linking a resource, then close the dialog", async () => { - const { wrapper } = await render({ - mode: RESOURCE_LINK - }) - // callback should be 'undefined' before resource is focused - expect(wrapper.find("Dialog").prop("onAccept")).toBeUndefined() - focusResource(wrapper, resource) - - expect(wrapper.find("Dialog").prop("acceptText")).toBe("Add link") - - act(() => { - // @ts-ignore - wrapper.find("Dialog").prop("onAccept")() - }) - - wrapper.update() - - expect(insertEmbedStub.args[0]).toStrictEqual([ - resource.text_id, - resource.title, - RESOURCE_LINK - ]) - expect(closeDialogStub.callCount).toBe(1) - }) - - it("should focusing and embedding a resource", async () => { - const { wrapper } = await render({ - mode: RESOURCE_EMBED - }) - // callback should be 'undefined' before resource is focused - expect(wrapper.find("Dialog").prop("onAccept")).toBeUndefined() - focusResource(wrapper, resource) - - expect(wrapper.find("Dialog").prop("acceptText")).toBe("Embed resource") + it.each([ + { mode: RESOURCE_LINK, attaching: "linking", acceptText: "Add link" }, + { + mode: RESOURCE_EMBED, + attaching: "embedding", + acceptText: "Embed resource" + } + ])( + "should allow focusing and $attaching a resource, then close the dialog", + async ({ mode, acceptText }) => { + const { wrapper } = await render({ mode }) + // callback should be 'undefined' before resource is focused + expect(wrapper.find(Dialog).prop("onAccept")).toBeUndefined() + focusResource(wrapper, resource) - act(() => { - // @ts-ignore - wrapper.find("Dialog").prop("onAccept")() - }) + expect(wrapper.find(Dialog).prop("acceptText")).toBe(acceptText) - wrapper.update() + act(() => { + wrapper.find(Dialog).prop("onAccept")?.() + }) - expect(insertEmbedStub.args[0]).toStrictEqual([ - resource.text_id, - resource.title, - RESOURCE_EMBED - ]) - }) + wrapper.update() - it("should pass basic props to ResourcePickerListing", async () => { - const { wrapper } = await render() - expect(wrapper.find("ResourcePickerListing").prop("contentType")).toEqual( - "resource" - ) - focusResource(wrapper, resource) - expect(wrapper.find("ResourcePickerListing").prop("focusedResource")).toBe( - resource - ) - }) + expect(insertEmbedStub.args[0]).toStrictEqual([ + resource.text_id, + resource.title, + mode + ]) + expect(closeDialogStub.callCount).toBe(1) + } + ) it.each([ { @@ -211,37 +186,6 @@ describe("ResourcePickerDialog", () => { } ) - it.each([ - { - index: 0, - resourcetype: null, - contentType: "course_collections", - sourceWebsiteName: "ocw-www" - }, - { - index: 1, - resourcetype: null, - contentType: "resource_collections", - sourceWebsiteName: "ocw-www" - } - ])( - "passes the correct resourcetype and contentType when dropdown tab $index is clicked", - async ({ resourcetype, contentType, sourceWebsiteName, index }) => { - const { wrapper } = await render({ mode: RESOURCE_LINK }) - act(() => { - wrapper - .find(DropdownItem) - .at(index) - .simulate("click") - }) - wrapper.update() - const listing = wrapper.find(ResourcePickerListing) - expect(listing.prop("resourcetype")).toEqual(resourcetype) - expect(listing.prop("contentType")).toBe(contentType) - expect(listing.prop("sourceWebsiteName")).toBe(sourceWebsiteName) - } - ) - it("should pass filter string to picker, when filter is set", async () => { const setStub = helper.sandbox.stub() // @ts-ignore diff --git a/static/js/components/widgets/ResourcePickerDialog.tsx b/static/js/components/widgets/ResourcePickerDialog.tsx index c9fbe7c59..8c37a8d43 100644 --- a/static/js/components/widgets/ResourcePickerDialog.tsx +++ b/static/js/components/widgets/ResourcePickerDialog.tsx @@ -1,15 +1,5 @@ import React, { SyntheticEvent, useCallback, useState } from "react" -import { - Nav, - NavItem, - NavLink, - TabContent, - TabPane, - Dropdown, - DropdownItem, - DropdownToggle, - DropdownMenu -} from "reactstrap" +import { Nav, NavItem, NavLink, TabContent, TabPane } from "reactstrap" import Dialog from "../Dialog" import { ResourceType, ContentType } from "../../constants" @@ -28,9 +18,10 @@ interface Props { isOpen: boolean closeDialog: () => void insertEmbed: (id: string, title: string, variant: CKEResourceNodeType) => void + contentNames: string[] } -interface ResourceTabSettings { +interface TabSettings { title: string id: TabIds contentType: ContentType @@ -49,64 +40,67 @@ export enum TabIds { ResourceCollections = "resource_collections" } -const RESOURCE_PICKER_FULL_TABS: ResourceTabSettings[] = [ - { - title: "Documents", - id: TabIds.Documents, - contentType: ContentType.Resource, - resourcetype: ResourceType.Document, - embeddable: true, - singleColumn: true - }, - { - title: "Videos", - id: TabIds.Videos, - contentType: ContentType.Resource, - resourcetype: ResourceType.Video, - embeddable: true, - singleColumn: false - }, - { - title: "Images", - id: TabIds.Images, - contentType: ContentType.Resource, - resourcetype: ResourceType.Image, - embeddable: true, - singleColumn: false - }, - { - title: "Pages", - id: TabIds.Pages, - contentType: ContentType.Page, - resourcetype: null, - embeddable: false, - singleColumn: true - } -] -const RESOURCE_PICKER_DROPDOWN_TABS: ResourceTabSettings[] = [ - { - title: "Course Collections", - id: TabIds.CourseCollections, - contentType: ContentType.CourseCollections, - resourcetype: null, - embeddable: false, - singleColumn: true, - sourceWebsiteName: "ocw-www" - }, - { - title: "Resource Collections", - id: TabIds.ResourceCollections, - contentType: ContentType.ResourceCollections, - resourcetype: null, - embeddable: false, - singleColumn: true, - sourceWebsiteName: "ocw-www" - } -] -const RESOURCE_PICKER_ALL_TABS = [ - ...RESOURCE_PICKER_FULL_TABS, - ...RESOURCE_PICKER_DROPDOWN_TABS -] +const documentTab: TabSettings = { + title: "Documents", + id: TabIds.Documents, + contentType: ContentType.Resource, + resourcetype: ResourceType.Document, + embeddable: true, + singleColumn: true +} +const videoTab: TabSettings = { + title: "Videos", + id: TabIds.Videos, + contentType: ContentType.Resource, + resourcetype: ResourceType.Video, + embeddable: true, + singleColumn: false +} +const imageTab: TabSettings = { + title: "Images", + id: TabIds.Images, + contentType: ContentType.Resource, + resourcetype: ResourceType.Image, + embeddable: true, + singleColumn: false +} +const pageTab: TabSettings = { + title: "Pages", + id: TabIds.Pages, + contentType: ContentType.Page, + resourcetype: null, + embeddable: false, + singleColumn: true +} +const courseCollectionTab: TabSettings = { + title: "Course Collections", + id: TabIds.CourseCollections, + contentType: ContentType.CourseCollections, + resourcetype: null, + embeddable: false, + singleColumn: true, + sourceWebsiteName: "ocw-www" +} +const resourcCollectionTab: TabSettings = { + title: "Resource Collections", + id: TabIds.ResourceCollections, + contentType: ContentType.ResourceCollections, + resourcetype: null, + embeddable: false, + singleColumn: true +} + +/** + * Maps a content name from our config schemas to a group of tabs. + * Slightly awkward because most content types correspond to a single tab, + * except resources, which have separate tabs according to resourcetype + */ +const TABS: Record = { + resource: [documentTab, videoTab, imageTab], + page: [pageTab], + course_collections: [courseCollectionTab], + resource_collections: [resourcCollectionTab] +} const modeText = { [RESOURCE_EMBED]: { @@ -119,14 +113,11 @@ const modeText = { } } -const isTabEnabled = (mode: ResourceDialogMode) => (tab: ResourceTabSettings) => - mode !== RESOURCE_EMBED || tab.embeddable - export default function ResourcePickerDialog(props: Props): JSX.Element { - const { mode, isOpen, closeDialog, insertEmbed } = props + const { mode, isOpen, closeDialog, insertEmbed, contentNames } = props - const [activeTabId, setActiveTabId] = useState(TabIds.Images) - const [isDropdownOpen, setIsDropdownOpen] = useState(false) + const tabs = contentNames.flatMap(name => TABS[name]) + const [activeTabId, setActiveTabId] = useState(tabs[0].id) // filterInput is to store user input and is updated synchronously // so that the UI stays responsive @@ -159,25 +150,20 @@ export default function ResourcePickerDialog(props: Props): JSX.Element { } }, [insertEmbed, focusedResource, closeDialog, isOpen, mode]) - const allTabs = RESOURCE_PICKER_ALL_TABS.filter(isTabEnabled(mode)) - const fullTabs = RESOURCE_PICKER_FULL_TABS.filter(isTabEnabled(mode)) - const dropdownTabs = RESOURCE_PICKER_DROPDOWN_TABS.filter(isTabEnabled(mode)) const { acceptText, title } = modeText[mode] - const activeTab = allTabs.find(t => t.id === activeTabId) - const headerText = `${title}: ${activeTab?.title}` return ( - {allTabs.map(tab => ( + {tabs.map(tab => ( {activeTabId === tab.id ? ( { }) describe("widgetExtraProps", () => { - it("should grab the minimal prop for a markdown widget", () => { + it("should grab the markdown props for a markdown widget", () => { const field = makeWebsiteConfigField({ widget: WidgetVariant.Markdown, minimal: true, - attach: "resource" + link: ["resource", "page"], + embed: ["resource"] }) expect(widgetExtraProps(field)).toStrictEqual({ minimal: true, - attach: "resource" + link: ["resource", "page"], + embed: ["resource"] }) }) diff --git a/static/js/lib/site_content.ts b/static/js/lib/site_content.ts index f609c9727..c7dd8852c 100644 --- a/static/js/lib/site_content.ts +++ b/static/js/lib/site_content.ts @@ -97,7 +97,8 @@ export function widgetExtraProps(field: ConfigField): Record { case WidgetVariant.Markdown: return { minimal: field.minimal ?? false, - attach: field.attach ?? undefined + link: field.link ?? [], + embed: field.embed ?? [] } case WidgetVariant.Relation: return pick(RELATION_EXTRA_PROPS, field) diff --git a/static/js/pages/MarkdownEditorTestPage.tsx b/static/js/pages/MarkdownEditorTestPage.tsx index 5960b889c..a1f586d8f 100644 --- a/static/js/pages/MarkdownEditorTestPage.tsx +++ b/static/js/pages/MarkdownEditorTestPage.tsx @@ -62,6 +62,8 @@ function MarkdownEditorTestWrapper(props: Props) { name="markdown" onChange={(event: any) => setData(event.target.value)} minimal={minimal} + embed={[]} + link={[]} />
diff --git a/static/js/resources/ocw-course-site-config.json b/static/js/resources/ocw-course-site-config.json index 827296a69..f3c20fc50 100644 --- a/static/js/resources/ocw-course-site-config.json +++ b/static/js/resources/ocw-course-site-config.json @@ -10,8 +10,14 @@ "widget": "string" }, { - "attach": "resource", + "embed": [ + "resource" + ], "label": "Body", + "link": [ + "resource", + "page" + ], "name": "body", "widget": "markdown" } diff --git a/static/js/resources/ocw-www.json b/static/js/resources/ocw-www.json index 5cc23db10..d4cc96995 100644 --- a/static/js/resources/ocw-www.json +++ b/static/js/resources/ocw-www.json @@ -11,6 +11,10 @@ }, { "label": "Body", + "link": [ + "course_collections", + "resource_collections" + ], "name": "body", "widget": "markdown" } diff --git a/static/js/types/websites.ts b/static/js/types/websites.ts index d20864402..a5ad5a620 100644 --- a/static/js/types/websites.ts +++ b/static/js/types/websites.ts @@ -50,7 +50,8 @@ interface ConfigFieldBaseProps { export interface MarkdownConfigField extends ConfigFieldBaseProps { widget: WidgetVariant.Markdown minimal?: boolean - attach?: string + link?: string[] + embed?: string[] } export interface FileConfigField extends ConfigFieldBaseProps { diff --git a/websites/config_schema/site-config-schema.yml b/websites/config_schema/site-config-schema.yml index 269e6457d..b84f5d0f4 100644 --- a/websites/config_schema/site-config-schema.yml +++ b/websites/config_schema/site-config-schema.yml @@ -38,7 +38,8 @@ field: collections: list(str(), required=False) filter: include('relation_filter', required=False) website: str(required=False) - attach: str(required=False) + link: list(str(), required=False) + embed: list(str(), required=False) levels: list(include('level'), required=False) sortable: bool(required=False) cross_site: bool(required=False)