From c71964d2b2c0b7d895e7cb6262665fe4774711e9 Mon Sep 17 00:00:00 2001 From: Marcos Date: Mon, 16 Oct 2023 17:22:13 -0300 Subject: [PATCH 01/11] feat: Added Courseware Search container popover --- .../courseware-search/CoursewareSearch.jsx | 87 ++++++++++++++++--- .../CoursewareSearchToggle.jsx | 38 ++++++++ ...st.jsx => CoursewareSearchToggle.test.jsx} | 6 +- .../courseware-search/courseware-search.scss | 45 ++++++++++ src/course-home/courseware-search/hooks.js | 64 ++++++++++++++ src/course-home/courseware-search/index.js | 1 + src/course-home/courseware-search/messages.js | 10 +++ src/course-home/data/slice.js | 5 ++ src/course-tabs/CourseTabsNavigation.jsx | 54 +++++++----- src/index.scss | 1 + 10 files changed, 271 insertions(+), 40 deletions(-) create mode 100644 src/course-home/courseware-search/CoursewareSearchToggle.jsx rename src/course-home/courseware-search/{CoursewareSearch.test.jsx => CoursewareSearchToggle.test.jsx} (88%) create mode 100644 src/course-home/courseware-search/courseware-search.scss create mode 100644 src/course-home/courseware-search/hooks.js diff --git a/src/course-home/courseware-search/CoursewareSearch.jsx b/src/course-home/courseware-search/CoursewareSearch.jsx index a6bff1f23f..2f0f17bda3 100644 --- a/src/course-home/courseware-search/CoursewareSearch.jsx +++ b/src/course-home/courseware-search/CoursewareSearch.jsx @@ -1,25 +1,84 @@ -import React, { useState, useEffect } from 'react'; -import { useParams } from 'react-router-dom'; +import React from 'react'; +import { useDispatch } from 'react-redux'; import { injectIntl, intlShape } from '@edx/frontend-platform/i18n'; import { Button, Icon } from '@edx/paragon'; -import { Search } from '@edx/paragon/icons'; +import { + Close, +} from '@edx/paragon/icons'; +import { setShowSearch } from '../data/slice'; +import { useElementBoundingBox, useLockScroll } from './hooks'; import messages from './messages'; -import { fetchCoursewareSearchSettings } from '../data/thunks'; -const CoursewareSearch = ({ intl, ...rest }) => { - const { courseId } = useParams(); - const [enabled, setEnabled] = useState(false); +const CoursewareSearch = ({ intl }) => { + const dispatch = useDispatch(); - useEffect(() => { - fetchCoursewareSearchSettings(courseId).then(response => setEnabled(response.enabled)); - }, [courseId]); + useLockScroll(); - if (!enabled) { return null; } + const info = useElementBoundingBox('courseTabsNavigation'); + + const top = info ? `${Math.floor(info.top)}px` : 0; return ( - +
+
+ +
+
+
+

{intl.formatMessage(messages.searchModuleTitle)}

+

+ Lorem ipsum dolor sit amet, consectetur adipiscing elit. Duis semper rutrum odio quis congue. + Duis sodales nibh et sapien elementum fermentum. Quisque magna urna, gravida at gravida et, + ultricies vel massa.Aliquam in vehicula dolor, id scelerisque felis. + Morbi posuere scelerisque tincidunt. Proin et gravida tortor. Vestibulum vel orci vulputate, + gravida justo eu, varius dolor. Etiam viverra diam sed est tincidunt, et aliquam est efficitur. + Donec imperdiet eros quis est condimentum faucibus. +

+

+ In mattis, tellus ut lacinia viverra, ligula ex sagittis ex, sed mollis ex enim ut velit. + Nunc elementum, risus eget feugiat scelerisque, sapien felis laoreet nisl, ut pharetra neque + lorem a elit. Maecenas elementum, metus fringilla suscipit imperdiet, mi nunc efficitur elit, + sed consequat massa magna sit amet dui. Curabitur ultrices nisi vel lorem scelerisque, pharetra + luctus nunc pulvinar. Morbi aliquam ante eget arcu condimentum consectetur. Fusce faucibus lacus + sed pretium ultrices. Curabitur neque lacus, elementum convallis augue placerat, gravida + scelerisque ipsum. Donec bibendum lectus id ullamcorper sodales. Integer quis ante facilisis erat + maximus viverra. Nunc rutrum posuere lectus, aliquam congue odio blandit nec. Phasellus placerat, + magna non bibendum lacinia, tortor orci vulputate dui, vitae imperdiet turpis dui nec tortor. + Praesent porttitor mollis diam ut gravida. Praesent vitae felis dignissim sem accumsan dignissim. + Fusce ullamcorper bibendum ante ac pellentesque. Aliquam sed leo vel leo pellentesque cursus a at risus. + Donec sollicitudin maximus diam, sit amet molestie sapien commodo at. +

+

+ Cras ornare pulvinar est id rhoncus. Aenean ut risus magna. Fusce cursus pulvinar dui ut egestas. + Quisque condimentum risus non mi sagittis, eu facilisis enim hendrerit. Integer faucibus dapibus rutrum. + Nullam vitae mollis tortor, eu lacinia mi. Nunc commodo ex id eros hendrerit, vel interdum augue tristique. + Suspendisse ullamcorper, purus in vestibulum auctor, justo nisi finibus dolor, + nec dignissim arcu enim a augue. +

+

+ Fusce vel libero odio. Orci varius natoque penatibus et magnis dis parturient montes, + nascetur ridiculus mus. Pellentesque at varius turpis. Ut pulvinar efficitur congue. Vivamus cursus, + purus at aliquet malesuada, felis quam blandit dolor, a interdum justo est semper augue. + In eu lectus sit amet est pellentesque porta vel eget magna. Morbi sollicitudin turpis vitae faucibus + pulvinar. Etiam placerat pulvinar porta. +

+

+ Suspendisse mattis eget felis non sagittis. Nulla facilisi. In bibendum cursus purus, non venenatis tellus + dignissim sit amet. Phasellus volutpat ipsum turpis, non imperdiet nisi elementum a. Nunc mollis, sapien + cursus vehicula consectetur, nunc turpis pulvinar mauris, at varius justo mi egestas nisi. Fusce semper + sapien in orci rhoncus ornare. Donec maximus mi eu pulvinar convallis. +

+

+ Nullam tortor sem, hendrerit eu sapien ac, venenatis rhoncus ligula. Donec nibh leo, venenatis sed interdum + ac, pharetra sed nibh. Orci varius natoque penatibus et magnis dis parturient montes, + nascetur ridiculus mus. Sed congue risus eu mattis condimentum. In id nulla sit amet magna suscipit + consectetur. Nullam vitae augue felis. In consequat tempus diam, a eleifend ante bibendum ac. + Vivamus mi orci, fermentum ac viverra quis, tristique a ipsum. Morbi imperdiet porta sem, in sollicitudin + risus dignissim at. Nulla dapibus iaculis vestibulum. +

+
+
+
); }; diff --git a/src/course-home/courseware-search/CoursewareSearchToggle.jsx b/src/course-home/courseware-search/CoursewareSearchToggle.jsx new file mode 100644 index 0000000000..bb77a254be --- /dev/null +++ b/src/course-home/courseware-search/CoursewareSearchToggle.jsx @@ -0,0 +1,38 @@ +import React from 'react'; +import { injectIntl, intlShape } from '@edx/frontend-platform/i18n'; +import { Button, Icon } from '@edx/paragon'; +import { Search } from '@edx/paragon/icons'; +import { useDispatch } from 'react-redux'; +import { setShowSearch } from '../data/slice'; +import messages from './messages'; +import { useCoursewareSearchFeatureFlag } from './hooks'; + +const CoursewareSearchToggle = ({ + intl, +}) => { + const dispatch = useDispatch(); + const enabled = useCoursewareSearchFeatureFlag(); + + if (!enabled) { return null; } + + return ( +
+ +
+ ); +}; + +CoursewareSearchToggle.propTypes = { + intl: intlShape.isRequired, +}; + +export default injectIntl(CoursewareSearchToggle); diff --git a/src/course-home/courseware-search/CoursewareSearch.test.jsx b/src/course-home/courseware-search/CoursewareSearchToggle.test.jsx similarity index 88% rename from src/course-home/courseware-search/CoursewareSearch.test.jsx rename to src/course-home/courseware-search/CoursewareSearchToggle.test.jsx index a84f7ef685..85aab7c9e8 100644 --- a/src/course-home/courseware-search/CoursewareSearch.test.jsx +++ b/src/course-home/courseware-search/CoursewareSearchToggle.test.jsx @@ -7,16 +7,16 @@ import { waitFor, } from '../../setupTest'; import { fetchCoursewareSearchSettings } from '../data/thunks'; -import { CoursewareSearch } from './index'; +import { CoursewareSearchToggle } from './index'; jest.mock('../data/thunks'); function renderComponent() { - const { container } = render(); + const { container } = render(); return container; } -describe('CoursewareSearch', () => { +describe('CoursewareSearchToggle', () => { beforeAll(async () => { initializeMockApp(); }); diff --git a/src/course-home/courseware-search/courseware-search.scss b/src/course-home/courseware-search/courseware-search.scss new file mode 100644 index 0000000000..18cbebea11 --- /dev/null +++ b/src/course-home/courseware-search/courseware-search.scss @@ -0,0 +1,45 @@ +.courseware-search { + background: white; + position: fixed; + top: var(--modal-top-position, 0); + left: 0; + right: 0; + bottom: 0; + border-top: 1px solid $light-300; + z-index: 200; + + &__close { + position: absolute !important; // For some reason it gets overridden + top: 0.5rem; + right: 1rem; + font-size: 1.5rem; + line-height: 1; + } + + &__outer-content { + overflow-y: auto; + height: 100%; + } + + &__content { + padding-top: 2rem; + padding-left: 1rem; + padding-right: 1rem; + max-width: 42rem; + margin: auto; + + h2 { + margin-bottom: 2rem; + } + } +} + +@media (min-width: map-get($grid-breakpoints, 'md')) { + .courseware-search__content { + padding-top: 8rem; + } +} + +body._search-no-scroll { + overflow-y: hidden; +} diff --git a/src/course-home/courseware-search/hooks.js b/src/course-home/courseware-search/hooks.js new file mode 100644 index 0000000000..13e3fa713b --- /dev/null +++ b/src/course-home/courseware-search/hooks.js @@ -0,0 +1,64 @@ +import { useState, useLayoutEffect, useEffect } from 'react'; +import { useParams } from 'react-router-dom'; +import { useSelector } from 'react-redux'; +import { fetchCoursewareSearchSettings } from '../data/thunks'; + +export function useCoursewareSearchFeatureFlag() { + const { courseId } = useParams(); + const [enabled, setEnabled] = useState(false); + + useEffect(() => { + fetchCoursewareSearchSettings(courseId).then(response => setEnabled(response.enabled)); + }, [courseId]); + + return enabled; +} + +export function useCoursewareSearchState() { + const enabled = useCoursewareSearchFeatureFlag(); + const show = useSelector(state => state.courseHome.showSearch); + + return { show: enabled && show }; +} + +export function useElementBoundingBox(elementId) { + const [elementInfo, setElementInfo] = useState(undefined); + + useLayoutEffect(() => { + // Handler to call on window resize and scroll + function recalculate() { + const element = document.getElementById(elementId); + if (!element) { + throw new Error(`Unable to find element with id="${elementId}" in the document.`); + } + const info = element.getBoundingClientRect(); + setElementInfo(info); + } + + // Add event listener + global.addEventListener('resize', recalculate); + global.addEventListener('scroll', recalculate); + + // Call handler right away so state gets updated with initial window size + recalculate(); + + // Remove event listener on cleanup + return () => { + global.removeEventListener('resize', recalculate); + global.removeEventListener('scroll', recalculate); + }; + }, []); // Empty array ensures that effect is only run on mount + + return elementInfo; +} + +export function useLockScroll() { + useEffect(() => { + window.scrollTo(0, 0); + document.body.classList.add('_search-no-scroll'); + + return () => { + document.body.classList.remove('_search-no-scroll'); + }; + }, []); +} diff --git a/src/course-home/courseware-search/index.js b/src/course-home/courseware-search/index.js index c773bbcbed..78a701274b 100644 --- a/src/course-home/courseware-search/index.js +++ b/src/course-home/courseware-search/index.js @@ -1,2 +1,3 @@ /* eslint-disable import/prefer-default-export */ +export { default as CoursewareSearchToggle } from './CoursewareSearchToggle'; export { default as CoursewareSearch } from './CoursewareSearch'; diff --git a/src/course-home/courseware-search/messages.js b/src/course-home/courseware-search/messages.js index a857249429..dee1753bb3 100644 --- a/src/course-home/courseware-search/messages.js +++ b/src/course-home/courseware-search/messages.js @@ -6,6 +6,16 @@ const messages = defineMessages({ defaultMessage: 'Search within this course', description: 'Aria-label for a button that will pop up Courseware Search.', }, + searchCloseAction: { + id: 'learn.coursewareSerch.closeAction', + defaultMessage: 'Close the search form', + description: 'Aria-label for a button that will close Courseware Search.', + }, + searchModuleTitle: { + id: 'learn.coursewareSerch.searchModuleTitle', + defaultMessage: 'Search this course', + description: 'Title for the Courseware Search module.', + }, }); export default messages; diff --git a/src/course-home/data/slice.js b/src/course-home/data/slice.js index cefe91836c..e23801db07 100644 --- a/src/course-home/data/slice.js +++ b/src/course-home/data/slice.js @@ -15,6 +15,7 @@ const slice = createSlice({ toastBodyText: null, toastBodyLink: null, toastHeader: '', + showSearch: false, }, reducers: { fetchProctoringInfoResolved: (state) => { @@ -47,6 +48,9 @@ const slice = createSlice({ state.toastBodyText = linkText; state.toastHeader = header; }, + setShowSearch: (state, { payload }) => { + state.showSearch = payload; + }, }, }); @@ -57,6 +61,7 @@ export const { fetchTabRequest, fetchTabSuccess, setCallToActionToast, + setShowSearch, } = slice.actions; export const { diff --git a/src/course-tabs/CourseTabsNavigation.jsx b/src/course-tabs/CourseTabsNavigation.jsx index 2157c9f9cc..288c6c847b 100644 --- a/src/course-tabs/CourseTabsNavigation.jsx +++ b/src/course-tabs/CourseTabsNavigation.jsx @@ -5,33 +5,41 @@ import classNames from 'classnames'; import messages from './messages'; import Tabs from '../generic/tabs/Tabs'; -import { CoursewareSearch } from '../course-home/courseware-search'; +import { CoursewareSearch, CoursewareSearchToggle } from '../course-home/courseware-search'; +import { useCoursewareSearchState } from '../course-home/courseware-search/hooks'; const CourseTabsNavigation = ({ activeTabSlug, className, tabs, intl, -}) => ( -
-
- -
-
- - {tabs.map(({ url, title, slug }) => ( - - {title} - - ))} - +}) => { + const { show } = useCoursewareSearchState(); + + return ( +
+
+ +
+
+ + {tabs.map(({ url, title, slug }) => ( + + {title} + + ))} + +
+ {show ? ( + + ) : undefined}
-
-); + ); +}; CourseTabsNavigation.propTypes = { activeTabSlug: PropTypes.string, diff --git a/src/index.scss b/src/index.scss index 555c5b8f7e..55faac53fd 100755 --- a/src/index.scss +++ b/src/index.scss @@ -391,3 +391,4 @@ @import "course-home/progress-tab/grades/course-grade/GradeBar.scss"; @import "courseware/course/course-exit/CourseRecommendations"; @import "product-tours/newUserCourseHomeTour/NewUserCourseHomeTourModal.scss"; +@import "course-home/courseware-search/courseware-search.scss"; From c5be319b93a6d2f08d70ec44bbd658a87ca25fe4 Mon Sep 17 00:00:00 2001 From: Marcos Date: Wed, 18 Oct 2023 10:38:03 -0300 Subject: [PATCH 02/11] chore: Added unit tests for CoursewareSearch and CoursewareSearchToggle --- .../courseware-search/CoursewareSearch.jsx | 12 ++- .../CoursewareSearch.test.jsx | 75 +++++++++++++++++++ .../CoursewareSearchToggle.jsx | 2 +- .../CoursewareSearchToggle.test.jsx | 25 ++++++- src/course-home/courseware-search/hooks.js | 11 ++- 5 files changed, 114 insertions(+), 11 deletions(-) create mode 100644 src/course-home/courseware-search/CoursewareSearch.test.jsx diff --git a/src/course-home/courseware-search/CoursewareSearch.jsx b/src/course-home/courseware-search/CoursewareSearch.jsx index 2f0f17bda3..d9604ff1a8 100644 --- a/src/course-home/courseware-search/CoursewareSearch.jsx +++ b/src/course-home/courseware-search/CoursewareSearch.jsx @@ -15,13 +15,19 @@ const CoursewareSearch = ({ intl }) => { useLockScroll(); const info = useElementBoundingBox('courseTabsNavigation'); - const top = info ? `${Math.floor(info.top)}px` : 0; return ( -
+
- +
diff --git a/src/course-home/courseware-search/CoursewareSearch.test.jsx b/src/course-home/courseware-search/CoursewareSearch.test.jsx new file mode 100644 index 0000000000..170129ef7b --- /dev/null +++ b/src/course-home/courseware-search/CoursewareSearch.test.jsx @@ -0,0 +1,75 @@ +import React from 'react'; +import { + fireEvent, + initializeMockApp, + render, + screen, +} from '../../setupTest'; +import { CoursewareSearch } from './index'; +import { setShowSearch } from '../data/slice'; +import { useElementBoundingBox, useLockScroll } from './hooks'; + +const mockDispatch = jest.fn(); + +jest.mock('./hooks'); +jest.mock('../data/slice'); +jest.mock('react-redux', () => ({ + ...jest.requireActual('react-redux'), + useDispatch: () => mockDispatch, +})); + +const tabsTopPosition = 128; + +function renderComponent() { + const { container } = render(); + return container; +} + +describe('CoursewareSearch', () => { + beforeAll(async () => { + initializeMockApp(); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + describe('when rendering normally', () => { + beforeAll(() => { + useElementBoundingBox.mockImplementation(() => ({ top: tabsTopPosition })); + renderComponent(); + }); + + it('Should use useElementBoundingBox() and useLockScroll() hooks', () => { + expect(useElementBoundingBox).toBeCalledTimes(1); + expect(useLockScroll).toBeCalledTimes(1); + }); + + it('Should have a "--modal-top-position" CSS variable matching the elementBoundingBox top position', () => { + renderComponent(); + + const section = screen.getByTestId('courseware-search-section'); + expect(section.style.getPropertyValue('--modal-top-position')).toBe(`${tabsTopPosition}px`); + }); + + it('Should dispatch setShowSearch(true) when clicking the close button', () => { + renderComponent(); + + const button = screen.getByTestId('courseware-search-close-button'); + fireEvent.click(button); + expect(mockDispatch).toHaveBeenCalledTimes(1); + expect(setShowSearch).toHaveBeenCalledTimes(1); + expect(setShowSearch).toHaveBeenCalledWith(false); + }); + }); + + describe('when CourseTabsNavigation is not present', () => { + it('Should use "--modal-top-position: 0" if the elementBoundingBox returns undefined', () => { + useElementBoundingBox.mockImplementation(() => undefined); + renderComponent(); + + const section = screen.getByTestId('courseware-search-section'); + expect(section.style.getPropertyValue('--modal-top-position')).toBe('0'); + }); + }); +}); diff --git a/src/course-home/courseware-search/CoursewareSearchToggle.jsx b/src/course-home/courseware-search/CoursewareSearchToggle.jsx index bb77a254be..0a5707bfca 100644 --- a/src/course-home/courseware-search/CoursewareSearchToggle.jsx +++ b/src/course-home/courseware-search/CoursewareSearchToggle.jsx @@ -22,8 +22,8 @@ const CoursewareSearchToggle = ({ size="sm" className="p-1 mt-2 mr-2 rounded-lg" aria-label={intl.formatMessage(messages.searchOpenAction)} - data-testid="courseware-search-button" onClick={() => dispatch(setShowSearch(true))} + data-testid="courseware-search-open-button" > diff --git a/src/course-home/courseware-search/CoursewareSearchToggle.test.jsx b/src/course-home/courseware-search/CoursewareSearchToggle.test.jsx index 85aab7c9e8..fcf5de3bab 100644 --- a/src/course-home/courseware-search/CoursewareSearchToggle.test.jsx +++ b/src/course-home/courseware-search/CoursewareSearchToggle.test.jsx @@ -1,15 +1,24 @@ import React from 'react'; import { act, + fireEvent, initializeMockApp, render, screen, waitFor, } from '../../setupTest'; import { fetchCoursewareSearchSettings } from '../data/thunks'; +import { setShowSearch } from '../data/slice'; import { CoursewareSearchToggle } from './index'; +const mockDispatch = jest.fn(); + jest.mock('../data/thunks'); +jest.mock('../data/slice'); +jest.mock('react-redux', () => ({ + ...jest.requireActual('react-redux'), + useDispatch: () => mockDispatch, +})); function renderComponent() { const { container } = render(); @@ -21,7 +30,7 @@ describe('CoursewareSearchToggle', () => { initializeMockApp(); }); - beforeEach(() => { + afterEach(() => { jest.clearAllMocks(); }); @@ -31,7 +40,7 @@ describe('CoursewareSearchToggle', () => { await act(async () => renderComponent()); await waitFor(() => { expect(fetchCoursewareSearchSettings).toHaveBeenCalledTimes(1); - expect(screen.queryByTestId('courseware-search-button')).not.toBeInTheDocument(); + expect(screen.queryByTestId('courseware-search-open-button')).not.toBeInTheDocument(); }); }); @@ -40,7 +49,17 @@ describe('CoursewareSearchToggle', () => { await act(async () => renderComponent()); await waitFor(() => { expect(fetchCoursewareSearchSettings).toHaveBeenCalledTimes(1); - expect(screen.queryByTestId('courseware-search-button')).toBeInTheDocument(); + expect(screen.queryByTestId('courseware-search-open-button')).toBeInTheDocument(); }); }); + + it('Should dispatch setShowSearch(true) when clicking the search button', async () => { + fetchCoursewareSearchSettings.mockImplementation(() => Promise.resolve({ enabled: true })); + await act(async () => renderComponent()); + const button = await screen.findByTestId('courseware-search-open-button'); + fireEvent.click(button); + expect(mockDispatch).toHaveBeenCalledTimes(1); + expect(setShowSearch).toHaveBeenCalledTimes(1); + expect(setShowSearch).toHaveBeenCalledWith(true); + }); }); diff --git a/src/course-home/courseware-search/hooks.js b/src/course-home/courseware-search/hooks.js index 13e3fa713b..2be3dd5984 100644 --- a/src/course-home/courseware-search/hooks.js +++ b/src/course-home/courseware-search/hooks.js @@ -24,13 +24,16 @@ export function useCoursewareSearchState() { export function useElementBoundingBox(elementId) { const [elementInfo, setElementInfo] = useState(undefined); + const element = document.getElementById(elementId); + + if (!element) { + console.warn(`useElementBoundingBox(): Unable to find element with id='${elementId}' in the document.'`); // eslint-disable-line no-console + return undefined; + } + useLayoutEffect(() => { // Handler to call on window resize and scroll function recalculate() { - const element = document.getElementById(elementId); - if (!element) { - throw new Error(`Unable to find element with id="${elementId}" in the document.`); - } const info = element.getBoundingClientRect(); setElementInfo(info); } From 0e6ef21c3c758989e0a0c467cfd07b6e7a1447c1 Mon Sep 17 00:00:00 2001 From: Marcos Date: Wed, 18 Oct 2023 11:15:34 -0300 Subject: [PATCH 03/11] chore: Updated unit test for CourseTabsNavigation --- .../courseware-search/CoursewareSearch.jsx | 4 +- .../CoursewareSearch.test.jsx | 19 +++++-- .../CoursewareSearchToggle.test.jsx | 2 + src/course-tabs/CourseTabsNavigation.jsx | 4 +- src/course-tabs/CourseTabsNavigation.test.jsx | 51 +++++++++++++++++-- 5 files changed, 68 insertions(+), 12 deletions(-) diff --git a/src/course-home/courseware-search/CoursewareSearch.jsx b/src/course-home/courseware-search/CoursewareSearch.jsx index d9604ff1a8..452d24b2e8 100644 --- a/src/course-home/courseware-search/CoursewareSearch.jsx +++ b/src/course-home/courseware-search/CoursewareSearch.jsx @@ -9,7 +9,7 @@ import { setShowSearch } from '../data/slice'; import { useElementBoundingBox, useLockScroll } from './hooks'; import messages from './messages'; -const CoursewareSearch = ({ intl }) => { +const CoursewareSearch = ({ intl, ...sectionProps }) => { const dispatch = useDispatch(); useLockScroll(); @@ -18,7 +18,7 @@ const CoursewareSearch = ({ intl }) => { const top = info ? `${Math.floor(info.top)}px` : 0; return ( -
+
{show ? ( - - ) : undefined} + + ) : null}
); }; diff --git a/src/course-tabs/CourseTabsNavigation.test.jsx b/src/course-tabs/CourseTabsNavigation.test.jsx index ae3eb9a6a4..af93569306 100644 --- a/src/course-tabs/CourseTabsNavigation.test.jsx +++ b/src/course-tabs/CourseTabsNavigation.test.jsx @@ -1,14 +1,46 @@ import React from 'react'; -import { initializeMockApp, render, screen } from '../setupTest'; +import { AppProvider } from '@edx/frontend-platform/react'; +import { + initializeMockApp, render, screen, +} from '../setupTest'; +import { useCoursewareSearchState } from '../course-home/courseware-search/hooks'; import { CourseTabsNavigation } from './index'; +import initializeStore from '../store'; + +jest.mock('../course-home/courseware-search/hooks'); + +const mockDispatch = jest.fn(); + +jest.mock('react-redux', () => ({ + ...jest.requireActual('react-redux'), + useDispatch: () => mockDispatch, +})); + +function renderComponent(props = { tabs: [] }) { + const store = initializeStore(); + const { container } = render( + + + , + ); + return container; +} describe('Course Tabs Navigation', () => { beforeAll(async () => { initializeMockApp(); }); + beforeEach(() => { + useCoursewareSearchState.mockImplementation(() => ({ show: false })); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + it('renders without tabs', () => { - render(); + renderComponent(); expect(screen.getByRole('button', { name: 'More...' })).toBeInTheDocument(); }); @@ -21,7 +53,7 @@ describe('Course Tabs Navigation', () => { tabs, activeTabSlug: tabs[0].slug, }; - render(); + renderComponent(mockData); expect(screen.getByRole('link', { name: tabs[0].title })).toHaveAttribute('href', tabs[0].url); expect(screen.getByRole('link', { name: tabs[0].title })).toHaveClass('active'); @@ -29,4 +61,17 @@ describe('Course Tabs Navigation', () => { expect(screen.getByRole('link', { name: tabs[1].title })).toHaveAttribute('href', tabs[1].url); expect(screen.getByRole('link', { name: tabs[1].title })).not.toHaveClass('active'); }); + + it('should NOT render CoursewareSearch if the flag is off', () => { + renderComponent(); + + expect(screen.queryByTestId('courseware-search')).not.toBeInTheDocument(); + }); + + it('should render CoursewareSearch if the flag is on', () => { + useCoursewareSearchState.mockImplementation(() => ({ show: true })); + renderComponent(); + + expect(screen.queryByTestId('courseware-search')).toBeInTheDocument(); + }); }); From eeeaaafe036ef0e0e47ed2830533142738c1ca07 Mon Sep 17 00:00:00 2001 From: Marcos Date: Wed, 18 Oct 2023 20:02:10 -0300 Subject: [PATCH 04/11] chore: Partial coverage on Courseware Search Hooks --- package-lock.json | 59 ++++---- package.json | 1 + src/course-home/courseware-search/hooks.js | 35 +++-- .../courseware-search/hooks.test.jsx | 143 ++++++++++++++++++ 4 files changed, 195 insertions(+), 43 deletions(-) create mode 100644 src/course-home/courseware-search/hooks.test.jsx diff --git a/package-lock.json b/package-lock.json index aaae67d0ce..fffb2f4b03 100644 --- a/package-lock.json +++ b/package-lock.json @@ -31,6 +31,7 @@ "@fortawesome/react-fontawesome": "^0.1.4", "@popperjs/core": "2.11.8", "@reduxjs/toolkit": "1.8.1", + "@testing-library/react-hooks": "^8.0.1", "classnames": "2.3.2", "core-js": "3.22.2", "history": "5.3.0", @@ -3869,35 +3870,6 @@ "node": ">=14" } }, - "node_modules/@edx/react-unit-test-utils/node_modules/@testing-library/react-hooks": { - "version": "8.0.1", - "resolved": "https://registry.npmjs.org/@testing-library/react-hooks/-/react-hooks-8.0.1.tgz", - "integrity": "sha512-Aqhl2IVmLt8IovEVarNDFuJDVWVvhnr9/GCU6UUnrYXwgDFF9h2L2o2P9KBni1AST5sT6riAyoukFLyjQUgD/g==", - "dependencies": { - "@babel/runtime": "^7.12.5", - "react-error-boundary": "^3.1.0" - }, - "engines": { - "node": ">=12" - }, - "peerDependencies": { - "@types/react": "^16.9.0 || ^17.0.0", - "react": "^16.9.0 || ^17.0.0", - "react-dom": "^16.9.0 || ^17.0.0", - "react-test-renderer": "^16.9.0 || ^17.0.0" - }, - "peerDependenciesMeta": { - "@types/react": { - "optional": true - }, - "react-dom": { - "optional": true - }, - "react-test-renderer": { - "optional": true - } - } - }, "node_modules/@edx/react-unit-test-utils/node_modules/axios": { "version": "0.27.2", "resolved": "https://registry.npmjs.org/axios/-/axios-0.27.2.tgz", @@ -6234,6 +6206,35 @@ "react-dom": "<18.0.0" } }, + "node_modules/@testing-library/react-hooks": { + "version": "8.0.1", + "resolved": "https://registry.npmjs.org/@testing-library/react-hooks/-/react-hooks-8.0.1.tgz", + "integrity": "sha512-Aqhl2IVmLt8IovEVarNDFuJDVWVvhnr9/GCU6UUnrYXwgDFF9h2L2o2P9KBni1AST5sT6riAyoukFLyjQUgD/g==", + "dependencies": { + "@babel/runtime": "^7.12.5", + "react-error-boundary": "^3.1.0" + }, + "engines": { + "node": ">=12" + }, + "peerDependencies": { + "@types/react": "^16.9.0 || ^17.0.0", + "react": "^16.9.0 || ^17.0.0", + "react-dom": "^16.9.0 || ^17.0.0", + "react-test-renderer": "^16.9.0 || ^17.0.0" + }, + "peerDependenciesMeta": { + "@types/react": { + "optional": true + }, + "react-dom": { + "optional": true + }, + "react-test-renderer": { + "optional": true + } + } + }, "node_modules/@testing-library/user-event": { "version": "13.5.0", "resolved": "https://registry.npmjs.org/@testing-library/user-event/-/user-event-13.5.0.tgz", diff --git a/package.json b/package.json index 11a9edfdb7..d4fc1fdee1 100644 --- a/package.json +++ b/package.json @@ -44,6 +44,7 @@ "@fortawesome/react-fontawesome": "^0.1.4", "@popperjs/core": "2.11.8", "@reduxjs/toolkit": "1.8.1", + "@testing-library/react-hooks": "^8.0.1", "classnames": "2.3.2", "core-js": "3.22.2", "history": "5.3.0", diff --git a/src/course-home/courseware-search/hooks.js b/src/course-home/courseware-search/hooks.js index 2be3dd5984..c354c6800f 100644 --- a/src/course-home/courseware-search/hooks.js +++ b/src/course-home/courseware-search/hooks.js @@ -1,8 +1,11 @@ -import { useState, useLayoutEffect, useEffect } from 'react'; +import { useState, useEffect } from 'react'; import { useParams } from 'react-router-dom'; import { useSelector } from 'react-redux'; +import { debounce } from 'lodash'; import { fetchCoursewareSearchSettings } from '../data/thunks'; +const DEBOUNCE_WAIT = 300; // ms + export function useCoursewareSearchFeatureFlag() { const { courseId } = useParams(); const [enabled, setEnabled] = useState(false); @@ -16,43 +19,47 @@ export function useCoursewareSearchFeatureFlag() { export function useCoursewareSearchState() { const enabled = useCoursewareSearchFeatureFlag(); + + if (!enabled) { return { show: false }; } + const show = useSelector(state => state.courseHome.showSearch); - return { show: enabled && show }; + return { show }; } export function useElementBoundingBox(elementId) { - const [elementInfo, setElementInfo] = useState(undefined); + const [info, setInfo] = useState(undefined); const element = document.getElementById(elementId); if (!element) { - console.warn(`useElementBoundingBox(): Unable to find element with id='${elementId}' in the document.'`); // eslint-disable-line no-console + console.warn(`useElementBoundingBox(): Unable to find element with id='${elementId}' in the document.`); // eslint-disable-line no-console return undefined; } - useLayoutEffect(() => { + useEffect(() => { // Handler to call on window resize and scroll function recalculate() { - const info = element.getBoundingClientRect(); - setElementInfo(info); + const bounds = element.getBoundingClientRect(); + setInfo(bounds); } + const debouncedRecalculate = debounce(recalculate, DEBOUNCE_WAIT, { leading: true }); // Add event listener - global.addEventListener('resize', recalculate); - global.addEventListener('scroll', recalculate); + global.addEventListener('resize', debouncedRecalculate); + global.addEventListener('scroll', debouncedRecalculate); // Call handler right away so state gets updated with initial window size - recalculate(); + debouncedRecalculate(); // Remove event listener on cleanup return () => { - global.removeEventListener('resize', recalculate); - global.removeEventListener('scroll', recalculate); + global.removeEventListener('resize', debouncedRecalculate); + global.removeEventListener('scroll', debouncedRecalculate); }; - }, []); // Empty array ensures that effect is only run on mount + }, []); - return elementInfo; + return info; } export function useLockScroll() { diff --git a/src/course-home/courseware-search/hooks.test.jsx b/src/course-home/courseware-search/hooks.test.jsx new file mode 100644 index 0000000000..6f6d06c579 --- /dev/null +++ b/src/course-home/courseware-search/hooks.test.jsx @@ -0,0 +1,143 @@ +import { renderHook, act } from '@testing-library/react-hooks'; +import { useParams } from 'react-router-dom'; +import { useSelector } from 'react-redux'; +import { fetchCoursewareSearchSettings } from '../data/thunks'; +import { useCoursewareSearchFeatureFlag, useCoursewareSearchState, useElementBoundingBox } from './hooks'; + +jest.mock('react-redux'); +jest.mock('react-router-dom'); +jest.mock('../data/thunks'); + +describe('CoursewareSearch Hooks', () => { + const courses = { + 123: { enabled: true }, + 456: { enabled: false }, + }; + + beforeEach(() => { + fetchCoursewareSearchSettings.mockImplementation((courseId) => Promise.resolve(courses[courseId])); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + describe('useCoursewareSearchFeatureFlag', () => { + const renderTestHook = async (enabled = true) => { + useParams.mockImplementation(() => ({ courseId: enabled ? 123 : 456 })); + let state; + await act(async () => { (state = renderHook(() => useCoursewareSearchFeatureFlag())); }); + return state; + }; + + test('should return true if feature is enabled', async () => { + const state = await renderTestHook(); + await state.waitFor(() => expect(fetchCoursewareSearchSettings).toBeCalledTimes(1)); + expect(state.result.current).toBe(true); + }); + + test('should return false if feature is disabled', async () => { + const state = await renderTestHook(false); + await state.waitFor(() => expect(fetchCoursewareSearchSettings).toBeCalledTimes(1)); + expect(state.result.current).toBe(false); + }); + }); + + describe('useCoursewareSearchState', () => { + const renderTestHook = async ({ enabled, showSearch }) => { + useParams.mockImplementation(() => ({ courseId: enabled ? 123 : 456 })); + const mockedStoreState = { courseHome: { showSearch } }; + useSelector.mockImplementation(selector => selector(mockedStoreState)); + + let state; + await act(async () => { (state = renderHook(() => useCoursewareSearchState())); }); + return state; + }; + + test('should return show: true if feature is enabled and showSearch is true', async () => { + const state = await renderTestHook({ enabled: true, showSearch: true }); + + expect(state.result.current).toEqual({ show: true }); + }); + + test('should return show: false in any other case', async () => { + let state; + + state = await renderTestHook({ enabled: true, showSearch: false }); + expect(state.result.current).toEqual({ show: false }); + + state = await renderTestHook({ enabled: false, showSearch: true }); + expect(state.result.current).toEqual({ show: false }); + + state = await renderTestHook({ enabled: false, showSearch: false }); + expect(state.result.current).toEqual({ show: false }); + }); + }); + + describe('useElementBoundingBox', () => { + let getBoundingClientRectSpy; + let addEventListenerSpy; + let removeEventListenerSpy; + + const renderTestHook = async ({ elementId, mockedInfo }) => { + getBoundingClientRectSpy = jest.spyOn(document, 'getElementById').mockImplementation(() => ( + mockedInfo + ? { getBoundingClientRect: () => ({ ...mockedInfo }) } + : undefined + )); + + let hook; + await act(async () => { + hook = renderHook(() => useElementBoundingBox(elementId)); + }); + + return hook; + }; + + beforeEach(() => { + addEventListenerSpy = jest.spyOn(global, 'addEventListener'); + removeEventListenerSpy = jest.spyOn(global, 'removeEventListener'); + }); + + describe('when element is present', () => { + const mockedInfo = { top: 128 }; + + test('should bind resize and scroll events on mount', async () => { + await renderTestHook({ elementId: 'test', mockedInfo }); + + expect(addEventListenerSpy).toHaveBeenCalledWith('resize', expect.anything()); + expect(addEventListenerSpy).toHaveBeenCalledWith('scroll', expect.anything()); + }); + + test('should unbindbind resize and scroll events when unmounted', async () => { + const state = await renderTestHook({ elementId: 'test', mockedInfo }); + state.unmount(); + + expect(removeEventListenerSpy).toHaveBeenCalledWith('resize', expect.anything()); + expect(removeEventListenerSpy).toHaveBeenCalledWith('scroll', expect.anything()); + }); + + // This test is failing, the hook state is not being updated. + xtest('should return the element bounding box', async () => { + const state = await renderTestHook({ elementId: 'test', mockedInfo }); + + state.waitFor(() => expect(getBoundingClientRectSpy).toHaveBeenCalled()); + + expect(state.result.current).toEqual(mockedInfo); + }); + + // This test is failing, the hook state is not being updated. + xtest('should call getBoundingClientRect on window resize', async () => { + const state = await renderTestHook({ elementId: 'test', mockedInfo }); + + act(() => { + // Trigger the window resize event. + global.innerWidth = 500; + global.dispatchEvent(new Event('resize')); + }); + + expect(state.result.current).toEqual(mockedInfo); + }); + }); + }); +}); From ec3b09473be470768634e2a7d3931d1870a75007 Mon Sep 17 00:00:00 2001 From: Marcos Date: Thu, 19 Oct 2023 08:55:52 -0300 Subject: [PATCH 05/11] chore: Finished Courseware Search Hooks unit testing --- .../courseware-search/hooks.test.jsx | 116 ++++++++++++------ 1 file changed, 80 insertions(+), 36 deletions(-) diff --git a/src/course-home/courseware-search/hooks.test.jsx b/src/course-home/courseware-search/hooks.test.jsx index 6f6d06c579..cb2844e1a3 100644 --- a/src/course-home/courseware-search/hooks.test.jsx +++ b/src/course-home/courseware-search/hooks.test.jsx @@ -2,7 +2,9 @@ import { renderHook, act } from '@testing-library/react-hooks'; import { useParams } from 'react-router-dom'; import { useSelector } from 'react-redux'; import { fetchCoursewareSearchSettings } from '../data/thunks'; -import { useCoursewareSearchFeatureFlag, useCoursewareSearchState, useElementBoundingBox } from './hooks'; +import { + useCoursewareSearchFeatureFlag, useCoursewareSearchState, useElementBoundingBox, useLockScroll, +} from './hooks'; jest.mock('react-redux'); jest.mock('react-router-dom'); @@ -25,21 +27,21 @@ describe('CoursewareSearch Hooks', () => { describe('useCoursewareSearchFeatureFlag', () => { const renderTestHook = async (enabled = true) => { useParams.mockImplementation(() => ({ courseId: enabled ? 123 : 456 })); - let state; - await act(async () => { (state = renderHook(() => useCoursewareSearchFeatureFlag())); }); - return state; + let hook; + await act(async () => { (hook = renderHook(() => useCoursewareSearchFeatureFlag())); }); + return hook; }; test('should return true if feature is enabled', async () => { - const state = await renderTestHook(); - await state.waitFor(() => expect(fetchCoursewareSearchSettings).toBeCalledTimes(1)); - expect(state.result.current).toBe(true); + const hook = await renderTestHook(); + await hook.waitFor(() => expect(fetchCoursewareSearchSettings).toBeCalledTimes(1)); + expect(hook.result.current).toBe(true); }); test('should return false if feature is disabled', async () => { - const state = await renderTestHook(false); - await state.waitFor(() => expect(fetchCoursewareSearchSettings).toBeCalledTimes(1)); - expect(state.result.current).toBe(false); + const hook = await renderTestHook(false); + await hook.waitFor(() => expect(fetchCoursewareSearchSettings).toBeCalledTimes(1)); + expect(hook.result.current).toBe(false); }); }); @@ -49,36 +51,33 @@ describe('CoursewareSearch Hooks', () => { const mockedStoreState = { courseHome: { showSearch } }; useSelector.mockImplementation(selector => selector(mockedStoreState)); - let state; - await act(async () => { (state = renderHook(() => useCoursewareSearchState())); }); - return state; + let hook; + await act(async () => { (hook = renderHook(() => useCoursewareSearchState())); }); + return hook; }; test('should return show: true if feature is enabled and showSearch is true', async () => { - const state = await renderTestHook({ enabled: true, showSearch: true }); + const hook = await renderTestHook({ enabled: true, showSearch: true }); - expect(state.result.current).toEqual({ show: true }); + expect(hook.result.current).toEqual({ show: true }); }); test('should return show: false in any other case', async () => { - let state; + let hook; - state = await renderTestHook({ enabled: true, showSearch: false }); - expect(state.result.current).toEqual({ show: false }); + hook = await renderTestHook({ enabled: true, showSearch: false }); + expect(hook.result.current).toEqual({ show: false }); - state = await renderTestHook({ enabled: false, showSearch: true }); - expect(state.result.current).toEqual({ show: false }); + hook = await renderTestHook({ enabled: false, showSearch: true }); + expect(hook.result.current).toEqual({ show: false }); - state = await renderTestHook({ enabled: false, showSearch: false }); - expect(state.result.current).toEqual({ show: false }); + hook = await renderTestHook({ enabled: false, showSearch: false }); + expect(hook.result.current).toEqual({ show: false }); }); }); describe('useElementBoundingBox', () => { let getBoundingClientRectSpy; - let addEventListenerSpy; - let removeEventListenerSpy; - const renderTestHook = async ({ elementId, mockedInfo }) => { getBoundingClientRectSpy = jest.spyOn(document, 'getElementById').mockImplementation(() => ( mockedInfo @@ -94,6 +93,8 @@ describe('CoursewareSearch Hooks', () => { return hook; }; + let addEventListenerSpy; + let removeEventListenerSpy; beforeEach(() => { addEventListenerSpy = jest.spyOn(global, 'addEventListener'); removeEventListenerSpy = jest.spyOn(global, 'removeEventListener'); @@ -110,25 +111,23 @@ describe('CoursewareSearch Hooks', () => { }); test('should unbindbind resize and scroll events when unmounted', async () => { - const state = await renderTestHook({ elementId: 'test', mockedInfo }); - state.unmount(); + const hook = await renderTestHook({ elementId: 'test', mockedInfo }); + hook.unmount(); expect(removeEventListenerSpy).toHaveBeenCalledWith('resize', expect.anything()); expect(removeEventListenerSpy).toHaveBeenCalledWith('scroll', expect.anything()); }); - // This test is failing, the hook state is not being updated. - xtest('should return the element bounding box', async () => { - const state = await renderTestHook({ elementId: 'test', mockedInfo }); + test('should return the element bounding box', async () => { + const hook = await renderTestHook({ elementId: 'test', mockedInfo }); - state.waitFor(() => expect(getBoundingClientRectSpy).toHaveBeenCalled()); + hook.waitFor(() => expect(getBoundingClientRectSpy).toHaveBeenCalled()); - expect(state.result.current).toEqual(mockedInfo); + expect(hook.result.current).toEqual(mockedInfo); }); - // This test is failing, the hook state is not being updated. - xtest('should call getBoundingClientRect on window resize', async () => { - const state = await renderTestHook({ elementId: 'test', mockedInfo }); + test('should call getBoundingClientRect on window resize', async () => { + const hook = await renderTestHook({ elementId: 'test', mockedInfo }); act(() => { // Trigger the window resize event. @@ -136,8 +135,53 @@ describe('CoursewareSearch Hooks', () => { global.dispatchEvent(new Event('resize')); }); - expect(state.result.current).toEqual(mockedInfo); + expect(hook.result.current).toEqual(mockedInfo); }); }); + + describe('when element is NOT present', () => { + let consoleWarnSpy; + beforeEach(() => { + consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {}); + }); + + it('should log a warning and return undefined', async () => { + await renderTestHook({ elementId: 'happiness' }); + + expect(consoleWarnSpy).toHaveBeenCalledWith("useElementBoundingBox(): Unable to find element with id='happiness' in the document."); + }); + }); + }); + + describe('useLockScroll', () => { + const renderTestHook = () => ( + renderHook(() => useLockScroll()) + ); + + let windowScrollSpy; + let addBodyClassSpy; + let removeBodyClassSpy; + let hook; + + beforeEach(() => { + windowScrollSpy = jest.spyOn(window, 'scrollTo'); + addBodyClassSpy = jest.spyOn(document.body.classList, 'add'); + removeBodyClassSpy = jest.spyOn(document.body.classList, 'remove'); + hook = renderTestHook(); + }); + + it('should perform a scrollTo(0, 0) on mount', () => { + expect(windowScrollSpy).toHaveBeenCalledWith(0, 0); + }); + + it('should append a _search-no-scroll on mount to the document body', () => { + expect(addBodyClassSpy).toHaveBeenCalledWith('_search-no-scroll'); + }); + + it('should remove the _search-no-scroll on unmount', () => { + hook.unmount(); + + expect(removeBodyClassSpy).toHaveBeenCalledWith('_search-no-scroll'); + }); }); }); From 36367b2d8939a3c31b930f33f11d31a259bf4ca7 Mon Sep 17 00:00:00 2001 From: Marcos Date: Thu, 19 Oct 2023 09:01:47 -0300 Subject: [PATCH 06/11] fix: Fixed an overlook that caused a conditional hook --- src/course-home/courseware-search/hooks.js | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/course-home/courseware-search/hooks.js b/src/course-home/courseware-search/hooks.js index c354c6800f..b008063e49 100644 --- a/src/course-home/courseware-search/hooks.js +++ b/src/course-home/courseware-search/hooks.js @@ -19,12 +19,9 @@ export function useCoursewareSearchFeatureFlag() { export function useCoursewareSearchState() { const enabled = useCoursewareSearchFeatureFlag(); - - if (!enabled) { return { show: false }; } - const show = useSelector(state => state.courseHome.showSearch); - return { show }; + return { show: enabled && show }; } export function useElementBoundingBox(elementId) { From f7fdb8ebad94c58a49a708781aa5e409e68ddb07 Mon Sep 17 00:00:00 2001 From: Marcos Date: Thu, 19 Oct 2023 09:13:54 -0300 Subject: [PATCH 07/11] fix: Reduced bounce timeout on scroll/resize to 100ms --- src/course-home/courseware-search/hooks.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/course-home/courseware-search/hooks.js b/src/course-home/courseware-search/hooks.js index b008063e49..0fe7cb82a1 100644 --- a/src/course-home/courseware-search/hooks.js +++ b/src/course-home/courseware-search/hooks.js @@ -4,7 +4,7 @@ import { useSelector } from 'react-redux'; import { debounce } from 'lodash'; import { fetchCoursewareSearchSettings } from '../data/thunks'; -const DEBOUNCE_WAIT = 300; // ms +const DEBOUNCE_WAIT = 100; // ms export function useCoursewareSearchFeatureFlag() { const { courseId } = useParams(); From d08d9e9b08c87a6c0a1983f174a0d7459e2c3b13 Mon Sep 17 00:00:00 2001 From: Marcos Date: Thu, 19 Oct 2023 09:39:14 -0300 Subject: [PATCH 08/11] chore: Updated snapshots --- src/course-home/courseware-search/hooks.js | 6 +++--- src/course-home/data/__snapshots__/redux.test.js.snap | 3 +++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/course-home/courseware-search/hooks.js b/src/course-home/courseware-search/hooks.js index 0fe7cb82a1..85c985021a 100644 --- a/src/course-home/courseware-search/hooks.js +++ b/src/course-home/courseware-search/hooks.js @@ -1,4 +1,4 @@ -import { useState, useEffect } from 'react'; +import { useState, useEffect, useLayoutEffect } from 'react'; import { useParams } from 'react-router-dom'; import { useSelector } from 'react-redux'; import { debounce } from 'lodash'; @@ -34,7 +34,7 @@ export function useElementBoundingBox(elementId) { return undefined; } - useEffect(() => { + useLayoutEffect(() => { // Handler to call on window resize and scroll function recalculate() { const bounds = element.getBoundingClientRect(); @@ -60,7 +60,7 @@ export function useElementBoundingBox(elementId) { } export function useLockScroll() { - useEffect(() => { + useLayoutEffect(() => { window.scrollTo(0, 0); document.body.classList.add('_search-no-scroll'); diff --git a/src/course-home/data/__snapshots__/redux.test.js.snap b/src/course-home/data/__snapshots__/redux.test.js.snap index be8367b7ed..36bd15993c 100644 --- a/src/course-home/data/__snapshots__/redux.test.js.snap +++ b/src/course-home/data/__snapshots__/redux.test.js.snap @@ -6,6 +6,7 @@ Object { "courseId": "course-v1:edX+DemoX+Demo_Course", "courseStatus": "loaded", "proctoringPanelStatus": "loading", + "showSearch": false, "targetUserId": undefined, "toastBodyLink": null, "toastBodyText": null, @@ -327,6 +328,7 @@ Object { "courseId": "course-v1:edX+DemoX+Demo_Course", "courseStatus": "loaded", "proctoringPanelStatus": "loading", + "showSearch": false, "targetUserId": undefined, "toastBodyLink": null, "toastBodyText": null, @@ -526,6 +528,7 @@ Object { "courseId": "course-v1:edX+DemoX+Demo_Course", "courseStatus": "loaded", "proctoringPanelStatus": "loading", + "showSearch": false, "targetUserId": undefined, "toastBodyLink": null, "toastBodyText": null, From 554a6fd74cb73726a2c500b495ea0e93b8458885 Mon Sep 17 00:00:00 2001 From: Marcos Date: Thu, 19 Oct 2023 09:48:27 -0300 Subject: [PATCH 09/11] chore: Moved @testing-library/react-hooks dep to DEV --- package-lock.json | 2 +- package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/package-lock.json b/package-lock.json index fffb2f4b03..e1e363ab05 100644 --- a/package-lock.json +++ b/package-lock.json @@ -31,7 +31,6 @@ "@fortawesome/react-fontawesome": "^0.1.4", "@popperjs/core": "2.11.8", "@reduxjs/toolkit": "1.8.1", - "@testing-library/react-hooks": "^8.0.1", "classnames": "2.3.2", "core-js": "3.22.2", "history": "5.3.0", @@ -59,6 +58,7 @@ "@pact-foundation/pact": "^11.0.2", "@testing-library/jest-dom": "5.16.5", "@testing-library/react": "12.1.5", + "@testing-library/react-hooks": "^8.0.1", "@testing-library/user-event": "13.5.0", "axios-mock-adapter": "1.20.0", "copy-webpack-plugin": "^11.0.0", diff --git a/package.json b/package.json index d4fc1fdee1..bba2e3822a 100644 --- a/package.json +++ b/package.json @@ -44,7 +44,6 @@ "@fortawesome/react-fontawesome": "^0.1.4", "@popperjs/core": "2.11.8", "@reduxjs/toolkit": "1.8.1", - "@testing-library/react-hooks": "^8.0.1", "classnames": "2.3.2", "core-js": "3.22.2", "history": "5.3.0", @@ -72,6 +71,7 @@ "@pact-foundation/pact": "^11.0.2", "@testing-library/jest-dom": "5.16.5", "@testing-library/react": "12.1.5", + "@testing-library/react-hooks": "^8.0.1", "@testing-library/user-event": "13.5.0", "axios-mock-adapter": "1.20.0", "copy-webpack-plugin": "^11.0.0", From 9dc1a995e710ebc44f9f88354df9854f7fa5f115 Mon Sep 17 00:00:00 2001 From: Marcos Date: Thu, 19 Oct 2023 11:17:07 -0300 Subject: [PATCH 10/11] chore: Minor adjustments on unit tests --- .../courseware-search/CoursewareSearch.test.jsx | 3 --- src/course-home/courseware-search/hooks.test.jsx | 16 ++++++++-------- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/src/course-home/courseware-search/CoursewareSearch.test.jsx b/src/course-home/courseware-search/CoursewareSearch.test.jsx index ec59a2d0fa..3e42be2f13 100644 --- a/src/course-home/courseware-search/CoursewareSearch.test.jsx +++ b/src/course-home/courseware-search/CoursewareSearch.test.jsx @@ -46,14 +46,11 @@ describe('CoursewareSearch', () => { }); it('Should have a "--modal-top-position" CSS variable matching the CourseTabsNavigation top position', () => { - renderComponent(); - const section = screen.getByTestId('courseware-search-section'); expect(section.style.getPropertyValue('--modal-top-position')).toBe(`${tabsTopPosition}px`); }); it('Should dispatch setShowSearch(true) when clicking the close button', () => { - renderComponent(); const button = screen.getByTestId('courseware-search-close-button'); fireEvent.click(button); diff --git a/src/course-home/courseware-search/hooks.test.jsx b/src/course-home/courseware-search/hooks.test.jsx index cb2844e1a3..cc6b67554a 100644 --- a/src/course-home/courseware-search/hooks.test.jsx +++ b/src/course-home/courseware-search/hooks.test.jsx @@ -32,13 +32,13 @@ describe('CoursewareSearch Hooks', () => { return hook; }; - test('should return true if feature is enabled', async () => { + it('should return true if feature is enabled', async () => { const hook = await renderTestHook(); await hook.waitFor(() => expect(fetchCoursewareSearchSettings).toBeCalledTimes(1)); expect(hook.result.current).toBe(true); }); - test('should return false if feature is disabled', async () => { + it('should return false if feature is disabled', async () => { const hook = await renderTestHook(false); await hook.waitFor(() => expect(fetchCoursewareSearchSettings).toBeCalledTimes(1)); expect(hook.result.current).toBe(false); @@ -56,13 +56,13 @@ describe('CoursewareSearch Hooks', () => { return hook; }; - test('should return show: true if feature is enabled and showSearch is true', async () => { + it('should return show: true if feature is enabled and showSearch is true', async () => { const hook = await renderTestHook({ enabled: true, showSearch: true }); expect(hook.result.current).toEqual({ show: true }); }); - test('should return show: false in any other case', async () => { + it('should return show: false in any other case', async () => { let hook; hook = await renderTestHook({ enabled: true, showSearch: false }); @@ -103,14 +103,14 @@ describe('CoursewareSearch Hooks', () => { describe('when element is present', () => { const mockedInfo = { top: 128 }; - test('should bind resize and scroll events on mount', async () => { + it('should bind resize and scroll events on mount', async () => { await renderTestHook({ elementId: 'test', mockedInfo }); expect(addEventListenerSpy).toHaveBeenCalledWith('resize', expect.anything()); expect(addEventListenerSpy).toHaveBeenCalledWith('scroll', expect.anything()); }); - test('should unbindbind resize and scroll events when unmounted', async () => { + it('should unbindbind resize and scroll events when unmounted', async () => { const hook = await renderTestHook({ elementId: 'test', mockedInfo }); hook.unmount(); @@ -118,7 +118,7 @@ describe('CoursewareSearch Hooks', () => { expect(removeEventListenerSpy).toHaveBeenCalledWith('scroll', expect.anything()); }); - test('should return the element bounding box', async () => { + it('should return the element bounding box', async () => { const hook = await renderTestHook({ elementId: 'test', mockedInfo }); hook.waitFor(() => expect(getBoundingClientRectSpy).toHaveBeenCalled()); @@ -126,7 +126,7 @@ describe('CoursewareSearch Hooks', () => { expect(hook.result.current).toEqual(mockedInfo); }); - test('should call getBoundingClientRect on window resize', async () => { + it('should call getBoundingClientRect on window resize', async () => { const hook = await renderTestHook({ elementId: 'test', mockedInfo }); act(() => { From 6a2bd79d3c10bbbe45022dd4174cf7c5453f63af Mon Sep 17 00:00:00 2001 From: Marcos Date: Fri, 20 Oct 2023 10:14:50 -0300 Subject: [PATCH 11/11] chore: Fixed test issue --- src/course-home/courseware-search/CoursewareSearch.test.jsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/course-home/courseware-search/CoursewareSearch.test.jsx b/src/course-home/courseware-search/CoursewareSearch.test.jsx index 3e42be2f13..276ff1a194 100644 --- a/src/course-home/courseware-search/CoursewareSearch.test.jsx +++ b/src/course-home/courseware-search/CoursewareSearch.test.jsx @@ -37,6 +37,9 @@ describe('CoursewareSearch', () => { describe('when rendering normally', () => { beforeAll(() => { useElementBoundingBox.mockImplementation(() => ({ top: tabsTopPosition })); + }); + + beforeEach(() => { renderComponent(); });