Skip to content

Commit

Permalink
feat: redirect to unit page if the hit or its parent is a unit
Browse files Browse the repository at this point in the history
  • Loading branch information
rpenido committed Apr 24, 2024
1 parent c32462e commit 9901d9b
Show file tree
Hide file tree
Showing 8 changed files with 156 additions and 36 deletions.
15 changes: 15 additions & 0 deletions src/course-unit/CourseUnit.scss
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,18 @@
@import "./course-xblock/CourseXBlock";
@import "./sidebar/Sidebar";
@import "./header-title/HeaderTitle";

div.xblock-highlight {
animation: 5s glow;
animation-timing-function: cubic-bezier(1, 0, .72, .04);
}

@keyframes glow {
0% {
box-shadow: 0 0 5px 5px $primary-500;
}

100% {
box-shadow: unset;
}
}
16 changes: 12 additions & 4 deletions src/course-unit/course-xblock/CourseXBlock.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
import { EditOutline as EditIcon, MoreVert as MoveVertIcon } from '@openedx/paragon/icons';
import { useIntl } from '@edx/frontend-platform/i18n';
import { useSelector } from 'react-redux';
import { useNavigate } from 'react-router-dom';
import { useNavigate, useSearchParams } from 'react-router-dom';

import DeleteModal from '../../generic/delete-modal/DeleteModal';
import ConfigureModal from '../../generic/configure-modal/ConfigureModal';
Expand All @@ -28,6 +28,10 @@ const CourseXBlock = ({
const courseId = useSelector(getCourseId);
const intl = useIntl();

const [searchParams] = useSearchParams();
const locatorId = searchParams.get('show');
const isScrolledToElement = locatorId === id;

const visibilityMessage = userPartitionInfo.selectedGroupsLabel
? intl.formatMessage(messages.visibilityMessage, { selectedGroupsLabel: userPartitionInfo.selectedGroupsLabel })
: null;
Expand Down Expand Up @@ -61,13 +65,17 @@ const CourseXBlock = ({

useEffect(() => {
// if this item has been newly added, scroll to it.
if (courseXBlockElementRef.current && shouldScroll) {
if (courseXBlockElementRef.current && (shouldScroll || isScrolledToElement)) {
scrollToElement(courseXBlockElementRef.current);
}
}, []);
}, [isScrolledToElement]);

return (
<div ref={courseXBlockElementRef} {...props}>
<div
ref={courseXBlockElementRef}
{...props}
className={isScrolledToElement ? 'xblock-highlight' : undefined}
>
<Card className="mb-1">
<Card.Header
title={title}
Expand Down
14 changes: 12 additions & 2 deletions src/course-unit/hooks.jsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { useEffect, useState } from 'react';
import { useDispatch, useSelector } from 'react-redux';
import { useNavigate } from 'react-router-dom';
import { useNavigate, useSearchParams } from 'react-router-dom';

import { RequestStatus } from '../data/constants';
import {
Expand All @@ -27,6 +27,7 @@ import { PUBLISH_TYPES } from './constants';
// eslint-disable-next-line import/prefer-default-export
export const useCourseUnit = ({ courseId, blockId }) => {
const dispatch = useDispatch();
const [searchParams] = useSearchParams();

const [isErrorAlert, toggleErrorAlert] = useState(false);
const [hasInternetConnectionError, setInternetConnectionError] = useState(false);
Expand Down Expand Up @@ -76,7 +77,16 @@ export const useCourseUnit = ({ courseId, blockId }) => {

const handleNavigate = (id) => {
if (sequenceId) {
navigate(`/course/${courseId}/container/${blockId}/${id}`, { replace: true });
const path = `/course/${courseId}/container/${blockId}/${id}`;
const options = { replace: true };
if (searchParams.size) {
navigate({

Check warning on line 83 in src/course-unit/hooks.jsx

View check run for this annotation

Codecov / codecov/patch

src/course-unit/hooks.jsx#L83

Added line #L83 was not covered by tests
pathname: path,
search: `?${searchParams}`,
}, options);
} else {
navigate(path, options);
}
}
};

Expand Down
5 changes: 5 additions & 0 deletions src/custom.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,8 @@ declare module '*.svg' {
const content: string;
export default content;
}

declare module '*.json' {
const value: any;
export default value;
}
111 changes: 94 additions & 17 deletions src/search-modal/SearchResult.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,80 @@ function getItemIcon(blockType) {
return STRUCTURAL_TYPE_ICONS[blockType] ?? COMPONENT_TYPE_ICON_MAP[blockType] ?? Article;
}

/**
* Returns the URL Suffix for library/library component hit
* @param {import('./data/api').ContentHit} hit
* @param {string} libraryAuthoringMfeUrl
* @returns string
*/
function getLibraryHitUrl(hit, libraryAuthoringMfeUrl) {
const { contextKey } = hit;
return `${libraryAuthoringMfeUrl}library/${contextKey}`;

Check warning on line 44 in src/search-modal/SearchResult.jsx

View check run for this annotation

Codecov / codecov/patch

src/search-modal/SearchResult.jsx#L42-L44

Added lines #L42 - L44 were not covered by tests
}

/**
* Returns the URL Suffix for a unit hit
* @param {import('./data/api').ContentHit} hit
* @returns string
*/
function getUnitUrlSuffix(hit) {
const { contextKey, usageKey } = hit;
return `course/${contextKey}/container/${usageKey}`;

Check warning on line 54 in src/search-modal/SearchResult.jsx

View check run for this annotation

Codecov / codecov/patch

src/search-modal/SearchResult.jsx#L52-L54

Added lines #L52 - L54 were not covered by tests
}

/**
* Returns the URL Suffix for a unit component hit
* @param {import('./data/api').ContentHit} hit
* @returns string
*/
function getUnitComponentUrlSuffix(hit) {
const { breadcrumbs, contextKey, usageKey } = hit;
if (breadcrumbs.length > 1) {
const parent = breadcrumbs[breadcrumbs.length - 1];

if ('usageKey' in parent) {
return `course/${contextKey}/container/${parent.usageKey}?show=${encodeURIComponent(usageKey)}`;
}
}

return `course/${contextKey}/`;

Check warning on line 72 in src/search-modal/SearchResult.jsx

View check run for this annotation

Codecov / codecov/patch

src/search-modal/SearchResult.jsx#L72

Added line #L72 was not covered by tests
}

/**
* Returns the URL Suffix for a course component hit
* @param {import('./data/api').ContentHit} hit
* @returns string
*/
function getCourseComponentUrlSuffix(hit) {
const { contextKey, usageKey } = hit;
return `course/${contextKey}?show=${encodeURIComponent(usageKey)}`;

Check warning on line 82 in src/search-modal/SearchResult.jsx

View check run for this annotation

Codecov / codecov/patch

src/search-modal/SearchResult.jsx#L80-L82

Added lines #L80 - L82 were not covered by tests
}

/**
* Returns the URL Suffix for the search hit param
* @param {import('./data/api').ContentHit} hit
* @returns string
*/
function getUrlSuffix(hit) {
const { blockType, breadcrumbs } = hit;

// Check if is a unit
if (blockType === 'vertical') {
return getUnitUrlSuffix(hit);

Check warning on line 95 in src/search-modal/SearchResult.jsx

View check run for this annotation

Codecov / codecov/patch

src/search-modal/SearchResult.jsx#L95

Added line #L95 was not covered by tests
}

// Check if the parent is a unit
if (breadcrumbs.length > 1) {
const parent = breadcrumbs[breadcrumbs.length - 1];

if ('usageKey' in parent && parent.usageKey.includes('type@vertical')) {
return getUnitComponentUrlSuffix(hit);
}
}

return getCourseComponentUrlSuffix(hit);

Check warning on line 107 in src/search-modal/SearchResult.jsx

View check run for this annotation

Codecov / codecov/patch

src/search-modal/SearchResult.jsx#L107

Added line #L107 was not covered by tests
}

/**
* A single search result (row), usually represents an XBlock/Component
* @type {React.FC<{hit: import('./data/api').ContentHit}>}
Expand All @@ -43,33 +117,34 @@ const SearchResult = ({ hit }) => {
const { closeSearchModal } = useSearchContext();
const { libraryAuthoringMfeUrl, redirectToLibraryAuthoringMfe } = useSelector(getStudioHomeData);

const { usageKey } = hit;

const noRedirectUrl = usageKey.startsWith('lb:') && !redirectToLibraryAuthoringMfe;

/**
* Returns the URL for the context of the hit
*/
const getContextUrl = React.useCallback((newWindow = false) => {
const { contextKey, usageKey } = hit;
const { contextKey } = hit;

if (contextKey.startsWith('course-v1:')) {
const courseSufix = `course/${contextKey}?show=${encodeURIComponent(usageKey)}`;
const urlSuffix = getUrlSuffix(hit);

if (newWindow) {
return `${getPath(getConfig().PUBLIC_PATH)}${courseSufix}`;
return `${getPath(getConfig().PUBLIC_PATH)}${urlSuffix}`;
}
return `/${courseSufix}`;
return `/${urlSuffix}`;
}

if (usageKey.startsWith('lb:')) {
if (redirectToLibraryAuthoringMfe) {
return `${libraryAuthoringMfeUrl}library/${contextKey}`;
return getLibraryHitUrl(hit, libraryAuthoringMfeUrl);

Check warning on line 141 in src/search-modal/SearchResult.jsx

View check run for this annotation

Codecov / codecov/patch

src/search-modal/SearchResult.jsx#L141

Added line #L141 was not covered by tests
}
}

// No context URL for this hit
return undefined;
}, [libraryAuthoringMfeUrl, redirectToLibraryAuthoringMfe]);

const redirectUrl = React.useMemo(() => getContextUrl(), [libraryAuthoringMfeUrl, redirectToLibraryAuthoringMfe]);
const newWindowUrl = React.useMemo(
() => getContextUrl(true),
[libraryAuthoringMfeUrl, redirectToLibraryAuthoringMfe],
);
}, [libraryAuthoringMfeUrl, redirectToLibraryAuthoringMfe, hit]);

/**
* Opens the context of the hit in a new window
Expand All @@ -78,6 +153,7 @@ const SearchResult = ({ hit }) => {
*/
const openContextInNewWindow = (e) => {
e.stopPropagation();
const newWindowUrl = getContextUrl(true);
/* istanbul ignore next */
if (!newWindowUrl) {
return;
Expand All @@ -90,8 +166,9 @@ const SearchResult = ({ hit }) => {
* @param {(React.MouseEvent | React.KeyboardEvent)} e
* @returns {void}
*/
const navigateToContext = (e) => {
const navigateToContext = React.useCallback((e) => {
e.stopPropagation();
const redirectUrl = getContextUrl();

/* istanbul ignore next */
if (!redirectUrl) {
Expand All @@ -112,16 +189,16 @@ const SearchResult = ({ hit }) => {

navigate(redirectUrl);
closeSearchModal();
};
}, [getContextUrl]);

return (
<Stack
className={`border-bottom search-result p-2 align-items-start ${!redirectUrl ? 'text-muted' : ''}`}
className={`border-bottom search-result p-2 align-items-start ${noRedirectUrl ? 'text-muted' : ''}`}
direction="horizontal"
gap={3}
onClick={navigateToContext}
onKeyDown={navigateToContext}
tabIndex={redirectUrl ? 0 : undefined}
tabIndex={noRedirectUrl ? undefined : 0}
role="button"
>
<Icon className="text-muted" src={getItemIcon(hit.blockType)} />
Expand All @@ -140,7 +217,7 @@ const SearchResult = ({ hit }) => {
<IconButton
src={OpenInNew}
iconAs={Icon}
disabled={!newWindowUrl}
disabled={noRedirectUrl}
onClick={openContextInNewWindow}
alt={intl.formatMessage(messages.openInNewWindow)}
/>
Expand Down
14 changes: 6 additions & 8 deletions src/search-modal/SearchUI.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,11 @@ import {
import fetchMock from 'fetch-mock-jest';

import initializeStore from '../store';
// @ts-ignore
import mockResult from './__mocks__/search-result.json';
// @ts-ignore
import mockEmptyResult from './__mocks__/empty-search-result.json';
// @ts-ignore
import mockTagsFacetResult from './__mocks__/facet-search.json';
// @ts-ignore
import mockTagsFacetResultLevel0 from './__mocks__/facet-search-level0.json';
// @ts-ignore
import mockTagsFacetResultLevel1 from './__mocks__/facet-search-level1.json';
// @ts-ignore
import mockTagsKeywordSearchResult from './__mocks__/tags-keyword-search.json';
import SearchUI from './SearchUI';
import { getContentSearchConfigUrl } from './data/api';
Expand Down Expand Up @@ -168,14 +162,18 @@ describe('<SearchUI />', () => {
window.open = jest.fn();
fireEvent.click(within(resultItem).getByRole('button', { name: 'Open in new window' }));
expect(window.open).toHaveBeenCalledWith(
'/course/course-v1:edx+TestCourse+24?show=block-v1%3Aedx%2BTestCourse%2B24%2Btype%40html%2Bblock%40test_html',
'/course/course-v1:edx+TestCourse+24/container/block-v1:edx+TestCourse+24+type@vertical+block@vertical_3_1'
+ '?show=block-v1%3Aedx%2BTestCourse%2B24%2Btype%40html%2Bblock%40test_html',
'_blank',
);
window.open = open;

// Clicking in the result should navigate to the result's URL:
fireEvent.click(resultItem);
expect(mockNavigate).toHaveBeenCalledWith('/course/course-v1:edx+TestCourse+24?show=block-v1%3Aedx%2BTestCourse%2B24%2Btype%40html%2Bblock%40test_html');
expect(mockNavigate).toHaveBeenCalledWith(
'/course/course-v1:edx+TestCourse+24/container/block-v1:edx+TestCourse+24+type@vertical+block@vertical_3_1'
+ '?show=block-v1%3Aedx%2BTestCourse%2B24%2Btype%40html%2Bblock%40test_html',
);
});

it('defaults to searching "This Course" if used in a course', async () => {
Expand Down
12 changes: 9 additions & 3 deletions src/search-modal/__mocks__/search-result.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,15 @@
"org": "edx",
"breadcrumbs": [
{ "display_name": "TheCourse" },
{ "display_name": "Section 2" },
{ "display_name": "Subsection 3" },
{ "display_name": "The Little Unit That Could" }
{ "display_name": "Section 2", "usage_key": "block-v1:edx+TestCourse+24+type@chapter+block@chapter_2" },
{
"display_name": "Subsection 3",
"usage_key": "block-v1:edx+TestCourse+24+type@sequential+block@sequential_3"
},
{
"display_name": "The Little Unit That Could",
"usage_key": "block-v1:edx+TestCourse+24+type@vertical+block@vertical_3_1"
}
],
"tags": {
"taxonomy": [
Expand Down
5 changes: 3 additions & 2 deletions src/search-modal/data/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,9 @@ function formatTagsFilter(tagsFilter) {
* @property {string} blockType The block_type part of the usage key. What type of XBlock this is.
* @property {string} contextKey The course or library ID
* @property {string} org
* @property {{displayName: string}[]} breadcrumbs First one is the name of the course/library itself.
* After that is the name of any parent Section/Subsection/Unit/etc.
* @property {[{displayName: string}, ...Array<{displayName: string, usageKey: string}>]} breadcrumbs
* First one is the name of the course/library itself.
* After that is the name and usage key of any parent Section/Subsection/Unit/etc.
* @property {Record<'taxonomy'|'level0'|'level1'|'level2'|'level3', string[]>} tags
* @property {ContentDetails} [content]
* @property {{displayName: string, content: ContentDetails}} formatted Same fields with <mark>...</mark> highlights
Expand Down

0 comments on commit 9901d9b

Please sign in to comment.