diff --git a/website/src/__mocks__/react-feather.tsx b/website/src/__mocks__/react-feather.tsx index dc9445cc11..fec1475806 100644 --- a/website/src/__mocks__/react-feather.tsx +++ b/website/src/__mocks__/react-feather.tsx @@ -3,7 +3,9 @@ import type { ComponentType } from 'react'; import * as feather from 'react-feather'; module.exports = mapValues(feather, (_component, name) => { - const MockComponent = jest.fn(() =>
); + const MockComponent = jest.fn((props) => ( +
+ )); (MockComponent as ComponentType).displayName = name; return MockComponent; }); diff --git a/website/src/test-utils/mockDom.ts b/website/src/test-utils/mockDom.ts index 8e773852d4..9c8b575424 100644 --- a/website/src/test-utils/mockDom.ts +++ b/website/src/test-utils/mockDom.ts @@ -1,3 +1,4 @@ +const nativeScroll = window.scroll; const nativeScrollTo = window.scrollTo; const nativePerformance = window.performance; const nativeMatchMedia = window.matchMedia; @@ -5,6 +6,7 @@ const nativeScrollIntoView = Element.prototype.scrollIntoView; export function mockDom() { // Mock some of the DOM environment functions that are missing from JSDom + window.scroll = jest.fn(); window.scrollTo = jest.fn(); if (!window.performance) { @@ -23,6 +25,7 @@ export function mockDom() { } export function mockDomReset() { + window.scroll = nativeScroll; window.scrollTo = nativeScrollTo; // @ts-expect-error We insist diff --git a/website/src/views/components/SearchBox.test.tsx b/website/src/views/components/SearchBox.test.tsx index 6268718b23..db0a82ab3f 100644 --- a/website/src/views/components/SearchBox.test.tsx +++ b/website/src/views/components/SearchBox.test.tsx @@ -1,48 +1,154 @@ -import { shallow } from 'enzyme'; -import SearchBox from './SearchBox'; +import { act, fireEvent, render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import SearchBox, { Props } from './SearchBox'; + +const SOME_TEXT = 'existing'; + +const defaultProps: Props = { + throttle: 0, + isLoading: false, + value: SOME_TEXT, + onChange: jest.fn(), + onSearch: jest.fn(), +}; describe(SearchBox, () => { - test('should match snapshot', () => { - expect( - shallow( - , - ), - ).toMatchSnapshot(); - - expect( - shallow( - , - ), - ).toMatchSnapshot(); - - expect( - shallow( - , - ), - ).toMatchSnapshot(); + test('should display loading indicator if isLoading', () => { + render(); + expect(screen.getByText(/loading/i)).toBeInTheDocument(); + }); + + test('should not display loading indicator if not isLoading', () => { + render(); + expect(screen.queryByText(/loading/i)).not.toBeInTheDocument(); + }); + + test('should display value', () => { + render(); + expect(screen.getByRole('searchbox')).toHaveValue(SOME_TEXT); + }); + + test('should display placeholder', () => { + render(); + expect(screen.getByRole('searchbox')).toHaveAttribute('placeholder', 'abc'); + expect(screen.getByRole('searchbox')).toHaveValue(''); + }); + + test('should call onChange and onSearch when user types into the input', async () => { + const mockOnChange = jest.fn(); + const mockOnSearch = jest.fn(); + render(); + + userEvent.type(screen.getByRole('searchbox'), 'ing'); + + expect(mockOnChange).toHaveBeenCalledTimes(3); + // `SearchBox` is a fully controlled component, so we expect the input to be + // appended to `value`. + expect(mockOnChange).toHaveBeenNthCalledWith(1, `${SOME_TEXT}i`); + expect(mockOnChange).toHaveBeenNthCalledWith(2, `${SOME_TEXT}n`); + expect(mockOnChange).toHaveBeenNthCalledWith(3, `${SOME_TEXT}g`); + + await waitFor(() => expect(mockOnSearch).toHaveBeenCalledTimes(1)); + }); + + test('should handle blur correctly', async () => { + const mockHandlers = [jest.fn(), jest.fn(), jest.fn()]; + const [mockOnBlur, mockOnChange, mockOnSearch] = mockHandlers; + render( + , + ); + + const box = screen.getByRole('searchbox'); + + // Test blur with no changes + userEvent.click(box); + fireEvent.blur(box); + expect(mockOnBlur).toHaveBeenCalledTimes(1); + expect(mockOnChange).not.toHaveBeenCalled(); + expect(mockOnSearch).not.toHaveBeenCalled(); + mockHandlers.forEach((m) => m.mockClear()); + + // Test blur with changes + userEvent.click(box); + userEvent.type(screen.getByRole('searchbox'), '.'); + fireEvent.blur(box); + expect(mockOnBlur).toHaveBeenCalledTimes(1); + expect(mockOnChange).toHaveBeenCalledTimes(2); // 1 call when character typed, 1 call on blur + expect(mockOnSearch).toHaveBeenCalledTimes(1); // No `waitFor` as we expect onSearch to be called immediately on blur + mockHandlers.forEach((m) => m.mockClear()); + + // Test blur with no changes *after* changes + userEvent.click(box); + fireEvent.blur(box); + expect(mockOnBlur).toHaveBeenCalledTimes(1); + expect(mockOnChange).not.toHaveBeenCalled(); + expect(mockOnSearch).not.toHaveBeenCalled(); + mockHandlers.forEach((m) => m.mockClear()); + }); + + test('should handle form submit correctly', () => { + const mockHandlers = [jest.fn(), jest.fn(), jest.fn()]; + const [mockOnBlur, mockOnChange, mockOnSearch] = mockHandlers; + const { container } = render( + , + ); + const box = screen.getByRole('searchbox'); + const form = container.getElementsByTagName('form')[0]; + + // Test submit with no changes + userEvent.click(box); + fireEvent.submit(form); + expect(box).not.toHaveFocus(); + expect(mockOnBlur).toHaveBeenCalledTimes(1); + expect(mockOnChange).not.toHaveBeenCalled(); + expect(mockOnSearch).not.toHaveBeenCalled(); + mockHandlers.forEach((m) => m.mockClear()); + + // Test submit with changes + userEvent.click(box); + userEvent.type(screen.getByRole('searchbox'), '.'); + fireEvent.submit(form); + expect(mockOnBlur).toHaveBeenCalledTimes(1); + expect(mockOnChange).toHaveBeenCalledTimes(2); // 1 call when character typed, 1 call on blur + expect(mockOnSearch).toHaveBeenCalledTimes(1); // No `waitFor` as we expect onSearch to be called immediately on blur + mockHandlers.forEach((m) => m.mockClear()); + }); + + test('should have a clear button', async () => { + const mockOnChange = jest.fn(); + const mockOnSearch = jest.fn(); + + render( + , + ); + const clearButton = screen.getByRole('button', { name: 'Clear search' }); + expect(clearButton).toBeInTheDocument(); + + userEvent.click(clearButton); + + expect(mockOnChange).toHaveBeenCalledTimes(1); + expect(mockOnChange).toHaveBeenCalledWith(''); + + await waitFor(() => expect(mockOnSearch).toHaveBeenCalledTimes(1)); + }); + + test('should not have a clear button if the box is empty', async () => { + render(); + expect(screen.queryByTestId('react-feather X icon')).not.toBeInTheDocument(); }); }); diff --git a/website/src/views/components/SearchBox.tsx b/website/src/views/components/SearchBox.tsx index 3e81b56951..76b1d14b11 100644 --- a/website/src/views/components/SearchBox.tsx +++ b/website/src/views/components/SearchBox.tsx @@ -1,15 +1,21 @@ -import * as React from 'react'; +import { + ChangeEventHandler, + FC, + FormEventHandler, + useCallback, + useMemo, + useRef, + useState, +} from 'react'; import classnames from 'classnames'; import { debounce } from 'lodash'; - -import LoadingSpinner from 'views/components/LoadingSpinner'; import { Search, X } from 'react-feather'; +import LoadingSpinner from 'views/components/LoadingSpinner'; import styles from './SearchBox.scss'; -type Props = { +export type Props = { className?: string; throttle: number; - useInstantSearch: boolean; isLoading: boolean; value: string | null; placeholder?: string; @@ -18,132 +24,143 @@ type Props = { onBlur?: () => void; }; -type State = { - isFocused: boolean; - hasChanges: boolean; -}; - -export default class SearchBox extends React.PureComponent { - searchElement = React.createRef(); - - constructor(props: Props) { - super(props); - - this.state = { - isFocused: false, - hasChanges: false, - }; - } - - onSubmit = () => { - const element = this.searchElement.current; +const SearchBox: FC = ({ + className, + throttle, + isLoading, + value, + placeholder, + onChange, + onSearch, + onBlur, +}) => { + const searchElement = useRef(null); + const [isFocused, setIsFocused] = useState(false); + + // This is a ref instead of state as it is only used within event handlers and + // we don't want to trigger unnecessary renders when it's changed. + const isDirty = useRef(false); + + const handleSubmit: FormEventHandler = useCallback((event) => { + event.preventDefault(); + const element = searchElement.current; if (!element) return; // element's onBlur callback will trigger the search flow. If we also // invoke onSearch in onSubmit, onSearch will be called twice. element.blur(); - }; - - onBlur = () => { - if (this.props.onBlur) this.props.onBlur(); - - const element = this.searchElement.current; + }, []); + + const debouncedSearch = useMemo( + () => + debounce( + () => { + isDirty.current = false; + onSearch(); + }, + throttle, + { leading: false }, + ), + // FIXME: If these props change, a new debouncedSearch will be created, + // which may cause weird timing bugs. + [onSearch, throttle], + ); + + const handleFocus = useCallback(() => setIsFocused(true), []); + + const handleBlur = useCallback(() => { + setIsFocused(false); + + onBlur?.(); + + const element = searchElement.current; if (!element) return; // Don't search if no changes - if (!this.state.hasChanges) return; + if (!isDirty.current) return; const searchTerm = element.value; - this.props.onChange(searchTerm); - this.debouncedSearch(); - this.debouncedSearch.flush(); - }; + onChange(searchTerm); + debouncedSearch(); + debouncedSearch.flush(); + }, [debouncedSearch, onBlur, onChange]); - onInput = (evt: React.ChangeEvent) => { - if (evt.target instanceof HTMLInputElement) { + const handleInput: ChangeEventHandler = useCallback( + (evt) => { const searchTerm = evt.target.value; - this.props.onChange(searchTerm); - this.setState({ hasChanges: true }); - if (this.props.useInstantSearch) this.debouncedSearch(); - } - }; - - onRemoveInput = () => { - this.props.onChange(''); - this.setState({ hasChanges: true }); - if (this.props.useInstantSearch) this.debouncedSearch(); - }; - - private search = () => { - this.setState({ hasChanges: false }); - this.props.onSearch(); - }; - - // eslint-disable-next-line react/sort-comp - private debouncedSearch = debounce(this.search, this.props.throttle, { - leading: false, - }); - - showSubmitHelp() { - return ( - !this.props.useInstantSearch && - this.state.hasChanges && - this.searchElement.current && - this.searchElement.current.value - ); - } - - render() { - const { value, placeholder, isLoading } = this.props; - return ( -
- -
{ - this.onSubmit(); - evt.preventDefault(); - }} - > - {isLoading ? ( -
- -
- ) : ( - - )} - {value && ( - - )} - this.setState({ isFocused: true })} - onBlur={() => { - this.setState({ isFocused: false }); - this.onBlur(); - }} - placeholder={placeholder} - spellCheck + onChange(searchTerm); + isDirty.current = true; + debouncedSearch(); + }, + [debouncedSearch, onChange], + ); + + const clearInput = useCallback(() => { + onChange(''); + isDirty.current = true; + debouncedSearch(); + // Don't flush debouncedSearch here. + // + // Reason: Consider a component (e.g. SearchkitSearchBox) that: + // 1. Stores the value it received from `onChange` in its component state + // using `setState`, and + // 2. Uses `onSearch` to trigger a search using that state. + // + // If we flush here, + // 1. `onChange` will call `setState` with the new search query. + // 2. `onSearch` will be called, triggering a search with the component's + // search query state variable. Since React hasn't rendered, this state is + // the *previous* search query, and the component will have triggered a + // search with the previous query. + // 3. React renders and updates the component's state with the new query. + // Of course, since the component doesn't know this happened (it requires a + // call to `onSearch`), it won't trigger a search with the new search query. + // + // We'll thus want to allow the standard deferring behavior to occur, or + // perhaps shorten the debouncing to after the next render. + }, [debouncedSearch, onChange]); + + return ( +
+ + + {isLoading ? ( +
+ +
+ ) : ( + + )} + {value && ( + - + )} + + +
+ ); +}; - {this.showSubmitHelp() &&

Press enter to search

} -
- ); - } -} +export default SearchBox; diff --git a/website/src/views/components/SearchkitSearchBox.tsx b/website/src/views/components/SearchkitSearchBox.tsx index 0df5e4a548..9d500774d2 100644 --- a/website/src/views/components/SearchkitSearchBox.tsx +++ b/website/src/views/components/SearchkitSearchBox.tsx @@ -101,7 +101,6 @@ export default class SearchkitSearchBox extends SearchkitComponent - -
- - - -
-`; - -exports[`SearchBox should match snapshot 2`] = ` -
- -
-
- -
- -
-
-`; - -exports[`SearchBox should match snapshot 3`] = ` -
- -
- - - - -
-`; diff --git a/website/src/views/venues/AvailabilitySearch.tsx b/website/src/views/venues/AvailabilitySearch.tsx index ec2d9cbdc4..0329de2d3d 100644 --- a/website/src/views/venues/AvailabilitySearch.tsx +++ b/website/src/views/venues/AvailabilitySearch.tsx @@ -1,4 +1,4 @@ -import * as React from 'react'; +import { ChangeEventHandler, FC, memo, SyntheticEvent, useMemo } from 'react'; import classnames from 'classnames'; import { range } from 'lodash'; @@ -19,32 +19,39 @@ const CLASS_START_HOURS = range(FIRST_CLASS_HOUR, LAST_CLASS_HOUR + 1); export function defaultSearchOptions( now: Date = new Date(), // Used for tests only ): VenueSearchOptions { - // Set day of week - if it is not a school day, then set to Monday (0) - const day = getDayIndex(now) === 6 ? 0 : getDayIndex(now); - - // Set time - if the current time is outside class hours, set it to the - // time of the earliest lesson - const time = Math.max(getCurrentHours(now), FIRST_CLASS_HOUR); - return { - time, - day, + // Set day of week - if it is not a school day, then set to Monday (0) + day: getDayIndex(now) === 6 ? 0 : getDayIndex(now), + // Set time - if the current time is outside class hours, set it to the + // time of the earliest lesson + time: Math.max(getCurrentHours(now), FIRST_CLASS_HOUR), duration: 1, }; } -const AvailabilitySearch = React.memo(({ className, searchOptions, onUpdate }) => { - const onUpdateInner = ( - event: React.SyntheticEvent, - key: keyof VenueSearchOptions, - ) => { - if (typeof event.currentTarget.value !== 'undefined') { - onUpdate({ - ...searchOptions, - [key]: +event.currentTarget.value, - }); +const AvailabilitySearch: FC = ({ className, searchOptions, onUpdate }) => { + const [ + handleDayChange, + handleTimeChange, + handleDurationChange, + ]: ChangeEventHandler[] = useMemo(() => { + function onUpdateInner( + event: SyntheticEvent, + key: keyof VenueSearchOptions, + ) { + if (typeof event.currentTarget.value !== 'undefined') { + onUpdate({ + ...searchOptions, + [key]: +event.currentTarget.value, + }); + } } - }; + return [ + (event) => onUpdateInner(event, 'day'), + (event) => onUpdateInner(event, 'time'), + (event) => onUpdateInner(event, 'duration'), + ]; + }, [onUpdate, searchOptions]); return (
@@ -54,7 +61,7 @@ const AvailabilitySearch = React.memo(({ className, searchOptions, onUpda id="venue-day" className="form-control" value={searchOptions.day} - onChange={(evt) => onUpdateInner(evt, 'day')} + onChange={handleDayChange} > {SCHOOLDAYS.map((name, day) => (
); -}); +}; -export default AvailabilitySearch; +export default memo(AvailabilitySearch); diff --git a/website/src/views/venues/VenueDetails.tsx b/website/src/views/venues/VenueDetails.tsx index 3d7a93d01c..f1ac61daf8 100644 --- a/website/src/views/venues/VenueDetails.tsx +++ b/website/src/views/venues/VenueDetails.tsx @@ -1,4 +1,4 @@ -import { FC, memo, useCallback, useMemo } from 'react'; +import { FC, useCallback, useMemo } from 'react'; import { Link, useHistory } from 'react-router-dom'; import { ChevronLeft, ChevronRight } from 'react-feather'; import classnames from 'classnames'; @@ -26,13 +26,7 @@ type Props = { highlightPeriod?: TimePeriod; }; -const VenueDetailsComponent: FC = ({ - venue, - previous, - next, - availability, - highlightPeriod, -}) => { +const VenueDetails: FC = ({ venue, previous, next, availability, highlightPeriod }) => { const arrangedLessons = useMemo(() => { const lessons: Lesson[] = flatMap(availability, (day) => day.classes).map((venueLesson) => ({ ...venueLesson, @@ -98,4 +92,4 @@ const VenueDetailsComponent: FC = ({ ); }; -export default memo(VenueDetailsComponent); +export default VenueDetails; diff --git a/website/src/views/venues/VenueDetailsPane.tsx b/website/src/views/venues/VenueDetailsPane.tsx new file mode 100644 index 0000000000..b9923fe7ab --- /dev/null +++ b/website/src/views/venues/VenueDetailsPane.tsx @@ -0,0 +1,114 @@ +import { FC, memo, useCallback, useMemo, useState } from 'react'; +import { Map } from 'react-feather'; +import { useHistory } from 'react-router-dom'; +import classnames from 'classnames'; + +import type { TimePeriod, VenueDetailList } from 'types/venues'; + +import { venuePage } from 'views/routes/paths'; +import Modal from 'views/components/Modal'; +import NoFooter from 'views/layout/NoFooter'; +import MapContext from 'views/components/map/MapContext'; +import useMediaQuery from 'views/hooks/useMediaQuery'; +import { breakpointDown } from 'utils/css'; +import VenueDetails from './VenueDetails'; + +import styles from './VenuesContainer.scss'; + +type Props = { + highlightPeriod?: TimePeriod; + matchedVenues: VenueDetailList; + selectedVenue?: string; + venues: VenueDetailList; +}; + +const VenueDetailsPaneComponent: FC = ({ + highlightPeriod, + matchedVenues, + selectedVenue, + venues, +}) => { + const [isMapExpanded, setIsMapExpanded] = useState(false); + + const history = useHistory(); + const onClearVenueSelect = useCallback( + () => + history.push({ + ...history.location, + pathname: venuePage(), + }), + [history], + ); + + const matchBreakpoint = useMediaQuery(breakpointDown('sm')); + + const venueDetailProps = useMemo(() => { + if (!venues || !selectedVenue) return null; + + // Find the index of the current venue on the list of matched venues so + // we can obtain the previous and next item + const lowercaseSelectedVenue = selectedVenue.toLowerCase(); + const venueIndex = matchedVenues.findIndex( + ([venue]) => venue.toLowerCase() === lowercaseSelectedVenue, + ); + + // The selected item may not be in the list of matched venues (if the user + // changed their search options afterwards), in which case we look for it in all + // venues + if (venueIndex === -1) { + const venueDetail = venues.find(([venue]) => venue.toLowerCase() === lowercaseSelectedVenue); + if (!venueDetail) return null; + const [venue, availability] = venueDetail; + return { venue, availability, next: undefined, previous: undefined }; + } + + const [venue, availability] = matchedVenues[venueIndex]; + const [previous] = matchedVenues[venueIndex - 1] ?? []; + const [next] = matchedVenues[venueIndex + 1] ?? []; + return { venue, availability, next, previous }; + }, [matchedVenues, selectedVenue, venues]); + + return ( + + {matchBreakpoint ? ( + + + {venueDetailProps && ( + + )} + + ) : ( + <> +
+ {venueDetailProps ? ( + + ) : ( +
+ +

Select a venue on the left to see its timetable

+
+ )} +
+ + + )} +
+ ); +}; + +export default memo(VenueDetailsPaneComponent); diff --git a/website/src/views/venues/VenueList.tsx b/website/src/views/venues/VenueList.tsx index 3f027c2118..b3aa3f538d 100644 --- a/website/src/views/venues/VenueList.tsx +++ b/website/src/views/venues/VenueList.tsx @@ -1,7 +1,7 @@ -import * as React from 'react'; +import { FC, memo, useMemo } from 'react'; import classnames from 'classnames'; import { groupBy, toPairs, sortBy } from 'lodash'; -import { Link, LinkProps } from 'react-router-dom'; +import { Link, LinkProps, useHistory } from 'react-router-dom'; import { Venue } from 'types/venues'; import { venuePage } from 'views/routes/paths'; @@ -10,35 +10,38 @@ import styles from './VenueList.scss'; type Props = { venues: Venue[]; - selectedVenue?: Venue | null; + selectedVenue?: Venue; linkProps?: Omit; }; -const VenueList: React.FC = (props) => { - // Added during the horrible COVID-19 times to hide E-Learning venues - const physicalVenues = props.venues.filter((venue) => !venue.startsWith('E-Learn')); - const venueList = groupBy(physicalVenues, (venue) => venue.charAt(0).toUpperCase()); - const sortedVenueList = sortBy(toPairs(venueList), ([key]) => key); +const VenueList: FC = ({ venues, selectedVenue, linkProps }) => { + const sortedVenueList = useMemo(() => { + // Added during the horrible COVID-19 times to hide E-Learning venues + const physicalVenues = venues.filter((venue) => !venue.startsWith('E-Learn')); + const venueList = groupBy(physicalVenues, (venue) => venue.charAt(0).toUpperCase()); + return sortBy(toPairs(venueList), ([key]) => key); + }, [venues]); + + const history = useHistory(); return (
    - {sortedVenueList.map(([heading, venues]) => ( + {sortedVenueList.map(([heading, sortedVenue]) => (
  • {heading}

    -
      - {venues.map((venue) => ( + {sortedVenue.map((venue) => (
    • {venue} @@ -51,4 +54,4 @@ const VenueList: React.FC = (props) => { ); }; -export default VenueList; +export default memo(VenueList); diff --git a/website/src/views/venues/VenueLocation/AddLocationModal.tsx b/website/src/views/venues/VenueLocation/AddLocationModal.tsx new file mode 100644 index 0000000000..0a8a5caafb --- /dev/null +++ b/website/src/views/venues/VenueLocation/AddLocationModal.tsx @@ -0,0 +1,23 @@ +import type { FC } from 'react'; + +import Modal from 'views/components/Modal'; +import CloseButton from 'views/components/CloseButton'; + +import ImproveVenueForm from './ImproveVenueForm'; +import styles from './VenueLocation.scss'; + +type Props = { + venue: string; + isOpen: boolean; + onRequestClose: () => void; +}; + +const AddLocationModal: FC = ({ venue, isOpen, onRequestClose }) => ( + + +

      Improve {venue}

      + +
      +); + +export default AddLocationModal; diff --git a/website/src/views/venues/VenueLocation/FeedbackModal.tsx b/website/src/views/venues/VenueLocation/FeedbackModal.tsx index ed78f116fa..9976b852e9 100644 --- a/website/src/views/venues/VenueLocation/FeedbackModal.tsx +++ b/website/src/views/venues/VenueLocation/FeedbackModal.tsx @@ -1,7 +1,8 @@ -import { PureComponent } from 'react'; +import { FC, useCallback, useState } from 'react'; import classnames from 'classnames'; -import { VenueLocation } from 'types/venues'; +import type { VenueLocation } from 'types/venues'; + import Modal from 'views/components/Modal'; import CloseButton from 'views/components/CloseButton'; import ExternalLink from 'views/components/ExternalLink'; @@ -19,22 +20,19 @@ type Props = { readonly existingLocation: VenueLocation | null; }; -type State = { - readonly page: Page; -}; +const FeedbackModal: FC = ({ venue, isOpen, onRequestClose, existingLocation }) => { + const [page, setPage] = useState('menu'); -export default class FeedbackModal extends PureComponent { - state: State = { - page: 'menu', - }; + const handleRequestClose = useCallback(() => { + setPage('menu'); + onRequestClose(); + }, [onRequestClose]); - onRequestClose = () => { - this.setState({ page: 'menu' }); - this.props.onRequestClose(); - }; + const goToForm = useCallback(() => setPage('form'), []); + const goToMenu = useCallback(() => setPage('menu'), []); - renderPage() { - switch (this.state.page) { + function renderPage() { + switch (page) { case 'menu': return (
      @@ -45,19 +43,15 @@ export default class FeedbackModal extends PureComponent { >

      Problem with map data

      -

      eg. incorrect building outline, missing walkways

      +

      e.g. incorrect building outline, missing walkways

      -
      @@ -65,27 +59,21 @@ export default class FeedbackModal extends PureComponent { case 'form': return ( - this.setState({ page: 'menu' })} - /> + ); default: - throw new Error(`Unknown page ${this.state.page}`); + throw new Error(`Unknown page ${page}`); } } - render() { - const { isOpen } = this.props; + return ( + + +

      Improve {venue}

      + {renderPage()} +
      + ); +}; - return ( - - -

      Improve {this.props.venue}

      - {this.renderPage()} -
      - ); - } -} +export default FeedbackModal; diff --git a/website/src/views/venues/VenueLocation/VenueLocation.tsx b/website/src/views/venues/VenueLocation/VenueLocation.tsx index 31233ac2a4..043dcfc6cb 100644 --- a/website/src/views/venues/VenueLocation/VenueLocation.tsx +++ b/website/src/views/venues/VenueLocation/VenueLocation.tsx @@ -1,18 +1,13 @@ -import { PureComponent } from 'react'; +import { FC, useCallback, useMemo, useState } from 'react'; import classnames from 'classnames'; -import type { - LatLngTuple, - VenueLocation as VenueLocationItem, - VenueLocationMap, -} from 'types/venues'; -import Modal from 'views/components/Modal'; +import type { LatLngTuple, VenueLocationMap } from 'types/venues'; + import LocationMap from 'views/components/map/LocationMap'; -import CloseButton from 'views/components/CloseButton'; import { floorName } from 'utils/venues'; +import AddLocationModal from './AddLocationModal'; import FeedbackModal from './FeedbackModal'; -import ImproveVenueForm from './ImproveVenueForm'; import styles from './VenueLocation.scss'; export type Props = { @@ -20,107 +15,80 @@ export type Props = { readonly venue: string; }; -type State = { - readonly isFeedbackModalOpen: boolean; -}; - -export default class VenueLocation extends PureComponent { - state: State = { - isFeedbackModalOpen: false, - }; - - openModal = () => this.setState({ isFeedbackModalOpen: true }); +const VenueLocation: FC = ({ venueLocations, venue }) => { + const [isFeedbackModalOpen, setIsFeedbackModalOpen] = useState(false); + const openModal = useCallback(() => setIsFeedbackModalOpen(true), []); + const closeModal = useCallback(() => setIsFeedbackModalOpen(false), []); - closeModal = () => this.setState({ isFeedbackModalOpen: false }); - - renderFeedbackMenu(existingLocation: VenueLocationItem | null = null) { - const { venue } = this.props; - const { isFeedbackModalOpen } = this.state; - - if (!existingLocation || !existingLocation.location) { - return ( - - -

      Improve {venue}

      - -
      - ); - } + const location = venueLocations[venue]; + const position: LatLngTuple | null = useMemo( + () => (location?.location ? [location.location.y, location.location.x] : null), + [location], + ); + if (!location) { return ( - + <> +
      +

      We don't have data for this venue.

      + +
      + + ); } - render() { - const { venue, venueLocations } = this.props; - const location = venueLocations[venue]; - - if (!location) { - return ( - <> -
      -

      We don't have data for this venue.

      - -
      - - {this.renderFeedbackMenu()} - - ); - } - - const position: LatLngTuple | null = location.location - ? [location.location.y, location.location.x] - : null; - - return ( -
      -

      - {location.roomName} ({venue}) - {location.floor != null && ( - <> - {' '} - is on {floorName(location.floor)} - - )} - . -

      - - {position ? ( + return ( +
      +

      + {location.roomName} ({venue}) + {location.floor != null && ( <> - - -

      - See a problem?{' '} - -

      - - ) : ( - <> -

      We don't have the location of this venue, sorry :(

      - + {' '} + is on {floorName(location.floor)} )} + . +

      - {this.renderFeedbackMenu(location)} + {position ? ( + <> + +

      + See a problem?{' '} + +

      + + + ) : ( + <> +

      We don't have the location of this venue, sorry :(

      + + + + )} +
      +
      + ); +}; -
      -
      - ); - } -} +export default VenueLocation; diff --git a/website/src/views/venues/VenueLocation/index.tsx b/website/src/views/venues/VenueLocation/index.tsx index 2b1aa1cc30..a3cc94d0d4 100644 --- a/website/src/views/venues/VenueLocation/index.tsx +++ b/website/src/views/venues/VenueLocation/index.tsx @@ -1,6 +1,6 @@ import retryImport from 'utils/retryImport'; import withVenueLocations from 'views/components/map/withVenueLocations'; -import { Props } from './VenueLocation'; +import type { Props } from './VenueLocation'; export default withVenueLocations(() => retryImport(() => import(/* webpackChunkName: "venue" */ './VenueLocation')).then( diff --git a/website/src/views/venues/VenuesContainer.test.tsx b/website/src/views/venues/VenuesContainer.test.tsx index f5bdf1f304..aac5b652cb 100644 --- a/website/src/views/venues/VenuesContainer.test.tsx +++ b/website/src/views/venues/VenuesContainer.test.tsx @@ -1,147 +1,224 @@ -import { shallow } from 'enzyme'; -import qs from 'query-string'; -// eslint-disable-next-line import/no-extraneous-dependencies -import { History } from 'history'; +import { screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import axios, { AxiosError } from 'axios'; +import ReactModal from 'react-modal'; + +import type { Venue, VenueInfo } from 'types/venues'; -import { Venue, VenueDetailList, VenueInfo } from 'types/venues'; -import venueInfo from '__mocks__/venueInformation.json'; -import createHistory from 'test-utils/createHistory'; import { sortVenues } from 'utils/venues'; import { venuePage } from 'views/routes/paths'; -import VenueDetails from 'views/venues/VenueDetails'; -import { Params, VenuesContainerComponent } from './VenuesContainer'; +import { mockDom, mockDomReset, mockWindowMatchMedia } from 'test-utils/mockDom'; +import renderWithRouterMatch from 'test-utils/renderWithRouterMatch'; -const venues = sortVenues(venueInfo as VenueInfo); +import venueInfo from '__mocks__/venueInformation.json'; + +import { VenuesContainerComponent } from './VenuesContainer'; -function createComponent(selectedVenue: Venue | null = null, search = '') { - const location = venuePage(selectedVenue) + search; - const match = { params: { venue: selectedVenue } }; - const router = createHistory(location, match); +const venues = sortVenues(venueInfo as VenueInfo); - return { - history: router.history, - wrapper: shallow(), - }; +const someError: Partial = { + response: { + data: undefined, + status: 500, + statusText: 'Test error', + headers: {}, + config: {}, + }, +}; + +function make(selectedVenue: Venue | null = null, search = '') { + const rendered = renderWithRouterMatch(, { + path: '/venues/:venue?', + location: venuePage(selectedVenue) + search, + }); + ReactModal.setAppElement(rendered.view.container as HTMLElement); + return rendered; } describe(VenuesContainerComponent, () => { - describe('URL handling', () => { - test('should not select or filter if no venue is present in URL', () => { - const { wrapper } = createComponent(); - - // Check that search and availability search is not enabled - expect(wrapper.state('searchTerm')).toEqual(''); - expect(wrapper.state('isAvailabilityEnabled')).toBe(false); - - wrapper.setProps({ - history: { action: 'POP' }, - selectedVenue: undefined, // No react-router match - }); - - // Check that search and availability search is not enabled after navigation - expect(wrapper.state('searchTerm')).toEqual(''); - expect(wrapper.state('isAvailabilityEnabled')).toBe(false); - }); + let mockAxiosRequest: jest.SpiedFunction; - test('initialize search based on params', () => { - const { wrapper } = createComponent(null, '?q=hello+world'); - expect(wrapper.state('searchTerm')).toEqual('hello world'); - expect(wrapper.state('isAvailabilityEnabled')).toBe(false); - }); + beforeEach(() => { + mockDom(); - test('initialize filters based on params', () => { - const { wrapper } = createComponent(null, '?day=1&time=9&duration=1'); - expect(wrapper.state()).toMatchObject({ - searchOptions: { - day: 1, - time: 9, - duration: 1, - }, - isAvailabilityEnabled: true, - }); - }); + // Default to wide viewport + mockWindowMatchMedia({ matches: false }); + + // TODO: Add tests with augmented venue info; we'll just pretend they don't exist for now + mockAxiosRequest = jest.spyOn(axios, 'request').mockRejectedValue(someError); }); - describe('#updateURL()', () => { - const getQueryParams = (history: History) => qs.parse(history.location.search); + afterEach(() => { + mockAxiosRequest.mockRestore(); + mockDomReset(); + }); - test('it should update search query', () => { - const { wrapper, history } = createComponent(); + test('should populate UI with URL info (no info)', () => { + make(); - // Should set query string - wrapper.setState({ searchTerm: 'covfefe' }); - expect(getQueryParams(history)).toEqual({ q: 'covfefe' }); + expect(screen.getByRole('searchbox')).toHaveValue(''); - // Should decode special chars - wrapper.setState({ searchTerm: 'Cdat/overThar1!' }); - expect(history.location.search).toEqual('?q=Cdat%2FoverThar1%21'); + // Expect all venues to be shown in list + expect(screen.getAllByRole('link').map((link) => link.textContent)).toMatchInlineSnapshot(` + Array [ + "CQT/SR0622", + "LT1", + "lt2", + "LT17", + "S11-0302", + ] + `); - // Should clear query string - wrapper.setState({ searchTerm: '' }); - expect(getQueryParams(history)).toEqual({}); - }); + // Expect availability search to be inactive + expect(screen.queryAllByRole('combobox')).toHaveLength(0); + }); - test('it should update search options', () => { - const { wrapper, history } = createComponent(); - - wrapper.setState({ - searchOptions: { - day: 1, - time: 9, - duration: 1, - }, - isAvailabilityEnabled: true, - }); - - expect(getQueryParams(history)).toEqual({ - day: '1', - time: '9', - duration: '1', - }); - - // Switching availability search off should clear params - wrapper.setState({ - isAvailabilityEnabled: false, - }); - expect(getQueryParams(history)).toEqual({}); - }); + test('should populate UI with URL info (selected venue)', () => { + make('CQT/SR0622'); + + expect(screen.getByRole('searchbox')).toHaveValue(''); + + // Expect selected venue info to be shown + expect(screen.getAllByRole('button').map((button) => button.textContent)) + .toMatchInlineSnapshot(` + Array [ + " Find free rooms", + "PC4241TUT [ST1]", + "PC2132TUT [ST2]", + "PC4241TUT [ST3]", + "PC2132TUT [ST4]", + "PC2132TUT [ST5]", + ] + `); + + // Expect availability search to be inactive + expect(screen.queryAllByRole('combobox')).toHaveLength(0); }); - describe('#renderSelectedVenue', () => { - const getVenueDetail = (selectedVenue: Venue | null, matched: VenueDetailList = venues) => { - const instance = createComponent( - selectedVenue, - ).wrapper.instance() as VenuesContainerComponent; + test('should populate UI with URL info (query)', () => { + make(null, '?q=hello+world'); - return shallow(
      {instance.renderSelectedVenue(matched)}
      ).find(VenueDetails); - }; + expect(screen.getByRole('searchbox')).toHaveValue('hello world'); - test('not render when there is no selected venue', () => { - expect(getVenueDetail(null).exists()).toBe(false); - }); + // Expect availability search to be inactive + expect(screen.queryAllByRole('combobox')).toHaveLength(0); + }); + + test('should populate UI with URL info (availability)', () => { + make(null, '?day=1&time=9&duration=1'); + + expect(screen.getByRole('searchbox')).toHaveValue(''); + + const availabilitySelects = screen.getAllByRole('combobox') as HTMLSelectElement[]; + expect(availabilitySelects.map((select) => select.value)).toEqual(['1', '9', '1']); + }); + + test('should update URL with typed query', () => { + const { history } = make(); + const searchbox = screen.getByRole('searchbox'); + userEvent.type(searchbox, 'covfefe'); + expect(history.location.search).toBe('?q=covfefe'); + }); - test('render when a venue is selected', () => { - expect(getVenueDetail('LT17').props()).toMatchObject({ - venue: 'LT17', - availability: venueInfo.LT17, - previous: 'lt2', - next: 'S11-0302', - }); - - const LTs = venues.filter(([venue]) => venue.includes('LT')); - expect(getVenueDetail('LT17', LTs).props()).toMatchObject({ - venue: 'LT17', - availability: venueInfo.LT17, - previous: 'LT1', - next: undefined, - }); + test('should update URL with availability input', () => { + const { history } = make(); + + userEvent.click(screen.getByRole('button', { name: 'Find free rooms' })); + expect(history.location.search).toMatch(/\?day=\d&duration=1&time=\d{1,2}/); // Actual value depends on current date + + userEvent.selectOptions(screen.getByRole('combobox', { name: 'On' }), '2'); + userEvent.selectOptions(screen.getByRole('combobox', { name: 'From' }), '15'); + userEvent.selectOptions(screen.getByRole('combobox', { name: 'To' }), '5'); + expect(history.location.search).toBe('?day=2&duration=5&time=15'); + }); + + test('should filter results based on typed query', () => { + make(); + const searchbox = screen.getByRole('searchbox'); + userEvent.type(searchbox, 'LT'); + expect(screen.getAllByRole('link').map((link) => link.textContent)).toMatchInlineSnapshot(` + Array [ + "LT1", + "lt2", + "LT17", + ] + `); + }); + + test('should filter results based on availability input', () => { + make(); + + // Manually select an availability so that the search results will be deterministic + userEvent.click(screen.getByRole('button', { name: 'Find free rooms' })); + userEvent.selectOptions(screen.getByRole('combobox', { name: 'On' }), '1'); + userEvent.selectOptions(screen.getByRole('combobox', { name: 'From' }), '8'); + userEvent.selectOptions(screen.getByRole('combobox', { name: 'To' }), '16'); + + expect(screen.getAllByRole('link').map((link) => link.textContent)).toMatchInlineSnapshot(` + Array [ + "CQT/SR0622", + "lt2", + ] + `); + }); + + test('should update URL when selecting a venue', () => { + const search = '?q=LT'; + const { history } = make(null, search); + + // Sanity checks + expect(history.location.pathname).toBe('/venues'); // Ensure nothing is selected + expect(history.location.search).toBe(search); // Ensure query is set + + // Test selecting a venue when no venue was selected + userEvent.click(screen.getByRole('link', { name: 'lt2' })); + expect(history.location.pathname).toBe('/venues/lt2'); + expect(history.location.search).toBe(search); // Expect query to be unchanged + + // Test selecting another venue + userEvent.click(screen.getAllByRole('link', { name: 'LT17' })[1]); // 2 links present: one in list, one in detail pane + expect(history.location.pathname).toBe('/venues/LT17'); + expect(history.location.search).toBe(search); // Expect query to be unchanged + }); + + test('should retain existing query and filters after selecting a venue', () => { + make(null, '?q=lt&day=1&time=9&duration=1'); + + userEvent.click(screen.getByRole('link', { name: 'lt2' })); + + // Expect below to be true before and after clicking the link + expect(screen.getByRole('searchbox')).toHaveValue('lt'); + const availabilitySelects = screen.getAllByRole('combobox') as HTMLSelectElement[]; + expect(availabilitySelects.map((select) => select.value)).toEqual(['1', '9', '1']); + }); + + test('should prompt for venue selection if no venue selected', () => { + make(); + expect(screen.getByText(/Select a venue on the left to see its timetable/)).toBeInTheDocument(); + }); + + test('should prompt for venue selection if non-existent venue selected', () => { + make('heaven'); + expect(screen.getByText(/Select a venue on the left to see its timetable/)).toBeInTheDocument(); + }); + + describe('narrow viewport detail modal', () => { + beforeEach(() => { + // Set up narrow viewport + mockWindowMatchMedia({ matches: true }); + }); + test('modal should have a back button after navigating to a venue', () => { + make(); + expect(screen.queryByRole('button', { name: 'Back to Venues' })).not.toBeInTheDocument(); + userEvent.click(screen.getByRole('link', { name: 'lt2' })); + expect(screen.getByRole('button', { name: 'Back to Venues' })).toBeInTheDocument(); }); - test('render when a venue is selected, and it is not in the list of matched venues', () => { - expect(getVenueDetail('LT17', []).props()).toMatchObject({ - venue: 'LT17', - availability: venueInfo.LT17, - }); + test('modal back button should navigate back when clicked', () => { + const { history } = make('lt2'); + expect(history.location.pathname).toBe('/venues/lt2'); // Sanity check + userEvent.click(screen.getByRole('button', { name: 'Back to Venues' })); + expect(history.location.pathname).toBe('/venues'); }); }); }); diff --git a/website/src/views/venues/VenuesContainer.tsx b/website/src/views/venues/VenuesContainer.tsx index 142b100d6d..27cb240b52 100644 --- a/website/src/views/venues/VenuesContainer.tsx +++ b/website/src/views/venues/VenuesContainer.tsx @@ -1,173 +1,101 @@ -import { Component } from 'react'; -import { RouteComponentProps, withRouter } from 'react-router-dom'; +import { FC, useCallback, useEffect, useMemo, useState } from 'react'; +import { useHistory, useLocation, useParams } from 'react-router-dom'; import Loadable, { LoadingComponentProps } from 'react-loadable'; +// eslint-disable-next-line import/no-extraneous-dependencies +import { Location, locationsAreEqual } from 'history'; import classnames from 'classnames'; -import axios, { AxiosResponse } from 'axios'; -import produce from 'immer'; +import axios from 'axios'; import qs from 'query-string'; -import { isEqual, mapValues, pick, size } from 'lodash'; +import { isEqual, mapValues, noop, pick, size } from 'lodash'; import type { TimePeriod, Venue, VenueDetailList, VenueSearchOptions } from 'types/venues'; import type { Subtract } from 'types/utils'; -import type { WithBreakpoint } from 'views/hocs/makeResponsive'; import deferComponentRender from 'views/hocs/deferComponentRender'; import ApiError from 'views/errors/ApiError'; import Warning from 'views/errors/Warning'; import LoadingSpinner from 'views/components/LoadingSpinner'; import SearchBox from 'views/components/SearchBox'; -import { Clock, Map } from 'react-feather'; +import { Clock } from 'react-feather'; import { venuePage } from 'views/routes/paths'; -import Modal from 'views/components/Modal'; import Title from 'views/components/Title'; -import NoFooter from 'views/layout/NoFooter'; -import MapContext from 'views/components/map/MapContext'; -import makeResponsive from 'views/hocs/makeResponsive'; import config from 'config'; import nusmods from 'apis/nusmods'; import HistoryDebouncer from 'utils/HistoryDebouncer'; import { clampClassDuration, filterAvailability, searchVenue, sortVenues } from 'utils/venues'; -import { breakpointDown } from 'utils/css'; -import { defer } from 'utils/react'; import { convertIndexToTime } from 'utils/timify'; import AvailabilitySearch, { defaultSearchOptions } from './AvailabilitySearch'; +import VenueDetailsPane from './VenueDetailsPane'; import VenueList from './VenueList'; -import VenueDetails from './VenueDetails'; import VenueLocation from './VenueLocation'; import styles from './VenuesContainer.scss'; -export type Params = { +type Params = { q: string; venue: string; }; type LoadedProps = { venues: VenueDetailList }; -type Props = RouteComponentProps & LoadedProps & WithBreakpoint; +type Props = LoadedProps; -type State = { - // View state - isMapExpanded: boolean; +export const VenuesContainerComponent: FC = ({ venues }) => { + const history = useHistory(); + const debouncedHistory = useMemo(() => new HistoryDebouncer(history), [history]); + const location = useLocation(); + const matchParams = useParams(); // Search state - searchBoxValue: string; // Value of the controlled search box; updated real-time - searchTerm: string; // Actual string to search with; deferred update - isAvailabilityEnabled: boolean; - searchOptions: VenueSearchOptions; - pristineSearchOptions: boolean; -}; - -const pageHead = Venues; - -export class VenuesContainerComponent extends Component { - history: HistoryDebouncer; - - constructor(props: Props) { - super(props); - - const { location, history } = props; + const [ + /** Value of the controlled search box; updated real-time */ + searchQuery, + setSearchQuery, + ] = useState(() => qs.parse(location.search).q || ''); + /** Actual string to search with; deferred update */ + const deferredSearchQuery = searchQuery; // TODO: Redundant now. Use React.useDeferredValue after we adopt concurrent mode + const [isAvailabilityEnabled, setIsAvailabilityEnabled] = useState(() => { + const params = qs.parse(location.search); + return !!(params.time && params.day && params.duration); + }); + const [searchOptions, setSearchOptions] = useState(() => { const params = qs.parse(location.search); - // Extract searchOptions from the query string if they are present - const isAvailabilityEnabled = !!(params.time && params.day && params.duration); - const searchOptions = isAvailabilityEnabled + return isAvailabilityEnabled ? (mapValues(pick(params, ['time', 'day', 'duration']), (i) => parseInt(i, 10), ) as VenueSearchOptions) : defaultSearchOptions(); + }); + const [pristineSearchOptions, setPristineSearchOptions] = useState(() => !isAvailabilityEnabled); - this.history = new HistoryDebouncer(history); - const searchTerm = params.q || ''; - this.state = { - searchOptions, - isAvailabilityEnabled, - isMapExpanded: false, - searchTerm, - searchBoxValue: searchTerm, - // eslint-disable-next-line react/no-unused-state - pristineSearchOptions: !isAvailabilityEnabled, - }; - } - - componentDidMount() { + // TODO: Check if this actually does anything useful + useEffect(() => { VenueLocation.preload(); - } - - componentDidUpdate(prevProps: Props, prevState: State) { - // Update URL if any of these props have changed - const { searchOptions, searchTerm, isAvailabilityEnabled } = this.state; - - if (isAvailabilityEnabled !== prevState.isAvailabilityEnabled) { - this.updateURL(false); - } else if (searchOptions !== prevState.searchOptions || searchTerm !== prevState.searchTerm) { - this.updateURL(); + }, []); + + const onFindFreeRoomsClicked = useCallback(() => { + setIsAvailabilityEnabled(!isAvailabilityEnabled); + if (pristineSearchOptions && !isAvailabilityEnabled) { + // Only reset search options if the user has never changed it, and if the + // search box is being opened. By resetting the option when the box is opened, + // the time when the box is opened will be used, instead of the time when the + // page is loaded + setSearchOptions(defaultSearchOptions()); } - } - - onFindFreeRoomsClicked = () => { - this.setState( - produce((draft) => { - const { pristineSearchOptions, isAvailabilityEnabled } = draft; - draft.isAvailabilityEnabled = !isAvailabilityEnabled; - - // Only reset search options if the user has never changed it, and if the - // search box is being opened. By resetting the option when the box is opened, - // the time when the box is opened will be used, instead of the time when the - // page is loaded - if (pristineSearchOptions && !isAvailabilityEnabled) { - draft.searchOptions = defaultSearchOptions(); - } - }), - ); - }; - - onClearVenueSelect = () => - this.props.history.push({ - ...this.props.history.location, - pathname: venuePage(), - }); - - onSearchBoxChange = (searchBoxValue: string) => { - this.setState({ searchBoxValue }); - }; - - onSearch = () => { - defer(() => this.setState((prevState) => ({ searchTerm: prevState.searchBoxValue.trim() }))); - }; - - onAvailabilityUpdate = (searchOptions: VenueSearchOptions) => { - if (!isEqual(searchOptions, this.state.searchOptions)) { - this.setState({ - searchOptions: clampClassDuration(searchOptions), - // eslint-disable-next-line react/no-unused-state - pristineSearchOptions: false, // user changed searchOptions - }); - } - }; - - onToggleMapExpanded = (isMapExpanded: boolean) => { - this.setState({ isMapExpanded }); - }; - - updateURL = (debounce = true) => { - const { searchTerm, isAvailabilityEnabled, searchOptions } = this.state; - let query: Partial = {}; - - if (searchTerm) query.q = searchTerm; - if (isAvailabilityEnabled) query = { ...query, ...searchOptions }; - - const pathname = venuePage(this.selectedVenue()); - const history = debounce ? this.history : this.props.history; - history.push({ - ...this.props.location, - search: qs.stringify(query), - pathname, - }); - }; - - getHighlightPeriod(): TimePeriod | undefined { - const { isAvailabilityEnabled, searchOptions } = this.state; + }, [isAvailabilityEnabled, pristineSearchOptions]); + + const onAvailabilityUpdate = useCallback( + (newSearchOptions: VenueSearchOptions) => { + if (!isEqual(newSearchOptions, searchOptions)) { + setSearchOptions(clampClassDuration(newSearchOptions)); + setPristineSearchOptions(false); // user changed searchOptions + } + }, + [searchOptions], + ); + + const highlightPeriod = useMemo(() => { if (!isAvailabilityEnabled) return undefined; return { @@ -175,17 +103,49 @@ export class VenuesContainerComponent extends Component { startTime: convertIndexToTime(searchOptions.time * 2), endTime: convertIndexToTime(2 * (searchOptions.time + searchOptions.duration)), }; - } + }, [isAvailabilityEnabled, searchOptions.day, searchOptions.duration, searchOptions.time]); - selectedVenue(): Venue | null { - const { venue } = this.props.match.params; - if (!venue) return null; - return decodeURIComponent(venue); - } + const selectedVenue = useMemo( + () => (matchParams.venue ? decodeURIComponent(matchParams.venue) : undefined), + [matchParams.venue], + ); - renderSearch() { - const { searchBoxValue, isAvailabilityEnabled, searchOptions } = this.state; + // Sync URL with component state + useEffect(() => { + let query: Partial = {}; + if (deferredSearchQuery) query.q = deferredSearchQuery; + if (isAvailabilityEnabled) query = { ...query, ...searchOptions }; + const search = qs.stringify(query); + + const pathname = venuePage(selectedVenue); + const proposedLocation: Location = { + ...history.location, + search, + pathname, + }; + + if (!locationsAreEqual(history.location, proposedLocation)) { + // TODO: Consider replacing our debounced history with + // `React.useTransition` or a React scheduler low priority callback. + debouncedHistory.push(proposedLocation); + } + }, [ + debouncedHistory, + deferredSearchQuery, + history, + isAvailabilityEnabled, + searchOptions, + selectedVenue, + ]); + + const matchedVenues = useMemo(() => { + const matched = searchVenue(venues, deferredSearchQuery); + return isAvailabilityEnabled ? filterAvailability(matched, searchOptions) : matched; + }, [isAvailabilityEnabled, searchOptions, deferredSearchQuery, venues]); + const matchedVenueNames = useMemo(() => matchedVenues.map(([venue]) => venue), [matchedVenues]); + + function renderSearch() { return (

      Venue Search

      @@ -193,12 +153,11 @@ export class VenuesContainerComponent extends Component {
      )} @@ -226,9 +185,9 @@ export class VenuesContainerComponent extends Component { ); } - renderNoResult(unfilteredCount: number) { - const { isAvailabilityEnabled } = this.state; - + const disableAvailability = useCallback(() => setIsAvailabilityEnabled(false), []); + function renderNoResult() { + const unfilteredCount = size(matchedVenues); return ( <> @@ -238,11 +197,7 @@ export class VenuesContainerComponent extends Component { ? 'There is a venue that is not shown because it is not free' : `There are ${unfilteredCount} venues that are not shown because they are not free`}
      -

      @@ -251,127 +206,36 @@ export class VenuesContainerComponent extends Component { ); } - renderSelectedVenue(matchedVenues: VenueDetailList) { - const selectedVenue = this.selectedVenue(); - const { venues } = this.props; - - if (!venues || !selectedVenue) return null; + return ( +
      + Venues - // Find the index of the current venue on the list of matched venues so - // we can obtain the previous and next item - const lowercaseSelectedVenue = selectedVenue.toLowerCase(); - const venueIndex = matchedVenues.findIndex( - ([venue]) => venue.toLowerCase() === lowercaseSelectedVenue, - ); +
      + {renderSearch()} - // The selected item may not be in the list of matched venues (if the user - // changed their search options afterwards), in which case we look for it in all - // venues - if (venueIndex === -1) { - const venueDetail = venues.find(([venue]) => venue.toLowerCase() === lowercaseSelectedVenue); - if (!venueDetail) return null; - const [venue, availability] = venueDetail; - return ( - - ); - } - - const [venue, availability] = matchedVenues[venueIndex]; - const [previous] = matchedVenues[venueIndex - 1] || ([] as string[]); - const [next] = matchedVenues[venueIndex + 1] || ([] as string[]); + {size(matchedVenues) === 0 ? ( + renderNoResult() + ) : ( + + )} +
      - return ( - - ); - } - - render() { - const selectedVenue = this.selectedVenue(); - const { searchTerm, isAvailabilityEnabled, isMapExpanded, searchOptions } = this.state; - const { venues } = this.props; - - let matchedVenues = searchVenue(venues, searchTerm); - const unfilteredCount = size(matchedVenues); - - if (isAvailabilityEnabled) { - matchedVenues = filterAvailability(matchedVenues, searchOptions); - } - - return ( -
      - {pageHead} - -
      - {this.renderSearch()} - - {size(matchedVenues) === 0 ? ( - this.renderNoResult(unfilteredCount) - ) : ( - venue)} - selectedVenue={selectedVenue} - /> - )} -
      - - - {this.props.matchBreakpoint ? ( - - - {this.renderSelectedVenue(matchedVenues)} - - ) : ( - <> -
      - {selectedVenue == null ? ( -
      - -

      Select a venue on the left to see its timetable

      -
      - ) : ( - this.renderSelectedVenue(matchedVenues) - )} -
      - - - )} -
      -
      - ); - } -} +
      + ); +}; -// Explicitly declare top level components for React hot reloading to work. -const ResponsiveVenuesContainer = makeResponsive(VenuesContainerComponent, breakpointDown('sm')); -const RoutedVenuesContainer = withRouter(ResponsiveVenuesContainer); -const AsyncVenuesContainer = Loadable.Map, { venues: AxiosResponse }>({ +const AsyncVenuesContainer = Loadable.Map, LoadedProps>({ loader: { - venues: () => axios.get(nusmods.venuesUrl(config.semester)), + venues: async () => { + const response = await axios.get(nusmods.venuesUrl(config.semester)); + return sortVenues(response.data); + }, }, loading: (props: LoadingComponentProps) => { if (props.error) { @@ -384,8 +248,8 @@ const AsyncVenuesContainer = Loadable.Map, { venues return null; }, - render(loaded, props) { - return ; + render({ venues }, props) { + return ; }, });