Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: shareable URLs for library components and searches [FC-0076] #1575

Merged
merged 19 commits into from
Jan 10, 2025
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
aa25827
fix: use generatePath in testUtils
pomegranited Dec 18, 2024
a4728e1
fix: test uses contentId=undefined, so made this param optional in th…
pomegranited Dec 19, 2024
a6465cb
Merge remote-tracking branch 'origin/master' into jill/fal-3984-shara…
pomegranited Dec 19, 2024
ce4c27e
feat: adds sharable URLs for library components/collections
pomegranited Dec 12, 2024
fc2f467
feat: store search keywords in query string
pomegranited Dec 19, 2024
f6b46c4
feat: store sidebar tab and additional action in query string
pomegranited Dec 19, 2024
37370d2
feat: store selected tags in the URL search string
pomegranited Dec 20, 2024
1736942
refactor: use one typesFilter state in SearchManager
pomegranited Dec 20, 2024
a3ee610
docs: adds more comments to the library authoring routes
pomegranited Dec 20, 2024
9d3e049
Merge remote-tracking branch 'origin/master' into jill/fal-3984-shara…
pomegranited Dec 30, 2024
5054020
fix: clicking "Library Info" navigates to the Library URL
pomegranited Dec 30, 2024
88dfb0e
fix: use sidebarAction instead of managing separate state
pomegranited Dec 30, 2024
9ae67f5
refactor: use getActiveKey to initialize the tab state
pomegranited Dec 30, 2024
2f28e3b
refactor: allow Type or Type[] values in useStateUrlSearchParams
pomegranited Dec 30, 2024
b4c70ca
fix: disable filter by type on collections tab
pomegranited Jan 1, 2025
ced360e
test: fixes broken tests
pomegranited Jan 1, 2025
d060113
test: clear search filters in between tests
pomegranited Jan 1, 2025
4c1855d
test: split long-running "can filter by capa problem type" test into two
pomegranited Jan 1, 2025
39fd745
test: adds tests for library authoring routes.ts
pomegranited Jan 10, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/content-tags-drawer/ContentTagsDrawer.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {
} from './data/api.mocks';
import { getContentTaxonomyTagsApiUrl } from './data/api';

const path = '/content/:contentId/*';
const path = '/content/:contentId?/*';
const mockOnClose = jest.fn();
const mockSetBlockingSheet = jest.fn();
const mockNavigate = jest.fn();
Expand Down
132 changes: 130 additions & 2 deletions src/hooks.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
import { useEffect, useState } from 'react';
import {
type Dispatch,
type SetStateAction,
useCallback,
useEffect,
useRef,
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 +84,124 @@ export const useLoadOnScroll = (
return () => { };
}, [hasNextPage, isFetchingNextPage, fetchNextPage]);
};

/**
* Types used by the useListHelpers and useStateWithUrlSearchParam hooks.
*/
export type FromStringFn<Type> = (value: string | null) => Type | undefined;
export type ToStringFn<Type> = (value: Type | undefined) => string | undefined;

/**
* Hook that stores/retrieves state variables using the URL search parameters.
*
* @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: FromStringFn<Type>,
toString: ToStringFn<Type>,
): [value: Type, setter: Dispatch<SetStateAction<Type>>] {
// STATE WORKAROUND:
// If we use this hook to control multiple state parameters on the same
// page, we can run into state update issues. Because our state variables
// are actually stored in setSearchParams, and not in separate variables like
// useState would do, the searchParams "previous" state may not be updated
// for sequential calls to returnSetter in the same render loop (like in
// SearchManager's clearFilters).
//
// One workaround could be to use window.location.search as the "previous"
// value when returnSetter constructs the new URLSearchParams. This works
// fine with BrowserRouter, but our test suite uses MemoryRouter, and that
// router doesn't store URL search params, cf
// https://github.com/remix-run/react-router/issues/9757
//
// So instead, we maintain a reference to the current useLocation()
// object, and use its search params as the "previous" value when
// initializing URLSearchParams.
const location = useLocation();
const locationRef = useRef(location);
const [searchParams, setSearchParams] = useSearchParams();

const returnValue: Type = fromString(searchParams.get(paramName)) ?? defaultValue;
// Update the url search parameter using:
type ReturnSetterParams = (
// a Type value
value?: Type
// or a function that returns a Type from the previous returnValue
| ((value: Type) => Type)
) => void;
const returnSetter: Dispatch<SetStateAction<Type>> = useCallback<ReturnSetterParams>((value) => {
setSearchParams((/* prev */) => {
const useValue = value instanceof Function ? value(returnValue) : value;
const paramValue = toString(useValue);
const newSearchParams = new URLSearchParams(locationRef.current.search);
// If the provided value was invalid (toString returned undefined)
// or the same as the defaultValue, remove it from the search params.
if (paramValue === undefined || paramValue === defaultValue) {
newSearchParams.delete(paramName);
} else {
newSearchParams.set(paramName, paramValue);
}

// Update locationRef
locationRef.current.search = newSearchParams.toString();

return newSearchParams;
}, { replace: true });
}, [returnValue, setSearchParams]);

// Return the computed value and wrapped set state function
return [returnValue, returnSetter];
}

/**
* Helper hook for useStateWithUrlSearchParam<Type[]>.
*
* useListHelpers provides toString and fromString handlers that can:
* - split/join a list of values using a separator string, and
* - validate each value using the provided functions, omitting any invalid values.
*
* @param fromString
* Serialize a string to a Type, or undefined if not valid.
* @param toString
* Deserialize a Type to a string.
* @param separator : string to use when splitting/joining the types.
* Defaults value is ','.
*/
export function useListHelpers<Type>({
fromString,
toString,
separator = ',',
}: {
fromString: FromStringFn<Type>,
toString: ToStringFn<Type>,
separator?: string;
}): [ FromStringFn<Type[]>, ToStringFn<Type[]> ] {
pomegranited marked this conversation as resolved.
Show resolved Hide resolved
const isType = (item: Type | undefined): item is Type => item !== undefined;

// Split the given string with separator,
// and convert the parts to a list of Types, omiting any invalid Types.
const fromStringToList : FromStringFn<Type[]> = (value: string) => (
value
? value.split(separator).map(fromString).filter(isType)
: []
);
// Convert an array of Types to strings and join with separator.
// Returns undefined if the given list contains no valid Types.
const fromListToString : ToStringFn<Type[]> = (value: Type[]) => {
const stringValues = value.map(toString).filter((val) => val !== undefined);
return (
stringValues && stringValues.length
? stringValues.join(separator)
: undefined
);
};
return [fromStringToList, fromListToString];
}
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
59 changes: 34 additions & 25 deletions src/library-authoring/LibraryAuthoringPage.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { useEffect, useState } from 'react';
import { useCallback, useEffect, useState } from 'react';
import { Helmet } from 'react-helmet';
import classNames from 'classnames';
import { StudioFooter } from '@edx/frontend-component-footer';
Expand All @@ -16,12 +16,7 @@ import {
Tabs,
} from '@openedx/paragon';
import { Add, ArrowBack, InfoOutline } from '@openedx/paragon/icons';
import {
Link,
useLocation,
useNavigate,
useSearchParams,
} from 'react-router-dom';
import { Link } from 'react-router-dom';

import Loading from '../generic/Loading';
import SubHeader from '../generic/sub-header/SubHeader';
Expand All @@ -36,11 +31,12 @@ import {
SearchKeywordsField,
SearchSortWidget,
} from '../search-manager';
import LibraryContent, { ContentType } from './LibraryContent';
import LibraryContent from './LibraryContent';
import { LibrarySidebar } from './library-sidebar';
import { useComponentPickerContext } from './common/context/ComponentPickerContext';
import { useLibraryContext } from './common/context/LibraryContext';
import { SidebarBodyComponentId, useSidebarContext } from './common/context/SidebarContext';
import { ContentType, useLibraryRoutes } from './routes';

import messages from './messages';

Expand All @@ -51,7 +47,7 @@ const HeaderActions = () => {

const {
openAddContentSidebar,
openInfoSidebar,
openLibrarySidebar,
closeLibrarySidebar,
sidebarComponentInfo,
} = useSidebarContext();
Expand All @@ -60,13 +56,19 @@ const HeaderActions = () => {

const infoSidebarIsOpen = sidebarComponentInfo?.type === SidebarBodyComponentId.Info;

const handleOnClickInfoSidebar = () => {
const { navigateTo } = useLibraryRoutes();
const handleOnClickInfoSidebar = useCallback(() => {
if (infoSidebarIsOpen) {
closeLibrarySidebar();
} else {
openInfoSidebar();
openLibrarySidebar();
}
};

if (!componentPickerMode) {
// Reset URL to library home
navigateTo();
}
}, [navigateTo, sidebarComponentInfo, closeLibrarySidebar, openLibrarySidebar]);

return (
<div className="header-actions">
Expand Down Expand Up @@ -124,8 +126,6 @@ interface LibraryAuthoringPageProps {

const LibraryAuthoringPage = ({ returnToLibrarySelection }: LibraryAuthoringPageProps) => {
const intl = useIntl();
const location = useLocation();
const navigate = useNavigate();

const {
isLoadingPage: isLoadingStudioHome,
Expand All @@ -139,29 +139,41 @@ const LibraryAuthoringPage = ({ returnToLibrarySelection }: LibraryAuthoringPage
libraryData,
isLoadingLibraryData,
showOnlyPublished,
componentId,
collectionId,
} = useLibraryContext();
const { openInfoSidebar, sidebarComponentInfo } = useSidebarContext();

const { insideCollections, insideComponents, navigateTo } = useLibraryRoutes();

// The activeKey determines the currently selected tab.
const [activeKey, setActiveKey] = useState<ContentType>(ContentType.home);
const getActiveKey = () => {
pomegranited marked this conversation as resolved.
Show resolved Hide resolved
if (insideCollections) {
return ContentType.collections;
}
if (insideComponents) {
return ContentType.components;
}
return ContentType.home;
};

useEffect(() => {
const currentPath = location.pathname.split('/').pop();
const contentType = getActiveKey();

if (componentPickerMode || currentPath === libraryId || currentPath === '') {
if (componentPickerMode) {
setActiveKey(ContentType.home);
} else if (currentPath && currentPath in ContentType) {
setActiveKey(ContentType[currentPath] || ContentType.home);
} else {
setActiveKey(contentType);
}
}, []);

useEffect(() => {
if (!componentPickerMode) {
openInfoSidebar();
openInfoSidebar(componentId, collectionId);
}
}, []);

const [searchParams] = useSearchParams();

if (isLoadingLibraryData) {
return <Loading />;
}
Expand All @@ -181,10 +193,7 @@ const LibraryAuthoringPage = ({ returnToLibrarySelection }: LibraryAuthoringPage
const handleTabChange = (key: ContentType) => {
setActiveKey(key);
if (!componentPickerMode) {
navigate({
pathname: key,
search: searchParams.toString(),
});
navigateTo({ contentType: key });
}
};

Expand Down
7 changes: 1 addition & 6 deletions src/library-authoring/LibraryContent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,10 @@ import { useLibraryContext } from './common/context/LibraryContext';
import { useSidebarContext } from './common/context/SidebarContext';
import CollectionCard from './components/CollectionCard';
import ComponentCard from './components/ComponentCard';
import { ContentType } from './routes';
import { useLoadOnScroll } from '../hooks';
import messages from './collections/messages';

export enum ContentType {
home = '',
components = 'components',
collections = 'collections',
}

/**
* Library Content to show content grid
*
Expand Down
Loading
Loading