Skip to content

Commit

Permalink
feat: store sidebar tab and additional action in query string
Browse files Browse the repository at this point in the history
Related changes:

* adds "manage-team" sidebar action to trigger LibraryInfo's "Manage Team" modal
* moves useStateWithUrlSearchParam up to hooks.ts so it can be used by
  search-manager and library-authoring
* splits sidebarCurrentTab and additionalAction out of SidebarComponentInfo
  -- they're now managed independently as url parameters.

  This also simplifies setSidebarComponentInfo: we can simply set the
  new id + key, and so there's no need to merge the ...prev state and
  new state.
* shortens some sidebar property names:
   sidebarCurrentTab => sidebarTab
   sidebarAdditionalAction => sidebarAction
* test: Tab changes now trigger a navigate() call, which invalidates the
  within(sidebar) element in the tests. So using screen instead.
  • Loading branch information
pomegranited committed Dec 19, 2024
1 parent fc2f467 commit b1fe66e
Show file tree
Hide file tree
Showing 10 changed files with 154 additions and 113 deletions.
51 changes: 49 additions & 2 deletions src/hooks.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
import { useEffect, useState } from 'react';
import {
type Dispatch,
type SetStateAction,
useCallback,
useEffect,
useState,
} from 'react';
import { history } from '@edx/frontend-platform';
import { useLocation } from 'react-router-dom';
import { useLocation, useSearchParams } from 'react-router-dom';

export const useScrollToHashElement = ({ isLoading }: { isLoading: boolean }) => {
const [elementWithHash, setElementWithHash] = useState<string | null>(null);
Expand Down Expand Up @@ -77,3 +83,44 @@ export const useLoadOnScroll = (
return () => { };
}, [hasNextPage, isFetchingNextPage, fetchNextPage]);
};

/**
* Hook which stores state variables in the URL search parameters.
*
* It wraps useState with functions that get/set a query string
* search parameter when returning/setting the state variable.
*
* @param defaultValue: Type
* Returned when no valid value is found in the url search parameter.
* @param paramName: name of the url search parameter to store this value in.
* @param fromString: returns the Type equivalent of the given string value,
* or undefined if the value is invalid.
* @param toString: returns the string equivalent of the given Type value.
* Return defaultValue to clear the url search paramName.
*/
export function useStateWithUrlSearchParam<Type>(
defaultValue: Type,
paramName: string,
fromString: (value: string | null) => Type | undefined,
toString: (value: Type) => string | undefined,
): [value: Type, setter: Dispatch<SetStateAction<Type>>] {
const [searchParams, setSearchParams] = useSearchParams();
const returnValue: Type = fromString(searchParams.get(paramName)) ?? defaultValue;
// Function to update the url search parameter
const returnSetter: Dispatch<SetStateAction<Type>> = useCallback((value: Type) => {
setSearchParams((prevParams) => {
const paramValue: string = toString(value) ?? '';
const newSearchParams = new URLSearchParams(prevParams);
// If using the default paramValue, remove it from the search params.
if (paramValue === defaultValue) {
newSearchParams.delete(paramName);
} else {
newSearchParams.set(paramName, paramValue);
}
return newSearchParams;
}, { replace: true });
}, [setSearchParams]);

// Return the computed value and wrapped set state function
return [returnValue, returnSetter];
}
12 changes: 4 additions & 8 deletions src/library-authoring/LibraryAuthoringPage.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -431,27 +431,23 @@ describe('<LibraryAuthoringPage />', () => {
// Click on the first collection
fireEvent.click((await screen.findByText('Collection 1')));

const sidebar = screen.getByTestId('library-sidebar');

const { getByRole } = within(sidebar);

// Click on the Details tab
fireEvent.click(getByRole('tab', { name: 'Details' }));
fireEvent.click(screen.getByRole('tab', { name: 'Details' }));

// Change to a component
fireEvent.click((await screen.findAllByText('Introduction to Testing'))[0]);

// Check that the Details tab is still selected
expect(getByRole('tab', { name: 'Details' })).toHaveAttribute('aria-selected', 'true');
expect(screen.getByRole('tab', { name: 'Details' })).toHaveAttribute('aria-selected', 'true');

// Click on the Previews tab
fireEvent.click(getByRole('tab', { name: 'Preview' }));
fireEvent.click(screen.getByRole('tab', { name: 'Preview' }));

// Switch back to the collection
fireEvent.click((await screen.findByText('Collection 1')));

// The Manage (default) tab should be selected because the collection does not have a Preview tab
expect(getByRole('tab', { name: 'Manage' })).toHaveAttribute('aria-selected', 'true');
expect(screen.getByRole('tab', { name: 'Manage' })).toHaveAttribute('aria-selected', 'true');
});

it('can filter by capa problem type', async () => {
Expand Down
8 changes: 4 additions & 4 deletions src/library-authoring/collections/CollectionInfo.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,11 @@ const CollectionInfo = () => {

const { componentPickerMode } = useComponentPickerContext();
const { libraryId, setCollectionId } = useLibraryContext();
const { sidebarComponentInfo, setSidebarCurrentTab } = useSidebarContext();
const { sidebarComponentInfo, sidebarTab, setSidebarTab } = useSidebarContext();

const tab: CollectionInfoTab = (
sidebarComponentInfo?.currentTab && isCollectionInfoTab(sidebarComponentInfo.currentTab)
) ? sidebarComponentInfo?.currentTab : COLLECTION_INFO_TABS.Manage;
sidebarTab && isCollectionInfoTab(sidebarTab)
) ? sidebarTab : COLLECTION_INFO_TABS.Manage;

const collectionId = sidebarComponentInfo?.id;
// istanbul ignore if: this should never happen
Expand Down Expand Up @@ -70,7 +70,7 @@ const CollectionInfo = () => {
className="my-3 d-flex justify-content-around"
defaultActiveKey={COMPONENT_INFO_TABS.Manage}
activeKey={tab}
onSelect={setSidebarCurrentTab}
onSelect={setSidebarTab}
>
<Tab eventKey={COMPONENT_INFO_TABS.Manage} title={intl.formatMessage(messages.manageTabTitle)}>
<ContentTagsDrawer
Expand Down
92 changes: 54 additions & 38 deletions src/library-authoring/common/context/SidebarContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
useMemo,
useState,
} from 'react';
import { useStateWithUrlSearchParam } from '../../../hooks';

export enum SidebarBodyComponentId {
AddContent = 'add-content',
Expand Down Expand Up @@ -32,29 +33,36 @@ export const isComponentInfoTab = (tab: string): tab is ComponentInfoTab => (
Object.values<string>(COMPONENT_INFO_TABS).includes(tab)
);

type SidebarInfoTab = ComponentInfoTab | CollectionInfoTab;
const toSidebarInfoTab = (tab: string): SidebarInfoTab | undefined => (
isComponentInfoTab(tab) || isCollectionInfoTab(tab)
? tab : undefined
);

export interface SidebarComponentInfo {
type: SidebarBodyComponentId;
id: string;
/** Additional action on Sidebar display */
additionalAction?: SidebarAdditionalActions;
/** Current tab in the sidebar */
currentTab?: CollectionInfoTab | ComponentInfoTab;
}

export enum SidebarAdditionalActions {
export enum SidebarActions {
JumpToAddCollections = 'jump-to-add-collections',
ManageTeam = 'manage-team',
None = '',
}

export type SidebarContextData = {
closeLibrarySidebar: () => void;
openAddContentSidebar: () => void;
openInfoSidebar: (componentId?: string, collectionId?: string) => void;
openLibrarySidebar: () => void;
openCollectionInfoSidebar: (collectionId: string, additionalAction?: SidebarAdditionalActions) => void;
openComponentInfoSidebar: (usageKey: string, additionalAction?: SidebarAdditionalActions) => void;
openCollectionInfoSidebar: (collectionId: string) => void;
openComponentInfoSidebar: (usageKey: string) => void;
sidebarComponentInfo?: SidebarComponentInfo;
resetSidebarAdditionalActions: () => void;
setSidebarCurrentTab: (tab: CollectionInfoTab | ComponentInfoTab) => void;
sidebarAction: SidebarActions;
setSidebarAction: (action: SidebarActions) => void;
resetSidebarAction: () => void;
sidebarTab: SidebarInfoTab;
setSidebarTab: (tab: SidebarInfoTab) => void;
};

/**
Expand Down Expand Up @@ -82,12 +90,22 @@ export const SidebarProvider = ({
initialSidebarComponentInfo,
);

/** Helper function to consume additional action once performed.
Required to redo the action.
*/
const resetSidebarAdditionalActions = useCallback(() => {
setSidebarComponentInfo((prev) => (prev && { ...prev, additionalAction: undefined }));
}, []);
const [sidebarTab, setSidebarTab] = useStateWithUrlSearchParam<SidebarInfoTab>(
COMPONENT_INFO_TABS.Preview,
'st',
(value: string) => toSidebarInfoTab(value),
(value: SidebarInfoTab) => value.toString(),
);

const [sidebarAction, setSidebarAction] = useStateWithUrlSearchParam<SidebarActions>(
SidebarActions.None,
'sa',
(value: string) => Object.values(SidebarActions).find((enumValue) => value === enumValue),
(value: SidebarActions) => value.toString(),
);
const resetSidebarAction = useCallback(() => {
setSidebarAction(SidebarActions.None);
}, [setSidebarAction]);

const closeLibrarySidebar = useCallback(() => {
setSidebarComponentInfo(undefined);
Expand All @@ -99,25 +117,18 @@ export const SidebarProvider = ({
setSidebarComponentInfo({ id: '', type: SidebarBodyComponentId.Info });
}, []);

const openComponentInfoSidebar = useCallback((usageKey: string, additionalAction?: SidebarAdditionalActions) => {
setSidebarComponentInfo((prev) => ({
...prev,
const openComponentInfoSidebar = useCallback((usageKey: string) => {
setSidebarComponentInfo({
id: usageKey,
type: SidebarBodyComponentId.ComponentInfo,
additionalAction,
}));
});
}, []);

const openCollectionInfoSidebar = useCallback((
newCollectionId: string,
additionalAction?: SidebarAdditionalActions,
) => {
setSidebarComponentInfo((prev) => ({
...prev,
const openCollectionInfoSidebar = useCallback((newCollectionId: string) => {
setSidebarComponentInfo({
id: newCollectionId,
type: SidebarBodyComponentId.CollectionInfo,
additionalAction,
}));
});
}, []);

const openInfoSidebar = useCallback((componentId?: string, collectionId?: string) => {
Expand All @@ -130,10 +141,6 @@ export const SidebarProvider = ({
}
}, []);

const setSidebarCurrentTab = useCallback((tab: CollectionInfoTab | ComponentInfoTab) => {
setSidebarComponentInfo((prev) => (prev && { ...prev, currentTab: tab }));
}, []);

const context = useMemo<SidebarContextData>(() => {
const contextValue = {
closeLibrarySidebar,
Expand All @@ -143,8 +150,11 @@ export const SidebarProvider = ({
openComponentInfoSidebar,
sidebarComponentInfo,
openCollectionInfoSidebar,
resetSidebarAdditionalActions,
setSidebarCurrentTab,
sidebarAction,
setSidebarAction,
resetSidebarAction,
sidebarTab,
setSidebarTab,
};

return contextValue;
Expand All @@ -156,8 +166,11 @@ export const SidebarProvider = ({
openComponentInfoSidebar,
sidebarComponentInfo,
openCollectionInfoSidebar,
resetSidebarAdditionalActions,
setSidebarCurrentTab,
sidebarAction,
setSidebarAction,
resetSidebarAction,
sidebarTab,
setSidebarTab,
]);

return (
Expand All @@ -178,8 +191,11 @@ export function useSidebarContext(): SidebarContextData {
openLibrarySidebar: () => {},
openComponentInfoSidebar: () => {},
openCollectionInfoSidebar: () => {},
resetSidebarAdditionalActions: () => {},
setSidebarCurrentTab: () => {},
sidebarAction: SidebarActions.None,
setSidebarAction: () => {},
resetSidebarAction: () => {},
sidebarTab: COMPONENT_INFO_TABS.Preview,
setSidebarTab: () => {},
sidebarComponentInfo: undefined,
};
}
Expand Down
24 changes: 16 additions & 8 deletions src/library-authoring/component-info/ComponentInfo.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { useLibraryContext } from '../common/context/LibraryContext';
import {
type ComponentInfoTab,
COMPONENT_INFO_TABS,
SidebarAdditionalActions,
SidebarActions,
isComponentInfoTab,
useSidebarContext,
} from '../common/context/SidebarContext';
Expand Down Expand Up @@ -101,25 +101,33 @@ const ComponentInfo = () => {
const intl = useIntl();

const { readOnly, openComponentEditor } = useLibraryContext();
const { setSidebarCurrentTab, sidebarComponentInfo, resetSidebarAdditionalActions } = useSidebarContext();
const {
sidebarTab,
setSidebarTab,
sidebarComponentInfo,
sidebarAction,
resetSidebarAction,
} = useSidebarContext();

const jumpToCollections = sidebarComponentInfo?.additionalAction === SidebarAdditionalActions.JumpToAddCollections;
const jumpToCollections = sidebarAction === SidebarActions.JumpToAddCollections;

const tab: ComponentInfoTab = (
sidebarComponentInfo?.currentTab && isComponentInfoTab(sidebarComponentInfo.currentTab)
) ? sidebarComponentInfo?.currentTab : COMPONENT_INFO_TABS.Preview;
isComponentInfoTab(sidebarTab)
? sidebarTab
: COMPONENT_INFO_TABS.Preview

Check warning on line 117 in src/library-authoring/component-info/ComponentInfo.tsx

View check run for this annotation

Codecov / codecov/patch

src/library-authoring/component-info/ComponentInfo.tsx#L117

Added line #L117 was not covered by tests
);

useEffect(() => {
// Show Manage tab if JumpToAddCollections action is set in sidebarComponentInfo
if (jumpToCollections) {
setSidebarCurrentTab(COMPONENT_INFO_TABS.Manage);
setSidebarTab(COMPONENT_INFO_TABS.Manage);

Check warning on line 123 in src/library-authoring/component-info/ComponentInfo.tsx

View check run for this annotation

Codecov / codecov/patch

src/library-authoring/component-info/ComponentInfo.tsx#L123

Added line #L123 was not covered by tests
}
}, [jumpToCollections]);

useEffect(() => {
// This is required to redo actions.
if (tab !== COMPONENT_INFO_TABS.Manage) {
resetSidebarAdditionalActions();
resetSidebarAction();
}
}, [tab]);

Expand Down Expand Up @@ -169,7 +177,7 @@ const ComponentInfo = () => {
className="my-3 d-flex justify-content-around"
defaultActiveKey={COMPONENT_INFO_TABS.Preview}
activeKey={tab}
onSelect={setSidebarCurrentTab}
onSelect={setSidebarTab}
>
<Tab eventKey={COMPONENT_INFO_TABS.Preview} title={intl.formatMessage(messages.previewTabTitle)}>
<ComponentPreview />
Expand Down
8 changes: 4 additions & 4 deletions src/library-authoring/component-info/ComponentManagement.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
} from '@openedx/paragon/icons';

import { useLibraryContext } from '../common/context/LibraryContext';
import { SidebarAdditionalActions, useSidebarContext } from '../common/context/SidebarContext';
import { SidebarActions, useSidebarContext } from '../common/context/SidebarContext';
import { useLibraryBlockMetadata } from '../data/apiHooks';
import StatusWidget from '../generic/status-widget';
import messages from './messages';
Expand All @@ -18,8 +18,8 @@ import ManageCollections from './ManageCollections';
const ComponentManagement = () => {
const intl = useIntl();
const { readOnly, isLoadingLibraryData } = useLibraryContext();
const { sidebarComponentInfo, resetSidebarAdditionalActions } = useSidebarContext();
const jumpToCollections = sidebarComponentInfo?.additionalAction === SidebarAdditionalActions.JumpToAddCollections;
const { sidebarComponentInfo, sidebarAction, resetSidebarAction } = useSidebarContext();
const jumpToCollections = sidebarAction === SidebarActions.JumpToAddCollections;
const [tagsCollapseIsOpen, setTagsCollapseOpen] = React.useState(!jumpToCollections);
const [collectionsCollapseIsOpen, setCollectionsCollapseOpen] = React.useState(true);

Expand All @@ -33,7 +33,7 @@ const ComponentManagement = () => {
useEffect(() => {
// This is required to redo actions.
if (tagsCollapseIsOpen || !collectionsCollapseIsOpen) {
resetSidebarAdditionalActions();
resetSidebarAction();
}
}, [tagsCollapseIsOpen, collectionsCollapseIsOpen]);

Expand Down
Loading

0 comments on commit b1fe66e

Please sign in to comment.