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: improve search modal results [FC-0040] #946

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
15 changes: 15 additions & 0 deletions src/course-outline/CourseOutline.scss
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,18 @@
@import "./drag-helper/SortableItem";
@import "./xblock-status/XBlockStatus";
@import "./paste-button/PasteButton";

div.row:has(> div > div.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;
}
}
11 changes: 7 additions & 4 deletions src/course-outline/section-card/SectionCard.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { useDispatch } from 'react-redux';
import { useIntl } from '@edx/frontend-platform/i18n';
import { Bubble, Button, useToggle } from '@openedx/paragon';
import { Add as IconAdd } from '@openedx/paragon/icons';
import { useSearchParams } from 'react-router-dom';
import classNames from 'classnames';

import { setCurrentItem, setCurrentSection } from '../data/slice';
Expand Down Expand Up @@ -42,6 +43,9 @@ const SectionCard = ({
const dispatch = useDispatch();
const { activeId, overId } = useContext(DragContext);
const [isExpanded, setIsExpanded] = useState(isSectionsExpanded);
const [searchParams] = useSearchParams();
const locatorId = searchParams.get('show');
const isScrolledToElement = locatorId === section.id;
const [isFormOpen, openForm, closeForm] = useToggle(false);
const namePrefix = 'section';

Expand Down Expand Up @@ -70,11 +74,10 @@ const SectionCard = ({
}, [activeId, overId]);

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

// re-create actions object for customizations
const actions = { ...sectionActions };
Expand Down Expand Up @@ -155,7 +158,7 @@ const SectionCard = ({
}}
>
<div
className="section-card"
className={`section-card ${isScrolledToElement ? 'highlight' : ''}`}
data-testid="section-card"
ref={currentRef}
>
Expand Down
8 changes: 6 additions & 2 deletions src/course-outline/subsection-card/SubsectionCard.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ const SubsectionCard = ({
if (currentRef.current && (section.shouldScroll || subsection.shouldScroll || isScrolledToElement)) {
scrollToElement(currentRef.current);
}
}, []);
}, [isScrolledToElement]);

useEffect(() => {
if (savingStatus === RequestStatus.SUCCESSFUL) {
Expand All @@ -160,7 +160,11 @@ const SubsectionCard = ({
...borderStyle,
}}
>
<div className="subsection-card" data-testid="subsection-card" ref={currentRef}>
<div
className={`subsection-card ${isScrolledToElement ? 'highlight' : ''}`}
data-testid="subsection-card"
ref={currentRef}
>
{isHeaderVisible && (
<>
<CardHeader
Expand Down
10 changes: 7 additions & 3 deletions src/course-outline/unit-card/UnitCard.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import PropTypes from 'prop-types';
import { useDispatch } from 'react-redux';
import { useToggle } from '@openedx/paragon';
import { isEmpty } from 'lodash';
import { useSearchParams } from 'react-router-dom';

import { setCurrentItem, setCurrentSection, setCurrentSubsection } from '../data/slice';
import { RequestStatus } from '../../data/constants';
Expand Down Expand Up @@ -34,6 +35,9 @@ const UnitCard = ({
}) => {
const currentRef = useRef(null);
const dispatch = useDispatch();
const [searchParams] = useSearchParams();
const locatorId = searchParams.get('show');
const isScrolledToElement = locatorId === unit.id;
const [isFormOpen, openForm, closeForm] = useToggle(false);
const namePrefix = 'unit';

Expand Down Expand Up @@ -109,10 +113,10 @@ const UnitCard = ({
// if this items has been newly added, scroll to it.
// we need to check section.shouldScroll as whole section is fetched when a
// unit is duplicated under it.
if (currentRef.current && (section.shouldScroll || unit.shouldScroll)) {
if (currentRef.current && (section.shouldScroll || unit.shouldScroll || isScrolledToElement)) {
scrollToElement(currentRef.current);
}
}, []);
}, [isScrolledToElement]);

useEffect(() => {
if (savingStatus === RequestStatus.SUCCESSFUL) {
Expand All @@ -139,7 +143,7 @@ const UnitCard = ({
}}
>
<div
className="unit-card"
className={`unit-card ${isScrolledToElement ? 'highlight' : ''}`}
data-testid="unit-card"
ref={currentRef}
>
Expand Down
4 changes: 4 additions & 0 deletions src/custom.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
declare module '*.svg' {
const content: string;
export default content;
}
Comment on lines +1 to +4
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To support importing svg files as image source

32 changes: 28 additions & 4 deletions src/search-modal/EmptyStates.jsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,22 @@
/* eslint-disable react/prop-types */
// @ts-check
import React from 'react';
import { FormattedMessage } from '@edx/frontend-platform/i18n';
import { Stack } from '@openedx/paragon';
import { useStats, useClearRefinements } from 'react-instantsearch';

import EmptySearchImage from './images/empty-search.svg';
import NoResultImage from './images/no-results.svg';
import messages from './messages';

const InfoMessage = ({ title, subtitle, image }) => (
<Stack className="d-flex mt-6 align-items-center">
<p className="lead"> <FormattedMessage {...title} /> </p>
<p className="small text-muted"> <FormattedMessage {...subtitle} /> </p>
<img src={image} alt="" />
</Stack>
);

/**
* If the user hasn't put any keywords/filters yet, display an "empty state".
* Likewise, if the results are empty (0 results), display a special message.
Expand All @@ -16,12 +30,22 @@ const EmptyStates = ({ children }) => {

if (!hasQuery && !hasFiltersApplied) {
// We haven't started the search yet. Display the "start your search" empty state
// Note this isn't localized because it's going to be replaced in a fast-follow PR.
return <p className="text-muted text-center mt-6">Enter a keyword or select a filter to begin searching.</p>;
return (
<InfoMessage
title={messages.emptySearchTitle}
subtitle={messages.emptySearchSubtitle}
image={EmptySearchImage}
/>
);
}
if (nbHits === 0) {
// Note this isn't localized because it's going to be replaced in a fast-follow PR.
return <p className="text-muted text-center mt-6">No results found. Change your search and try again.</p>;
return (
<InfoMessage
title={messages.noResultsTitle}
subtitle={messages.noResultsSubtitle}
image={NoResultImage}
/>
);
}

return children;
Expand Down
6 changes: 3 additions & 3 deletions src/search-modal/SearchEndpointLoader.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ import { useContentSearch } from './data/apiHooks';
import SearchUI from './SearchUI';
import messages from './messages';

/** @type {React.FC<{courseId: string}>} */
const SearchEndpointLoader = ({ courseId }) => {
/** @type {React.FC<{courseId: string, closeSearch?: () => void}>} */
const SearchEndpointLoader = ({ courseId, closeSearch }) => {
const intl = useIntl();

// Load the Meilisearch connection details from the LMS: the URL to use, the index name, and an API key specific
Expand All @@ -25,7 +25,7 @@ const SearchEndpointLoader = ({ courseId }) => {
const title = intl.formatMessage(messages.title);

if (searchEndpointData) {
return <SearchUI {...searchEndpointData} courseId={courseId} />;
return <SearchUI {...searchEndpointData} courseId={courseId} closeSearch={closeSearch} />;
}
return (
<>
Expand Down
4 changes: 2 additions & 2 deletions src/search-modal/SearchModal.jsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
/* eslint-disable react/prop-types */
// @ts-check
import React from 'react';
import { ModalDialog } from '@openedx/paragon';
import { useIntl } from '@edx/frontend-platform/i18n';
import { ModalDialog } from '@openedx/paragon';

import SearchEndpointLoader from './SearchEndpointLoader';
import messages from './messages';
Expand All @@ -24,7 +24,7 @@ const SearchModal = ({ courseId, ...props }) => {
isFullscreenOnMobile
className="courseware-search-modal"
>
<SearchEndpointLoader courseId={courseId} />
<SearchEndpointLoader courseId={courseId} closeSearch={props.onClose} />
</ModalDialog>
);
};
Expand Down
13 changes: 10 additions & 3 deletions src/search-modal/SearchModal.scss
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,15 @@
}
}

.ais-InfiniteHits-loadPrevious,
.ais-InfiniteHits-loadMore--disabled {
display: none; // temporary; remove this once we implement our own <Hits>/<InfiniteHits> component.
.search-result {
&:hover {
background-color: $gray-100;
cursor: pointer;
}

&:hover.text-muted {
cursor: unset;
background-color: unset;
}
}
}
7 changes: 6 additions & 1 deletion src/search-modal/SearchModal.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ const queryClient = new QueryClient({
},
});

jest.mock('react-redux', () => ({
...jest.requireActual('react-redux'),
useSelector: jest.fn(),
}));

const RootWrapper = () => (
<AppProvider store={store}>
<IntlProvider locale="en" messages={{}}>
Expand Down Expand Up @@ -59,7 +64,7 @@ describe('<SearchModal />', () => {
apiKey: 'test-api-key',
});
const { findByText } = render(<RootWrapper />);
expect(await findByText('Enter a keyword or select a filter to begin searching.')).toBeInTheDocument();
expect(await findByText('Start searching to find content')).toBeInTheDocument();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add more tests to cover the added functionality for this SearchModal?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More tests: 879a395, 66612bc, 58a5c8c

});

it('should render the spinner while the config is loading', () => {
Expand Down
Loading
Loading