diff --git a/codecov.yml b/codecov.yml index 64b8f8001..3202455e6 100644 --- a/codecov.yml +++ b/codecov.yml @@ -10,4 +10,5 @@ coverage: threshold: 0% ignore: - "src/grading-settings/grading-scale/react-ranger.js" + - "src/generic/DraggableList/verticalSortableList.ts" - "src/index.js" diff --git a/src/content-tags-drawer/ContentTagsDrawer.test.jsx b/src/content-tags-drawer/ContentTagsDrawer.test.jsx index 000badea9..5035b2fe4 100644 --- a/src/content-tags-drawer/ContentTagsDrawer.test.jsx +++ b/src/content-tags-drawer/ContentTagsDrawer.test.jsx @@ -21,6 +21,7 @@ const path = '/content/:contentId?/*'; const mockOnClose = jest.fn(); const mockSetBlockingSheet = jest.fn(); const mockNavigate = jest.fn(); +const mockSidebarAction = jest.fn(); mockContentTaxonomyTagsData.applyMock(); mockTaxonomyListData.applyMock(); mockTaxonomyTagsData.applyMock(); @@ -40,6 +41,11 @@ jest.mock('react-router-dom', () => ({ useNavigate: () => mockNavigate, })); +jest.mock('../library-authoring/common/context/SidebarContext', () => ({ + ...jest.requireActual('../library-authoring/common/context/SidebarContext'), + useSidebarContext: () => ({ sidebarAction: mockSidebarAction() }), +})); + const renderDrawer = (contentId, drawerParams = {}) => ( render( @@ -184,6 +190,26 @@ describe('', () => { expect(screen.getByRole('button', { name: /save/i })).toBeInTheDocument(); }); + it('should change to edit mode sidebar action is set to JumpToManageTags', async () => { + mockSidebarAction.mockReturnValueOnce('jump-to-manage-tags'); + renderDrawer(stagedTagsId, { variant: 'component' }); + expect(await screen.findByText('Taxonomy 1')).toBeInTheDocument(); + + // Show delete tag buttons + expect(screen.getAllByRole('button', { + name: /delete/i, + }).length).toBe(2); + + // Show add a tag select + expect(screen.getByText(/add a tag/i)).toBeInTheDocument(); + + // Show cancel button + expect(screen.getByRole('button', { name: /cancel/i })).toBeInTheDocument(); + + // Show save button + expect(screen.getByRole('button', { name: /save/i })).toBeInTheDocument(); + }); + it('should change to read mode when click on `Cancel` on drawer variant', async () => { renderDrawer(stagedTagsId); expect(await screen.findByText('Taxonomy 1')).toBeInTheDocument(); diff --git a/src/content-tags-drawer/ContentTagsDrawer.tsx b/src/content-tags-drawer/ContentTagsDrawer.tsx index 9253205b3..7b278dcce 100644 --- a/src/content-tags-drawer/ContentTagsDrawer.tsx +++ b/src/content-tags-drawer/ContentTagsDrawer.tsx @@ -14,6 +14,7 @@ import ContentTagsCollapsible from './ContentTagsCollapsible'; import Loading from '../generic/Loading'; import { useCreateContentTagsDrawerContext } from './ContentTagsDrawerHelper'; import { ContentTagsDrawerContext, ContentTagsDrawerSheetContext } from './common/context'; +import { SidebarActions, useSidebarContext } from '../library-authoring/common/context/SidebarContext'; interface TaxonomyListProps { contentId: string; @@ -244,6 +245,7 @@ const ContentTagsDrawer = ({ if (contentId === undefined) { throw new Error('Error: contentId cannot be null.'); } + const { sidebarAction } = useSidebarContext(); const context = useCreateContentTagsDrawerContext(contentId, !readOnly, variant === 'drawer'); const { blockingSheet } = useContext(ContentTagsDrawerSheetContext); @@ -260,6 +262,7 @@ const ContentTagsDrawer = ({ closeToast, setCollapsibleToInitalState, otherTaxonomies, + toEditMode, } = context; let onCloseDrawer: () => void; @@ -302,8 +305,13 @@ const ContentTagsDrawer = ({ // First call of the initial collapsible states React.useEffect(() => { - setCollapsibleToInitalState(); - }, [isTaxonomyListLoaded, isContentTaxonomyTagsLoaded]); + // Open tag edit mode when sidebarAction is JumpToManageTags + if (sidebarAction === SidebarActions.JumpToManageTags) { + toEditMode(); + } else { + setCollapsibleToInitalState(); + } + }, [isTaxonomyListLoaded, isContentTaxonomyTagsLoaded, sidebarAction, toEditMode]); const renderFooter = () => { if (isTaxonomyListLoaded && isContentTaxonomyTagsLoaded) { diff --git a/src/content-tags-drawer/data/apiHooks.jsx b/src/content-tags-drawer/data/apiHooks.jsx index c32be8e3f..01ef3a3a7 100644 --- a/src/content-tags-drawer/data/apiHooks.jsx +++ b/src/content-tags-drawer/data/apiHooks.jsx @@ -7,6 +7,7 @@ import { useMutation, useQueryClient, } from '@tanstack/react-query'; +import { useParams } from 'react-router'; import { getTaxonomyTagsData, getContentTaxonomyTagsData, @@ -14,7 +15,7 @@ import { updateContentTaxonomyTags, getContentTaxonomyTagsCount, } from './api'; -import { libraryQueryPredicate, xblockQueryKeys } from '../../library-authoring/data/apiHooks'; +import { libraryAuthoringQueryKeys, libraryQueryPredicate, xblockQueryKeys } from '../../library-authoring/data/apiHooks'; import { getLibraryId } from '../../generic/key-utils'; /** @typedef {import("../../taxonomy/data/types.js").TagListData} TagListData */ @@ -129,6 +130,7 @@ export const useContentData = (contentId, enabled) => ( export const useContentTaxonomyTagsUpdater = (contentId) => { const queryClient = useQueryClient(); const unitIframe = window.frames['xblock-iframe']; + const { unitId } = useParams(); return useMutation({ /** @@ -158,6 +160,10 @@ export const useContentTaxonomyTagsUpdater = (contentId) => { queryClient.invalidateQueries(xblockQueryKeys.componentMetadata(contentId)); // Invalidate content search to update tags count queryClient.invalidateQueries(['content_search'], { predicate: (query) => libraryQueryPredicate(query, libraryId) }); + // If the tags for a compoent were edited from Unit page, invalidate children query to fetch count again. + if (unitId) { + queryClient.invalidateQueries(libraryAuthoringQueryKeys.containerChildren(unitId)); + } } }, onSuccess: /* istanbul ignore next */ () => { diff --git a/src/content-tags-drawer/data/apiHooks.test.jsx b/src/content-tags-drawer/data/apiHooks.test.jsx index 2ce95d846..ebfaf9e77 100644 --- a/src/content-tags-drawer/data/apiHooks.test.jsx +++ b/src/content-tags-drawer/data/apiHooks.test.jsx @@ -157,7 +157,7 @@ describe('useContentTaxonomyTagsUpdater', () => { const contentId = 'testerContent'; const taxonomyId = 123; - const mutation = useContentTaxonomyTagsUpdater(contentId); + const mutation = renderHook(() => useContentTaxonomyTagsUpdater(contentId)).result.current; const tagsData = [{ taxonomy: taxonomyId, tags: ['tag1', 'tag2'], diff --git a/src/course-outline/CourseOutline.test.jsx b/src/course-outline/CourseOutline.test.jsx index 055f56cdb..b0959c821 100644 --- a/src/course-outline/CourseOutline.test.jsx +++ b/src/course-outline/CourseOutline.test.jsx @@ -2230,7 +2230,7 @@ describe('', () => { .reply(200, courseSectionMock); let [subsectionElement] = await within(sectionElement).findAllByTestId('subsection-card'); const expandBtn = await within(subsectionElement).findByTestId('subsection-card-header__expanded-btn'); - await userEvent.click(expandBtn); + userEvent.click(expandBtn); const [unit] = subsection.childInfo.children; const [unitElement] = await within(subsectionElement).findAllByTestId('unit-card'); diff --git a/src/generic/DraggableList/DraggableList.jsx b/src/generic/DraggableList/DraggableList.jsx index 1515f29a0..ef86c45f0 100644 --- a/src/generic/DraggableList/DraggableList.jsx +++ b/src/generic/DraggableList/DraggableList.jsx @@ -1,10 +1,9 @@ -import React, { useCallback } from 'react'; +import { useCallback } from 'react'; import PropTypes from 'prop-types'; import { createPortal } from 'react-dom'; import { DndContext, - closestCenter, KeyboardSensor, PointerSensor, useSensor, @@ -18,6 +17,7 @@ import { verticalListSortingStrategy, } from '@dnd-kit/sortable'; import { restrictToVerticalAxis } from '@dnd-kit/modifiers'; +import { verticalSortableListCollisionDetection } from './verticalSortableList'; const DraggableList = ({ itemList, @@ -56,13 +56,20 @@ const DraggableList = ({ setActiveId?.(event.active.id); }, [setActiveId]); + const handleDragCancel = useCallback(() => { + setActiveId?.(null); + }, [setActiveId]); + return ( {actions} diff --git a/src/generic/DraggableList/verticalSortableList.ts b/src/generic/DraggableList/verticalSortableList.ts new file mode 100644 index 000000000..0d3409e32 --- /dev/null +++ b/src/generic/DraggableList/verticalSortableList.ts @@ -0,0 +1,80 @@ +/* istanbul ignore file */ +/** +This sorting strategy was copied over from https://github.com/clauderic/dnd-kit/pull/805 +to resolve issues with variable sized draggables. +*/ +import { CollisionDetection, DroppableContainer } from '@dnd-kit/core'; +import { sortBy } from 'lodash'; + +const collision = (dropppableContainer?: DroppableContainer) => ({ + id: dropppableContainer?.id ?? '', + value: dropppableContainer, +}); + +// Look for the first (/ furthest up / highest) droppable container that is at least +// 50% covered by the top edge of the dragging container. +const highestDroppableContainerMajorityCovered: CollisionDetection = ({ + droppableContainers, + collisionRect, +}) => { + const ascendingDroppabaleContainers = sortBy( + droppableContainers, + (c) => c?.rect.current?.top, + ); + + for (const droppableContainer of ascendingDroppabaleContainers) { + const { + rect: { current: droppableRect }, + } = droppableContainer; + + if (droppableRect) { + const coveredPercentage = (droppableRect.top + droppableRect.height - collisionRect.top) + / droppableRect.height; + + if (coveredPercentage > 0.5) { + return [collision(droppableContainer)]; + } + } + } + + // if we haven't found anything then we are off the top, so return the first item + return [collision(ascendingDroppabaleContainers[0])]; +}; + +// Look for the last (/ furthest down / lowest) droppable container that is at least +// 50% covered by the bottom edge of the dragging container. +const lowestDroppableContainerMajorityCovered: CollisionDetection = ({ + droppableContainers, + collisionRect, +}) => { + const descendingDroppabaleContainers = sortBy( + droppableContainers, + (c) => c?.rect.current?.top, + ).reverse(); + + for (const droppableContainer of descendingDroppabaleContainers) { + const { + rect: { current: droppableRect }, + } = droppableContainer; + + if (droppableRect) { + const coveredPercentage = (collisionRect.bottom - droppableRect.top) / droppableRect.height; + + if (coveredPercentage > 0.5) { + return [collision(droppableContainer)]; + } + } + } + + // if we haven't found anything then we are off the bottom, so return the last item + return [collision(descendingDroppabaleContainers[0])]; +}; + +export const verticalSortableListCollisionDetection: CollisionDetection = ( + args, +) => { + if (args.collisionRect.top < (args.active.rect.current?.initial?.top ?? 0)) { + return highestDroppableContainerMajorityCovered(args); + } + return lowestDroppableContainerMajorityCovered(args); +}; diff --git a/src/generic/hooks/tests/hooks.test.tsx b/src/generic/hooks/tests/hooks.test.tsx index bf15d83ba..d2e83f06f 100644 --- a/src/generic/hooks/tests/hooks.test.tsx +++ b/src/generic/hooks/tests/hooks.test.tsx @@ -96,7 +96,8 @@ describe('useIframeBehavior', () => { window.dispatchEvent(new MessageEvent('message', message)); }); - expect(setIframeHeight).toHaveBeenCalledWith(500); + // +10 padding + expect(setIframeHeight).toHaveBeenCalledWith(510); expect(setHasLoaded).toHaveBeenCalledWith(true); }); diff --git a/src/generic/hooks/useIframeBehavior.tsx b/src/generic/hooks/useIframeBehavior.tsx index 2c327a23d..1e60a6f94 100644 --- a/src/generic/hooks/useIframeBehavior.tsx +++ b/src/generic/hooks/useIframeBehavior.tsx @@ -46,7 +46,8 @@ export const useIframeBehavior = ({ switch (type) { case iframeMessageTypes.resize: - setIframeHeight(payload.height); + // Adding 10px as padding + setIframeHeight(payload.height + 10); if (!hasLoaded && iframeHeight === 0 && payload.height > 0) { setHasLoaded(true); } diff --git a/src/library-authoring/LibraryAuthoringPage.test.tsx b/src/library-authoring/LibraryAuthoringPage.test.tsx index 670c3af1a..cded3d334 100644 --- a/src/library-authoring/LibraryAuthoringPage.test.tsx +++ b/src/library-authoring/LibraryAuthoringPage.test.tsx @@ -55,6 +55,10 @@ const path = '/library/:libraryId/*'; const libraryTitle = mockContentLibrary.libraryData.title; describe('', () => { + beforeAll(() => { + jest.useFakeTimers(); + }); + beforeEach(async () => { const mocks = initializeMocks(); axiosMock = mocks.axiosMock; @@ -78,6 +82,10 @@ describe('', () => { }); }); + afterAll(() => { + jest.useRealTimers(); + }); + const renderLibraryPage = async () => { render(, { path, params: { libraryId: mockContentLibrary.libraryId } }); @@ -392,7 +400,7 @@ describe('', () => { await waitFor(() => expect(screen.queryByTestId('library-sidebar')).not.toBeInTheDocument()); }); - it('should open component sidebar, showing manage tab on clicking add to collection menu item (component)', async () => { + it('should open component sidebar, showing manage tab on clicking add to collection menu item - component', async () => { const mockResult0 = { ...mockResult }.results[0].hits[0]; const displayName = 'Introduction to Testing'; expect(mockResult0.display_name).toStrictEqual(displayName); @@ -407,9 +415,10 @@ describe('', () => { const sidebar = screen.getByTestId('library-sidebar'); - const { getByRole, queryByText } = within(sidebar); + const { getByRole, findByText } = within(sidebar); - await waitFor(() => expect(queryByText(displayName)).toBeInTheDocument()); + expect(await findByText(displayName)).toBeInTheDocument(); + jest.advanceTimersByTime(300); expect(getByRole('tab', { selected: true })).toHaveTextContent('Manage'); const closeButton = getByRole('button', { name: /close/i }); fireEvent.click(closeButton); @@ -417,7 +426,7 @@ describe('', () => { await waitFor(() => expect(screen.queryByTestId('library-sidebar')).not.toBeInTheDocument()); }); - it('should open component sidebar, showing manage tab on clicking add to collection menu item (unit)', async () => { + it('should open component sidebar, showing manage tab on clicking add to collection menu item - unit', async () => { const displayName = 'Test Unit'; await renderLibraryPage(); @@ -430,9 +439,10 @@ describe('', () => { const sidebar = screen.getByTestId('library-sidebar'); - const { getByRole, queryByText } = within(sidebar); + const { getByRole, findByText } = within(sidebar); - await waitFor(() => expect(queryByText(displayName)).toBeInTheDocument()); + expect(await findByText(displayName)).toBeInTheDocument(); + jest.advanceTimersByTime(300); expect(getByRole('tab', { selected: true })).toHaveTextContent('Manage'); const closeButton = getByRole('button', { name: /close/i }); fireEvent.click(closeButton); diff --git a/src/library-authoring/LibraryBlock/LibraryBlock.tsx b/src/library-authoring/LibraryBlock/LibraryBlock.tsx index c87091acd..52e0794e0 100644 --- a/src/library-authoring/LibraryBlock/LibraryBlock.tsx +++ b/src/library-authoring/LibraryBlock/LibraryBlock.tsx @@ -1,3 +1,4 @@ +import { useEffect } from 'react'; import { useIntl } from '@edx/frontend-platform/i18n'; import { getConfig } from '@edx/frontend-platform'; @@ -16,6 +17,7 @@ interface LibraryBlockProps { view?: string; scrolling?: string; minHeight?: string; + scrollIntoView?: boolean; } /** * React component that displays an XBlock in a sandboxed IFrame. @@ -33,6 +35,7 @@ export const LibraryBlock = ({ view, minHeight, scrolling = 'no', + scrollIntoView = false, }: LibraryBlockProps) => { const { iframeRef, setIframeRef } = useIframe(); const xblockView = view ?? 'student_view'; @@ -49,6 +52,13 @@ export const LibraryBlock = ({ onBlockNotification, }); + useEffect(() => { + /* istanbul ignore next */ + if (scrollIntoView) { + iframeRef?.current?.scrollIntoView({ behavior: 'smooth' }); + } + }, [scrollIntoView]); + useIframeContent(iframeRef, setIframeRef); return ( diff --git a/src/library-authoring/common/context/SidebarContext.tsx b/src/library-authoring/common/context/SidebarContext.tsx index f40ccfec1..065784287 100644 --- a/src/library-authoring/common/context/SidebarContext.tsx +++ b/src/library-authoring/common/context/SidebarContext.tsx @@ -63,7 +63,8 @@ export interface SidebarComponentInfo { } export enum SidebarActions { - JumpToAddCollections = 'jump-to-add-collections', + JumpToManageCollections = 'jump-to-manage-collections', + JumpToManageTags = 'jump-to-manage-tags', ManageTeam = 'manage-team', None = '', } diff --git a/src/library-authoring/component-info/ComponentInfo.tsx b/src/library-authoring/component-info/ComponentInfo.tsx index 1b542c695..635e54cc7 100644 --- a/src/library-authoring/component-info/ComponentInfo.tsx +++ b/src/library-authoring/component-info/ComponentInfo.tsx @@ -1,4 +1,4 @@ -import React, { useEffect } from 'react'; +import React from 'react'; import { useIntl } from '@edx/frontend-platform/i18n'; import { Button, @@ -17,7 +17,6 @@ import { useLibraryContext } from '../common/context/LibraryContext'; import { type ComponentInfoTab, COMPONENT_INFO_TABS, - SidebarActions, isComponentInfoTab, useSidebarContext, } from '../common/context/SidebarContext'; @@ -107,9 +106,9 @@ const ComponentInfo = () => { sidebarTab, setSidebarTab, sidebarComponentInfo, - sidebarAction, defaultTab, hiddenTabs, + resetSidebarAction, } = useSidebarContext(); const [ isPublishConfirmationOpen, @@ -117,20 +116,16 @@ const ComponentInfo = () => { closePublishConfirmation, ] = useToggle(false); - const jumpToCollections = sidebarAction === SidebarActions.JumpToAddCollections; - const tab: ComponentInfoTab = ( isComponentInfoTab(sidebarTab) ? sidebarTab : defaultTab.component ); - useEffect(() => { - // Show Manage tab if JumpToAddCollections action is set in sidebarComponentInfo - if (jumpToCollections) { - setSidebarTab(COMPONENT_INFO_TABS.Manage); - } - }, [jumpToCollections, setSidebarTab]); + const handleTabChange = (newTab: ComponentInfoTab) => { + resetSidebarAction(); + setSidebarTab(newTab); + }; const usageKey = sidebarComponentInfo?.id; // istanbul ignore if: this should never happen @@ -198,7 +193,7 @@ const ComponentInfo = () => { className="my-3 d-flex justify-content-around" defaultActiveKey={defaultTab.component} activeKey={tab} - onSelect={setSidebarTab} + onSelect={handleTabChange} > {renderTab(COMPONENT_INFO_TABS.Preview, , intl.formatMessage(messages.previewTabTitle))} {renderTab(COMPONENT_INFO_TABS.Manage, , intl.formatMessage(messages.manageTabTitle))} diff --git a/src/library-authoring/component-info/ComponentManagement.test.tsx b/src/library-authoring/component-info/ComponentManagement.test.tsx index 9e070cde9..0e4491265 100644 --- a/src/library-authoring/component-info/ComponentManagement.test.tsx +++ b/src/library-authoring/component-info/ComponentManagement.test.tsx @@ -8,7 +8,7 @@ import { waitFor, } from '../../testUtils'; import { LibraryProvider } from '../common/context/LibraryContext'; -import { SidebarBodyComponentId, SidebarProvider } from '../common/context/SidebarContext'; +import { SidebarActions, SidebarBodyComponentId, SidebarProvider } from '../common/context/SidebarContext'; import { mockContentLibrary, mockLibraryBlockMetadata } from '../data/api.mocks'; import ComponentManagement from './ComponentManagement'; @@ -19,6 +19,16 @@ jest.mock('../../content-tags-drawer', () => ({ ), })); +const mockSearchParam = jest.fn(); + +jest.mock('react-router-dom', () => ({ + ...jest.requireActual('react-router-dom'), // use actual for all non-hook parts + useSearchParams: () => [ + { getAll: (paramName: string) => mockSearchParam(paramName) }, + () => {}, + ], +})); + mockContentLibrary.applyMock(); mockLibraryBlockMetadata.applyMock(); mockContentTaxonomyTagsData.applyMock(); @@ -55,6 +65,11 @@ const render = (usageKey: string, libraryId?: string) => baseRender(', () => { beforeEach(() => { initializeMocks(); + mockSearchParam.mockResolvedValue([undefined, () => {}]); + }); + + afterEach(() => { + jest.clearAllMocks(); }); it('should render draft status', async () => { @@ -119,4 +134,34 @@ describe('', () => { render(mockLibraryBlockMetadata.usageKeyWithCollections); expect(await screen.findByText('Collections (1)')).toBeInTheDocument(); }); + + it('should open collection section when sidebarAction = JumpToManageCollections', async () => { + setConfig({ + ...getConfig(), + ENABLE_TAGGING_TAXONOMY_PAGES: 'true', + }); + mockSearchParam.mockReturnValue([SidebarActions.JumpToManageCollections]); + render(mockLibraryBlockMetadata.usageKeyWithCollections); + expect(await screen.findByText('Collections (1)')).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: 'Manage tags' })).not.toBeInTheDocument(); + const tagsSection = await screen.findByRole('button', { name: 'Tags (0)' }); + expect(tagsSection).toHaveAttribute('aria-expanded', 'false'); + const collectionsSection = await screen.findByRole('button', { name: 'Collections (1)' }); + expect(collectionsSection).toHaveAttribute('aria-expanded', 'true'); + }); + + it('should open tags section when sidebarAction = JumpToManageTags', async () => { + setConfig({ + ...getConfig(), + ENABLE_TAGGING_TAXONOMY_PAGES: 'true', + }); + mockSearchParam.mockReturnValue([SidebarActions.JumpToManageTags]); + render(mockLibraryBlockMetadata.usageKeyForTags); + expect(await screen.findByText('Collections (0)')).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: 'Manage tags' })).not.toBeInTheDocument(); + const tagsSection = await screen.findByRole('button', { name: 'Tags (6)' }); + expect(tagsSection).toHaveAttribute('aria-expanded', 'true'); + const collectionsSection = await screen.findByRole('button', { name: 'Collections (0)' }); + expect(collectionsSection).toHaveAttribute('aria-expanded', 'false'); + }); }); diff --git a/src/library-authoring/component-info/ComponentManagement.tsx b/src/library-authoring/component-info/ComponentManagement.tsx index c0eccdc5e..5c3b6a664 100644 --- a/src/library-authoring/component-info/ComponentManagement.tsx +++ b/src/library-authoring/component-info/ComponentManagement.tsx @@ -18,7 +18,8 @@ const ComponentManagement = () => { const intl = useIntl(); const { readOnly, isLoadingLibraryData } = useLibraryContext(); const { sidebarComponentInfo, sidebarAction, resetSidebarAction } = useSidebarContext(); - const jumpToCollections = sidebarAction === SidebarActions.JumpToAddCollections; + const jumpToCollections = sidebarAction === SidebarActions.JumpToManageCollections; + const jumpToTags = sidebarAction === SidebarActions.JumpToManageTags; const [tagsCollapseIsOpen, setTagsCollapseOpen] = React.useState(!jumpToCollections); const [collectionsCollapseIsOpen, setCollectionsCollapseOpen] = React.useState(true); @@ -26,8 +27,11 @@ const ComponentManagement = () => { if (jumpToCollections) { setTagsCollapseOpen(false); setCollectionsCollapseOpen(true); + } else if (jumpToTags) { + setTagsCollapseOpen(true); + setCollectionsCollapseOpen(false); } - }, [jumpToCollections, tagsCollapseIsOpen, collectionsCollapseIsOpen]); + }, [jumpToCollections, jumpToTags]); useEffect(() => { // This is required to redo actions. diff --git a/src/library-authoring/components/ComponentMenu.tsx b/src/library-authoring/components/ComponentMenu.tsx index 589901326..e3d0afe64 100644 --- a/src/library-authoring/components/ComponentMenu.tsx +++ b/src/library-authoring/components/ComponentMenu.tsx @@ -20,6 +20,8 @@ import { import { canEditComponent } from './ComponentEditorModal'; import ComponentDeleter from './ComponentDeleter'; import messages from './messages'; +import { useLibraryRoutes } from '../routes'; +import { useRunOnNextRender } from '../../utils'; export const ComponentMenu = ({ usageKey }: { usageKey: string }) => { const intl = useIntl(); @@ -36,6 +38,7 @@ export const ComponentMenu = ({ usageKey }: { usageKey: string }) => { closeLibrarySidebar, setSidebarAction, } = useSidebarContext(); + const { navigateTo } = useLibraryRoutes(); const canEdit = usageKey && canEditComponent(usageKey); const { showToast } = useContext(ToastContext); @@ -87,10 +90,22 @@ export const ComponentMenu = ({ usageKey }: { usageKey: string }) => { }); }; + const scheduleJumpToCollection = useRunOnNextRender(() => { + // TODO: Ugly hack to make sure sidebar shows add to collection section + // This needs to run after all changes to url takes place to avoid conflicts. + setTimeout(() => setSidebarAction(SidebarActions.JumpToManageCollections), 250); + }); + const showManageCollections = useCallback(() => { - setSidebarAction(SidebarActions.JumpToAddCollections); + navigateTo({ componentId: usageKey }); openComponentInfoSidebar(usageKey); - }, [setSidebarAction, openComponentInfoSidebar, usageKey]); + scheduleJumpToCollection(); + }, [ + scheduleJumpToCollection, + openComponentInfoSidebar, + usageKey, + navigateTo, + ]); return ( @@ -123,11 +138,9 @@ export const ComponentMenu = ({ usageKey }: { usageKey: string }) => { )} - {!unitId && ( - - - - )} + + + diff --git a/src/library-authoring/components/ContainerCard.tsx b/src/library-authoring/components/ContainerCard.tsx index a1d3115a6..97e30e382 100644 --- a/src/library-authoring/components/ContainerCard.tsx +++ b/src/library-authoring/components/ContainerCard.tsx @@ -24,6 +24,7 @@ import AddComponentWidget from './AddComponentWidget'; import BaseCard from './BaseCard'; import messages from './messages'; import ContainerDeleter from './ContainerDeleter'; +import { useRunOnNextRender } from '../../utils'; type ContainerMenuProps = { hit: ContainerHit, @@ -45,6 +46,7 @@ const ContainerMenu = ({ hit } : ContainerMenuProps) => { } = useSidebarContext(); const { showToast } = useContext(ToastContext); const [isConfirmingDelete, confirmDelete, cancelDelete] = useToggle(false); + const { navigateTo } = useLibraryRoutes(); const removeComponentsMutation = useRemoveItemsFromCollection(libraryId, collectionId); @@ -60,10 +62,17 @@ const ContainerMenu = ({ hit } : ContainerMenuProps) => { }); }; + const scheduleJumpToCollection = useRunOnNextRender(() => { + // TODO: Ugly hack to make sure sidebar shows add to collection section + // This needs to run after all changes to url takes place to avoid conflicts. + setTimeout(() => setSidebarAction(SidebarActions.JumpToManageCollections)); + }); + const showManageCollections = useCallback(() => { - setSidebarAction(SidebarActions.JumpToAddCollections); + navigateTo({ unitId: containerId }); openUnitInfoSidebar(containerId); - }, [setSidebarAction, openUnitInfoSidebar, containerId]); + scheduleJumpToCollection(); + }, [scheduleJumpToCollection, navigateTo, openUnitInfoSidebar, containerId]); return ( <> diff --git a/src/library-authoring/containers/ContainerOrganize.tsx b/src/library-authoring/containers/ContainerOrganize.tsx index 5c336abc0..6419bfd43 100644 --- a/src/library-authoring/containers/ContainerOrganize.tsx +++ b/src/library-authoring/containers/ContainerOrganize.tsx @@ -28,7 +28,7 @@ const ContainerOrganize = () => { const { readOnly } = useLibraryContext(); const { sidebarComponentInfo, sidebarAction } = useSidebarContext(); - const jumpToCollections = sidebarAction === SidebarActions.JumpToAddCollections; + const jumpToCollections = sidebarAction === SidebarActions.JumpToManageCollections; const containerId = sidebarComponentInfo?.id; // istanbul ignore if: this should never happen diff --git a/src/library-authoring/containers/UnitInfo.tsx b/src/library-authoring/containers/UnitInfo.tsx index df70791e1..e143e3ff3 100644 --- a/src/library-authoring/containers/UnitInfo.tsx +++ b/src/library-authoring/containers/UnitInfo.tsx @@ -9,7 +9,7 @@ import { IconButton, useToggle, } from '@openedx/paragon'; -import React, { useEffect, useCallback } from 'react'; +import React, { useCallback } from 'react'; import { Link } from 'react-router-dom'; import { MoreVert } from '@openedx/paragon/icons'; @@ -17,7 +17,6 @@ import { useComponentPickerContext } from '../common/context/ComponentPickerCont import { useLibraryContext } from '../common/context/LibraryContext'; import { type UnitInfoTab, - SidebarActions, UNIT_INFO_TABS, isUnitInfoTab, useSidebarContext, @@ -81,9 +80,8 @@ const UnitInfo = () => { sidebarTab, setSidebarTab, sidebarComponentInfo, - sidebarAction, + resetSidebarAction, } = useSidebarContext(); - const jumpToCollections = sidebarAction === SidebarActions.JumpToAddCollections; const { insideUnit } = useLibraryRoutes(); const tab: UnitInfoTab = ( @@ -96,6 +94,12 @@ const UnitInfo = () => { const showOpenUnitButton = !insideUnit && !componentPickerMode; + /* istanbul ignore next */ + const handleTabChange = (newTab: UnitInfoTab) => { + resetSidebarAction(); + setSidebarTab(newTab); + }; + const renderTab = useCallback((infoTab: UnitInfoTab, component: React.ReactNode, title: string) => { if (hiddenTabs.includes(infoTab)) { // For some reason, returning anything other than empty list breaks the tab style @@ -117,13 +121,6 @@ const UnitInfo = () => { } }, [publishContainer]); - useEffect(() => { - // Show Organize tab if JumpToAddCollections action is set in sidebarComponentInfo - if (jumpToCollections) { - setSidebarTab(UNIT_INFO_TABS.Manage); - } - }, [jumpToCollections, setSidebarTab]); - if (!container || !unitId) { return null; } @@ -163,7 +160,7 @@ const UnitInfo = () => { className="my-3 d-flex justify-content-around" defaultActiveKey={defaultTab.unit} activeKey={tab} - onSelect={setSidebarTab} + onSelect={handleTabChange} > {renderTab(UNIT_INFO_TABS.Preview, , intl.formatMessage(messages.previewTabTitle))} {renderTab(UNIT_INFO_TABS.Manage, , intl.formatMessage(messages.manageTabTitle))} diff --git a/src/library-authoring/data/api.ts b/src/library-authoring/data/api.ts index 220ce0a3f..d4a737f11 100644 --- a/src/library-authoring/data/api.ts +++ b/src/library-authoring/data/api.ts @@ -259,6 +259,9 @@ export interface LibraryBlockMetadata { modified: string | null; tagsCount: number; collections: CollectionMetadata[]; + // Local only variable set to true when a new block is added + // NOTE: Currently only updated when a new component is added inside a unit + isNew?: boolean; } export interface UpdateLibraryDataRequest { diff --git a/src/library-authoring/data/apiHooks.ts b/src/library-authoring/data/apiHooks.ts index b9a825b34..7ccc0a939 100644 --- a/src/library-authoring/data/apiHooks.ts +++ b/src/library-authoring/data/apiHooks.ts @@ -5,6 +5,7 @@ import { useQueryClient, type Query, type QueryClient, + replaceEqualDeep, } from '@tanstack/react-query'; import { useCallback } from 'react'; @@ -632,6 +633,22 @@ export const useContainerChildren = (containerId?: string) => ( enabled: !!containerId, queryKey: libraryAuthoringQueryKeys.containerChildren(containerId!), queryFn: () => api.getLibraryContainerChildren(containerId!), + structuralSharing: (oldData: api.LibraryBlockMetadata[], newData: api.LibraryBlockMetadata[]) => { + // This just sets `isNew` flag to new children components + if (oldData) { + const oldDataIds = oldData.map((obj) => obj.id); + // eslint-disable-next-line no-param-reassign + newData = newData.map((newObj) => { + if (!oldDataIds.includes(newObj.id)) { + // Set isNew = true if we have new child on refetch + // eslint-disable-next-line no-param-reassign + newObj.isNew = true; + } + return newObj; + }); + } + return replaceEqualDeep(oldData, newData); + }, }) ); diff --git a/src/library-authoring/generic/manage-collections/ManageCollections.tsx b/src/library-authoring/generic/manage-collections/ManageCollections.tsx index 96591b327..4cafadfa4 100644 --- a/src/library-authoring/generic/manage-collections/ManageCollections.tsx +++ b/src/library-authoring/generic/manage-collections/ManageCollections.tsx @@ -206,7 +206,7 @@ const ManageCollections = ({ opaqueKey, collections, useUpdateCollectionsHook }: const collectionNames = collections.map((collection) => collection.title); return ( - sidebarAction === SidebarActions.JumpToAddCollections + sidebarAction === SidebarActions.JumpToManageCollections ? ( setSidebarAction(SidebarActions.JumpToAddCollections)} + onManageClick={() => setSidebarAction(SidebarActions.JumpToManageCollections)} /> ) ); diff --git a/src/library-authoring/library-sidebar/LibrarySidebar.tsx b/src/library-authoring/library-sidebar/LibrarySidebar.tsx index a4443e73e..aeae34097 100644 --- a/src/library-authoring/library-sidebar/LibrarySidebar.tsx +++ b/src/library-authoring/library-sidebar/LibrarySidebar.tsx @@ -10,11 +10,17 @@ import { useIntl } from '@edx/frontend-platform/i18n'; import { AddContent, AddContentHeader } from '../add-content'; import { CollectionInfo, CollectionInfoHeader } from '../collections'; import { ContainerInfoHeader, UnitInfo } from '../containers'; -import { SidebarBodyComponentId, useSidebarContext } from '../common/context/SidebarContext'; +import { + COMPONENT_INFO_TABS, SidebarActions, SidebarBodyComponentId, useSidebarContext, +} from '../common/context/SidebarContext'; import { ComponentInfo, ComponentInfoHeader } from '../component-info'; import { LibraryInfo, LibraryInfoHeader } from '../library-info'; import messages from '../messages'; +interface LibrarySidebarProps { + onSidebarClose?: () => void; +} + /** * Sidebar container for library pages. * @@ -24,9 +30,25 @@ import messages from '../messages'; * You can add more components in `bodyComponentMap`. * Use the returned actions to open and close this sidebar. */ -const LibrarySidebar = () => { +const LibrarySidebar = ({ onSidebarClose }: LibrarySidebarProps) => { const intl = useIntl(); - const { sidebarComponentInfo, closeLibrarySidebar } = useSidebarContext(); + const { + sidebarAction, + setSidebarTab, + sidebarComponentInfo, + closeLibrarySidebar, + } = useSidebarContext(); + const jumpToCollections = sidebarAction === SidebarActions.JumpToManageCollections; + const jumpToTags = sidebarAction === SidebarActions.JumpToManageTags; + + React.useEffect(() => { + // Show Manage tab if JumpToManageCollections or JumpToManageTags action is set + if (jumpToCollections || jumpToTags) { + // COMPONENT_INFO_TABS.Manage works for containers as well as its value + // is same as UNIT_INFO_TABS.Manage. + setSidebarTab(COMPONENT_INFO_TABS.Manage); + } + }, [jumpToCollections, setSidebarTab, jumpToTags]); const bodyComponentMap = { [SidebarBodyComponentId.AddContent]: , @@ -49,6 +71,11 @@ const LibrarySidebar = () => { const buildBody = () : React.ReactNode => bodyComponentMap[sidebarComponentInfo?.type || 'unknown']; const buildHeader = (): React.ReactNode => headerComponentMap[sidebarComponentInfo?.type || 'unknown']; + const handleSidebarClose = () => { + closeLibrarySidebar(); + onSidebarClose?.(); + }; + return ( @@ -58,7 +85,7 @@ const LibrarySidebar = () => { src={Close} iconAs={Icon} alt={intl.formatMessage(messages.closeButtonAlt)} - onClick={closeLibrarySidebar} + onClick={handleSidebarClose} size="inline" /> diff --git a/src/library-authoring/routes.test.tsx b/src/library-authoring/routes.test.tsx index 8a58f3a41..ad03cbc4c 100644 --- a/src/library-authoring/routes.test.tsx +++ b/src/library-authoring/routes.test.tsx @@ -15,6 +15,14 @@ jest.mock('react-router-dom', () => ({ ...jest.requireActual('react-router-dom'), useNavigate: () => mockNavigate, })); +jest.mock('./common/context/LibraryContext', () => ({ + ...jest.requireActual('./common/context/LibraryContext'), + useLibraryContext: () => ({ + setComponentId: jest.fn(), + setUnitId: jest.fn(), + setCollectionId: jest.fn(), + }), +})); mockContentLibrary.applyMock(); diff --git a/src/library-authoring/routes.ts b/src/library-authoring/routes.ts index 1bee5219a..510368456 100644 --- a/src/library-authoring/routes.ts +++ b/src/library-authoring/routes.ts @@ -11,6 +11,7 @@ import { useSearchParams, type PathMatch, } from 'react-router-dom'; +import { useLibraryContext } from './common/context/LibraryContext'; export const BASE_ROUTE = '/library/:libraryId'; @@ -66,6 +67,7 @@ export const useLibraryRoutes = (): LibraryRoutesData => { const params = useParams(); const [searchParams] = useSearchParams(); const navigate = useNavigate(); + const { setComponentId, setUnitId, setCollectionId } = useLibraryContext(); const insideCollection = matchPath(BASE_ROUTE + ROUTES.COLLECTION, pathname); const insideCollections = matchPath(BASE_ROUTE + ROUTES.COLLECTIONS, pathname); @@ -99,6 +101,18 @@ export const useLibraryRoutes = (): LibraryRoutesData => { }; let route: string; + // Update componentId, unitId, collectionId in library context if is not undefined. + // Ids can be cleared from route by passing in empty string so we need to set it. + if (componentId !== undefined) { + setComponentId(componentId); + } + if (unitId !== undefined) { + setUnitId(unitId); + } + if (collectionId !== undefined) { + setCollectionId(collectionId); + } + // Providing contentType overrides the current route so we can change tabs. if (contentType === ContentType.components) { route = ROUTES.COMPONENTS; @@ -158,7 +172,15 @@ export const useLibraryRoutes = (): LibraryRoutesData => { pathname: newPath, search: searchParams.toString(), }); - }, [navigate, params, searchParams, pathname]); + }, [ + navigate, + params, + searchParams, + pathname, + setComponentId, + setUnitId, + setCollectionId, + ]); return { navigateTo, diff --git a/src/library-authoring/units/LibraryUnitBlocks.tsx b/src/library-authoring/units/LibraryUnitBlocks.tsx index 43e985eb8..aa2b01c88 100644 --- a/src/library-authoring/units/LibraryUnitBlocks.tsx +++ b/src/library-authoring/units/LibraryUnitBlocks.tsx @@ -1,12 +1,12 @@ import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n'; import { - ActionRow, Badge, Button, Icon, IconButton, Stack, useToggle, + ActionRow, Badge, Button, Icon, Stack, useToggle, } from '@openedx/paragon'; -import { Add, Description, DragIndicator } from '@openedx/paragon/icons'; -import { useQueryClient } from '@tanstack/react-query'; +import { Add, Description } from '@openedx/paragon/icons'; import classNames from 'classnames'; -import { useContext, useEffect, useState } from 'react'; -import { ContentTagsDrawerSheet } from '../../content-tags-drawer'; +import { + useCallback, useContext, useEffect, useState, +} from 'react'; import { blockTypes } from '../../editors/data/constants/app'; import DraggableList, { SortableItem } from '../../generic/DraggableList'; @@ -22,7 +22,6 @@ import { PickLibraryContentModal } from '../add-content'; import ComponentMenu from '../components'; import { LibraryBlockMetadata } from '../data/api'; import { - libraryAuthoringQueryKeys, useContainerChildren, useUpdateContainerChildren, useUpdateXBlockFields, @@ -30,9 +29,10 @@ import { import { LibraryBlock } from '../LibraryBlock'; import { useLibraryRoutes, ContentType } from '../routes'; import messages from './messages'; -import { useSidebarContext } from '../common/context/SidebarContext'; +import { SidebarActions, useSidebarContext } from '../common/context/SidebarContext'; import { ToastContext } from '../../generic/toast-context'; import { canEditComponent } from '../components/ComponentEditorModal'; +import { useRunOnNextRender } from '../../utils'; /** Components that need large min height in preview */ const LARGE_COMPONENTS = [ @@ -43,17 +43,24 @@ const LARGE_COMPONENTS = [ 'lti_consumer', ]; -interface BlockHeaderProps { - block: LibraryBlockMetadata; - onTagClick: () => void; +interface LibraryBlockMetadataWithUniqueId extends LibraryBlockMetadata { + originalId: string; } -/** Component header, split out to reuse in drag overlay */ -const BlockHeader = ({ block, onTagClick }: BlockHeaderProps) => { +interface ComponentBlockProps { + block: LibraryBlockMetadataWithUniqueId; + preview?: boolean; + isDragging?: boolean; +} + +/** Component header */ +const BlockHeader = ({ block }: ComponentBlockProps) => { const intl = useIntl(); const { showToast } = useContext(ToastContext); + const { navigateTo } = useLibraryRoutes(); + const { openComponentInfoSidebar, setSidebarAction } = useSidebarContext(); - const updateMutation = useUpdateXBlockFields(block.id); + const updateMutation = useUpdateXBlockFields(block.originalId); const handleSaveDisplayName = (newDisplayName: string) => { updateMutation.mutateAsync({ @@ -67,9 +74,30 @@ const BlockHeader = ({ block, onTagClick }: BlockHeaderProps) => { }); }; + /* istanbul ignore next */ + const scheduleJumpToTags = useRunOnNextRender(() => { + // TODO: Ugly hack to make sure sidebar shows manage tags section + // This needs to run after all changes to url takes place to avoid conflicts. + setTimeout(() => setSidebarAction(SidebarActions.JumpToManageTags), 250); + }); + + /* istanbul ignore next */ + const jumpToManageTags = () => { + navigateTo({ componentId: block.originalId }); + openComponentInfoSidebar(block.originalId); + scheduleJumpToTags(); + }; + return ( <> - + e.stopPropagation()} + > { /> - + e.stopPropagation()} + > {block.hasUnpublishedChanges && ( - + )} - - + + ); }; +/** ComponentBlock to render preview of given component under Unit */ +const ComponentBlock = ({ block, preview, isDragging }: ComponentBlockProps) => { + const { showOnlyPublished } = useLibraryContext(); + const { navigateTo } = useLibraryRoutes(); + + const { + unitId, collectionId, componentId, openComponentEditor, + } = useLibraryContext(); + + const { openInfoSidebar } = useSidebarContext(); + + const handleComponentSelection = useCallback((numberOfClicks: number) => { + navigateTo({ componentId: block.originalId }); + const canEdit = canEditComponent(block.originalId); + if (numberOfClicks > 1 && canEdit) { + // Open editor on double click. + openComponentEditor(block.originalId); + } else { + // open current component sidebar + openInfoSidebar(block.originalId, collectionId, unitId); + } + }, [block, collectionId, unitId, navigateTo, canEditComponent, openComponentEditor, openInfoSidebar]); + + useEffect(() => { + if (block.isNew) { + handleComponentSelection(1); + } + }, [block]); + + /* istanbul ignore next */ + const calculateMinHeight = () => { + if (LARGE_COMPONENTS.includes(block.blockType)) { + return '700px'; + } + return '200px'; + }; + + const getComponentStyle = useCallback(() => { + if (isDragging) { + return { + outline: '2px dashed gray', + maxHeight: '200px', + overflowY: 'hidden', + }; + } if (componentId === block.originalId) { + return { + outline: '2px solid black', + }; + } + return {}; + }, [isDragging, componentId, block]); + + return ( + + } + actionStyle={{ + borderRadius: '8px 8px 0px 0px', + padding: '0.5rem 1rem', + background: '#FBFAF9', + borderBottom: 'solid 1px #E1DDDB', + }} + isClickable + onClick={(e: { detail: number; }) => handleComponentSelection(e.detail)} + disabled={preview} + > + {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */} +
e.stopPropagation()} + > + +
+ + + ); +}; + interface LibraryUnitBlocksProps { /** set to true if it is rendered as preview * This disables drag and drop @@ -105,28 +227,16 @@ interface LibraryUnitBlocksProps { export const LibraryUnitBlocks = ({ preview }: LibraryUnitBlocksProps) => { const intl = useIntl(); - const [orderedBlocks, setOrderedBlocks] = useState([]); - const [isManageTagsDrawerOpen, openManageTagsDrawer, closeManageTagsDrawer] = useToggle(false); + const [orderedBlocks, setOrderedBlocks] = useState([]); const [isAddLibraryContentModalOpen, showAddLibraryContentModal, closeAddLibraryContentModal] = useToggle(); const [hidePreviewFor, setHidePreviewFor] = useState(null); - const { navigateTo } = useLibraryRoutes(); const { showToast } = useContext(ToastContext); - const { - unitId, - showOnlyPublished, - componentId, - readOnly, - setComponentId, - openComponentEditor, - } = useLibraryContext(); + const { unitId, readOnly } = useLibraryContext(); - const { - openAddContentSidebar, - } = useSidebarContext(); + const { openAddContentSidebar } = useSidebarContext(); - const queryClient = useQueryClient(); const orderMutator = useUpdateContainerChildren(unitId); const { data: blocks, @@ -135,7 +245,32 @@ export const LibraryUnitBlocks = ({ preview }: LibraryUnitBlocksProps) => { error, } = useContainerChildren(unitId); - useEffect(() => setOrderedBlocks(blocks || []), [blocks]); + const handleReorder = useCallback(() => async (newOrder?: LibraryBlockMetadataWithUniqueId[]) => { + if (!newOrder) { + return; + } + const usageKeys = newOrder.map((o) => o.originalId); + try { + await orderMutator.mutateAsync(usageKeys); + showToast(intl.formatMessage(messages.orderUpdatedMsg)); + } catch (e) { + showToast(intl.formatMessage(messages.failedOrderUpdatedMsg)); + } + }, [orderMutator]); + + useEffect(() => { + // Create new ids which are unique using index. + // This is required to support multiple components with same id under a unit. + const newBlocks = blocks?.map((block, idx) => { + const newBlock: LibraryBlockMetadataWithUniqueId = { + ...block, + id: `${block.id}----${idx}`, + originalId: block.id, + }; + return newBlock; + }); + return setOrderedBlocks(newBlocks || []); + }, [blocks, setOrderedBlocks]); if (isLoading) { return ; @@ -146,106 +281,25 @@ export const LibraryUnitBlocks = ({ preview }: LibraryUnitBlocksProps) => { return ; } - const handleReorder = () => async (newOrder: LibraryBlockMetadata[]) => { - const usageKeys = newOrder.map((o) => o.id); - try { - await orderMutator.mutateAsync(usageKeys); - showToast(intl.formatMessage(messages.orderUpdatedMsg)); - } catch (e) { - showToast(intl.formatMessage(messages.failedOrderUpdatedMsg)); - } - }; - - const onTagSidebarClose = () => { - queryClient.invalidateQueries(libraryAuthoringQueryKeys.containerChildren(unitId!)); - closeManageTagsDrawer(); - }; - - const handleComponentSelection = (block: LibraryBlockMetadata, numberOfClicks: number) => { - setComponentId(block.id); - navigateTo({ componentId: block.id }); - const canEdit = canEditComponent(block.id); - if (numberOfClicks > 1 && canEdit) { - // Open editor on double click. - openComponentEditor(block.id); - } - }; - - /* istanbul ignore next */ - const calculateMinHeight = (block: LibraryBlockMetadata) => { - if (LARGE_COMPONENTS.includes(block.blockType)) { - return '700px'; - } - return '200px'; - }; - - const renderOverlay = (activeId: string | null) => { - if (!activeId) { - return null; - } - const block = orderedBlocks?.find((val) => val.id === activeId); - if (!block) { - return null; - } - return ( - - - - - ); - }; - - const renderedBlocks = orderedBlocks?.map((block, idx) => ( - // A container can have multiple instances of the same block - // eslint-disable-next-line react/no-array-index-key - - } - actionStyle={{ - borderRadius: '8px 8px 0px 0px', - padding: '0.5rem 1rem', - background: '#FBFAF9', - borderBottom: 'solid 1px #E1DDDB', - outline: hidePreviewFor === block.id && '2px dashed gray', - }} - isClickable - onClick={(e: { detail: number; }) => handleComponentSelection(block, e.detail)} - disabled={preview} - > - {hidePreviewFor !== block.id && ( -
- -
- )} - - - )); - return (
- {renderedBlocks} + {orderedBlocks?.map((block, idx) => ( + // A container can have multiple instances of the same block + // eslint-disable-next-line react/no-array-index-key + + ))} {!preview && (
@@ -281,11 +335,6 @@ export const LibraryUnitBlocks = ({ preview }: LibraryUnitBlocksProps) => {
)} - ); }; diff --git a/src/library-authoring/units/LibraryUnitPage.test.tsx b/src/library-authoring/units/LibraryUnitPage.test.tsx index f90dcedd1..6d4ea7e31 100644 --- a/src/library-authoring/units/LibraryUnitPage.test.tsx +++ b/src/library-authoring/units/LibraryUnitPage.test.tsx @@ -42,14 +42,14 @@ mockContentLibrary.applyMock(); mockXBlockFields.applyMock(); mockLibraryBlockMetadata.applyMock(); -const closestCenter = jest.fn(); -jest.mock('@dnd-kit/core', () => ({ - ...jest.requireActual('@dnd-kit/core'), +const verticalSortableListCollisionDetection = jest.fn(); +jest.mock('../../generic/DraggableList/verticalSortableList', () => ({ + ...jest.requireActual('../../generic/DraggableList/verticalSortableList'), // Since jsdom (used by jest) does not support getBoundingClientRect function // which is required for drag-n-drop calculations, we mock closestCorners fn // from dnd-kit to return collided elements as per the test. This allows us to // test all drag-n-drop handlers. - closestCenter: () => closestCenter(), + verticalSortableListCollisionDetection: () => verticalSortableListCollisionDetection(), })); describe('', () => { @@ -187,9 +187,9 @@ describe('', () => { it('should open and close component sidebar on component selection', async () => { renderLibraryUnitPage(); - const component = await screen.findByText('text block 0'); - userEvent.click(component); + // Card is 3 levels up the component name div + userEvent.click(component.parentElement!.parentElement!.parentElement!); const sidebar = await screen.findByTestId('library-sidebar'); const { findByRole, findByText } = within(sidebar); @@ -276,29 +276,39 @@ describe('', () => { axiosMock .onPatch(getLibraryContainerChildrenApiUrl(mockGetContainerMetadata.containerId)) .reply(200); - closestCenter.mockReturnValue([{ id: 'lb:org1:Demo_course:html:text-1' }]); - await act(async () => { - fireEvent.keyDown(firstDragHandle, { code: 'Space' }); - }); + verticalSortableListCollisionDetection.mockReturnValue([{ id: 'lb:org1:Demo_course:html:text-1----1' }]); await act(async () => { fireEvent.keyDown(firstDragHandle, { code: 'Space' }); }); + setTimeout(() => fireEvent.keyDown(firstDragHandle, { code: 'Space' })); await waitFor(() => expect(mockShowToast).toHaveBeenLastCalledWith('Order updated')); }); + it('should cancel update order api on cancelling dragging component', async () => { + renderLibraryUnitPage(); + const firstDragHandle = (await screen.findAllByRole('button', { name: 'Drag to reorder' }))[0]; + axiosMock + .onPatch(getLibraryContainerChildrenApiUrl(mockGetContainerMetadata.containerId)) + .reply(200); + verticalSortableListCollisionDetection.mockReturnValue([{ id: 'lb:org1:Demo_course:html:text-1----1' }]); + await act(async () => { + fireEvent.keyDown(firstDragHandle, { code: 'Space' }); + }); + setTimeout(() => fireEvent.keyDown(firstDragHandle, { code: 'Escape' })); + await waitFor(() => expect(mockShowToast).not.toHaveBeenLastCalledWith('Order updated')); + }); + it('should show toast error message on update order failure', async () => { renderLibraryUnitPage(); const firstDragHandle = (await screen.findAllByRole('button', { name: 'Drag to reorder' }))[0]; axiosMock .onPatch(getLibraryContainerChildrenApiUrl(mockGetContainerMetadata.containerId)) .reply(500); - closestCenter.mockReturnValue([{ id: 'lb:org1:Demo_course:html:text-1' }]); - await act(async () => { - fireEvent.keyDown(firstDragHandle, { code: 'Space' }); - }); + verticalSortableListCollisionDetection.mockReturnValue([{ id: 'lb:org1:Demo_course:html:text-1----1' }]); await act(async () => { fireEvent.keyDown(firstDragHandle, { code: 'Space' }); }); + setTimeout(() => fireEvent.keyDown(firstDragHandle, { code: 'Space' })); await waitFor(() => expect(mockShowToast).toHaveBeenLastCalledWith('Failed to update components order')); }); @@ -311,7 +321,7 @@ describe('', () => { const menu = screen.getAllByRole('button', { name: /component actions menu/i })[0]; fireEvent.click(menu); - const removeButton = await screen.getByText('Remove from unit'); + const removeButton = await screen.findByText('Remove from unit'); fireEvent.click(removeButton); await waitFor(() => { @@ -342,7 +352,7 @@ describe('', () => { const menu = screen.getAllByRole('button', { name: /component actions menu/i })[0]; fireEvent.click(menu); - const removeButton = await screen.getByText('Remove from unit'); + const removeButton = await screen.findByText('Remove from unit'); fireEvent.click(removeButton); await waitFor(() => { @@ -360,7 +370,7 @@ describe('', () => { const menu = screen.getAllByRole('button', { name: /component actions menu/i })[0]; fireEvent.click(menu); - const removeButton = await screen.getByText('Remove from unit'); + const removeButton = await screen.findByText('Remove from unit'); fireEvent.click(removeButton); await waitFor(() => { @@ -388,7 +398,7 @@ describe('', () => { renderLibraryUnitPage(); const component = await screen.findByText('text block 0'); - userEvent.click(component); + userEvent.click(component.parentElement!.parentElement!.parentElement!); const sidebar = await screen.findByTestId('library-sidebar'); const { findByRole, findByText } = within(sidebar); @@ -409,7 +419,7 @@ describe('', () => { renderLibraryUnitPage(); const component = await screen.findByText('text block 0'); // trigger double click - userEvent.click(component, undefined, { clickCount: 2 }); + userEvent.click(component.parentElement!.parentElement!.parentElement!, undefined, { clickCount: 2 }); expect(await screen.findByRole('dialog', { name: 'Editor Dialog' })).toBeInTheDocument(); }); }); diff --git a/src/library-authoring/units/LibraryUnitPage.tsx b/src/library-authoring/units/LibraryUnitPage.tsx index a84bed2a9..a103a9896 100644 --- a/src/library-authoring/units/LibraryUnitPage.tsx +++ b/src/library-authoring/units/LibraryUnitPage.tsx @@ -91,7 +91,7 @@ const HeaderActions = () => { } else { openUnitInfoSidebar(unitId); } - navigateTo({ unitId }); + navigateTo({ unitId, componentId: '' }); }, [unitId, infoSidebarIsOpen]); return ( @@ -123,15 +123,24 @@ export const LibraryUnitPage = () => { const { libraryId, unitId, - collectionId, componentId, + collectionId, } = useLibraryContext(); const { - sidebarComponentInfo, openInfoSidebar, + sidebarComponentInfo, setDefaultTab, setHiddenTabs, } = useSidebarContext(); + const { navigateTo } = useLibraryRoutes(); + + // Open unit or component sidebar on mount + useEffect(() => { + // includes componentId to open correct sidebar on page mount from url + openInfoSidebar(componentId, collectionId, unitId); + // avoid including componentId in dependencies to prevent flicker on closing sidebar. + // See below useEffect that clears componentId on closing sidebar. + }, [unitId, collectionId]); useEffect(() => { setDefaultTab({ @@ -150,10 +159,6 @@ export const LibraryUnitPage = () => { }; }, [setDefaultTab, setHiddenTabs]); - useEffect(() => { - openInfoSidebar(componentId, collectionId, unitId); - }, [componentId, unitId, collectionId]); - if (!unitId || !libraryId) { // istanbul ignore next - This shouldn't be possible; it's just here to satisfy the type checker. throw new Error('Rendered without unitId or libraryId URL parameter'); @@ -232,7 +237,7 @@ export const LibraryUnitPage = () => { className="library-authoring-sidebar box-shadow-left-1 bg-white" data-testid="library-sidebar" > - + navigateTo({ componentId: '' })} /> )} diff --git a/src/library-authoring/units/index.scss b/src/library-authoring/units/index.scss index 91b5d78b1..038de081a 100644 --- a/src/library-authoring/units/index.scss +++ b/src/library-authoring/units/index.scss @@ -14,6 +14,11 @@ &:focus { // this is required for clicks to be passed to underlying iframe component pointer-events: none; + outline: solid 1px $dark-500; + } + + &::before { + border: none; } } diff --git a/src/utils.js b/src/utils.js index 00daa2be3..d763a6246 100644 --- a/src/utils.js +++ b/src/utils.js @@ -1,4 +1,4 @@ -import { useContext, useEffect } from 'react'; +import { useState, useContext, useEffect } from 'react'; import { useDispatch, useSelector } from 'react-redux'; import { useMediaQuery } from 'react-responsive'; import * as Yup from 'yup'; @@ -301,3 +301,22 @@ export const getFileSizeToClosestByte = (fileSize) => { const fileSizeFixedDecimal = Number.parseFloat(size).toFixed(2); return `${fileSizeFixedDecimal} ${units[divides]}`; }; + +/** +* A generic hook to run callback on next render cycle. +* @param {} callback - Callback function that needs to be run later +*/ +export const useRunOnNextRender = (callback) => { + const [scheduled, setScheduled] = useState(false); + + useEffect(() => { + if (!scheduled) { + return; + } + + setScheduled(false); + callback(); + }, [scheduled]); + + return () => setScheduled(true); +};