From a8f33e2824fb711a12c213519009d089c2734b91 Mon Sep 17 00:00:00 2001 From: Alejandro Celaya Date: Tue, 17 Sep 2024 10:26:41 +0200 Subject: [PATCH] Extract PaginatedMultiSelect to its own module --- .../dashboard/DashboardActivityFilters.tsx | 155 +------------ .../dashboard/PaginatedMultiSelect.tsx | 153 ++++++++++++ .../test/DashboardActivityFilters-test.js | 219 ++++-------------- .../test/PaginatedMultiSelect-test.js | 167 +++++++++++++ 4 files changed, 374 insertions(+), 320 deletions(-) create mode 100644 lms/static/scripts/frontend_apps/components/dashboard/PaginatedMultiSelect.tsx create mode 100644 lms/static/scripts/frontend_apps/components/dashboard/test/PaginatedMultiSelect-test.js diff --git a/lms/static/scripts/frontend_apps/components/dashboard/DashboardActivityFilters.tsx b/lms/static/scripts/frontend_apps/components/dashboard/DashboardActivityFilters.tsx index 9a7e4439df..858dfff85a 100644 --- a/lms/static/scripts/frontend_apps/components/dashboard/DashboardActivityFilters.tsx +++ b/lms/static/scripts/frontend_apps/components/dashboard/DashboardActivityFilters.tsx @@ -1,13 +1,10 @@ import { - Button, FilterClearIcon, LinkButton, MultiSelect, - RefreshIcon, } from '@hypothesis/frontend-shared'; -import type { ComponentChildren } from 'preact'; import type { MutableRef } from 'preact/hooks'; -import { useMemo, useRef } from 'preact/hooks'; +import { useMemo } from 'preact/hooks'; import { useParams } from 'wouter-preact'; import type { @@ -19,9 +16,9 @@ import type { StudentsResponse, } from '../../api-types'; import { useConfig } from '../../config'; -import type { PaginatedFetchResult } from '../../utils/api'; import { usePaginatedAPIFetch } from '../../utils/api'; import { formatDateTime } from '../../utils/date'; +import PaginatedMultiSelect from './PaginatedMultiSelect'; /** * Allow the user to select items from a paginated list of all items matching @@ -60,18 +57,6 @@ export type DashboardActivityFiltersProps = { onClearSelection?: () => void; }; -/** - * Checks if provided element's scroll is at the bottom. - * @param offset - Return true if the difference between the element's current - * and maximum scroll position is below this value. - * Defaults to 20. - */ -function elementScrollIsAtBottom(element: HTMLElement, offset = 20): boolean { - const distanceToTop = element.scrollTop + element.clientHeight; - const triggerPoint = element.scrollHeight - offset; - return distanceToTop >= triggerPoint; -} - type PropsWithElementRef = T & { /** Ref to be used on a `Select.Option` element */ elementRef?: MutableRef; @@ -121,139 +106,6 @@ function StudentOption({ ); } -type FiltersEntity = 'courses' | 'assignments' | 'students'; - -/** - * Placeholder to indicate a loading is in progress in one of the dropdowns - */ -function LoadingOption({ entity }: { entity: FiltersEntity }) { - return ( -
- Loading more {entity}... -
- ); -} - -type LoadingErrorProps = { - entity: FiltersEntity; - retry: () => void; -}; - -/** - * Indicates an error occurred while loading filters, and presents a button to - * retry. - */ -function LoadingError({ entity, retry }: LoadingErrorProps) { - return ( -
- Error loading {entity} - -
- ); -} - -type PaginatedMultiSelectProps = { - result: PaginatedFetchResult[]>; - activeItem?: TResult; - renderOption: ( - item: NonNullable, - ref?: MutableRef, - ) => ComponentChildren; - entity: FiltersEntity; - buttonContent?: ComponentChildren; - value: TSelect[]; - onChange: (newValue: TSelect[]) => void; -}; - -/** - * A MultiSelect whose data is fetched from a paginated API. - * It includes loading and error indicators, and transparently loads more data - * while scrolling. - */ -function PaginatedMultiSelect({ - result, - activeItem, - entity, - renderOption, - buttonContent, - value, - onChange, -}: PaginatedMultiSelectProps) { - const lastOptionRef = useRef(null); - - return ( - { - if (elementScrollIsAtBottom(e.target as HTMLUListElement)) { - result.loadNextPage(); - } - }} - > - - All {entity} - - {activeItem ? ( - renderOption(activeItem, lastOptionRef) - ) : ( - <> - {result.data?.map((item, index, list) => - renderOption( - item, - list.length - 1 === index ? lastOptionRef : undefined, - ), - )} - {result.isLoading && } - {result.error && ( - { - // Focus last option before retrying, to avoid the listbox to - // be closed: - // - Starting the fetch retry will cause the result to no - // longer be in the error state, hence the Retry button will - // be umounted. - // - If the retry button had focus when unmounted, the focus - // would revert to the document body. - // - Since the body is outside the select dropdown, this would - // cause the select dropdown to auto-close. - // - To avoid this we focus a different element just before - // initiating the retry. - lastOptionRef.current?.focus(); - result.retry(); - }} - /> - )} - - )} - - ); -} - /** * Renders drop-downs to select courses, assignments and/or students, used to * filter dashboard activity metrics. @@ -343,6 +195,7 @@ export default function DashboardActivityFilters({
@@ -377,6 +230,7 @@ export default function DashboardActivityFilters({ /> @@ -410,6 +264,7 @@ export default function DashboardActivityFilters({ /> students.onChange(newStudentIds)} diff --git a/lms/static/scripts/frontend_apps/components/dashboard/PaginatedMultiSelect.tsx b/lms/static/scripts/frontend_apps/components/dashboard/PaginatedMultiSelect.tsx new file mode 100644 index 0000000000..c80cf5f0ba --- /dev/null +++ b/lms/static/scripts/frontend_apps/components/dashboard/PaginatedMultiSelect.tsx @@ -0,0 +1,153 @@ +import { Button, MultiSelect, RefreshIcon } from '@hypothesis/frontend-shared'; +import type { ComponentChildren } from 'preact'; +import type { MutableRef } from 'preact/hooks'; +import { useRef } from 'preact/hooks'; + +import type { PaginatedFetchResult } from '../../utils/api'; + +type FiltersEntity = 'courses' | 'assignments' | 'students'; + +/** + * Placeholder to indicate a loading is in progress in one of the dropdowns + */ +function LoadingOption({ entity }: { entity: FiltersEntity }) { + return ( +
+ Loading more {entity}... +
+ ); +} + +type LoadingErrorProps = { + entity: FiltersEntity; + retry: () => void; +}; + +/** + * Indicates an error occurred while loading filters, and presents a button to + * retry. + */ +function LoadingError({ entity, retry }: LoadingErrorProps) { + return ( +
+ Error loading {entity} + +
+ ); +} + +/** + * Checks if provided element's scroll is at the bottom. + * + * @param offset - Return true if the difference between the element's current + * and maximum scroll position is below this value. + * Defaults to 20. + */ +function elementScrollIsAtBottom(element: HTMLElement, offset = 20): boolean { + const distanceToTop = element.scrollTop + element.clientHeight; + const triggerPoint = element.scrollHeight - offset; + return distanceToTop >= triggerPoint; +} + +export type PaginatedMultiSelectProps = { + result: PaginatedFetchResult[]>; + activeItem?: TResult; + renderOption: ( + item: NonNullable, + ref?: MutableRef, + ) => ComponentChildren; + entity: FiltersEntity; + buttonContent?: ComponentChildren; + value: TSelect[]; + onChange: (newValue: TSelect[]) => void; +}; + +/** + * A MultiSelect whose data is fetched from a paginated API. + * It includes loading and error indicators, and transparently loads more data + * while scrolling. + */ +export default function PaginatedMultiSelect({ + result, + activeItem, + entity, + renderOption, + buttonContent, + value, + onChange, +}: PaginatedMultiSelectProps) { + const lastOptionRef = useRef(null); + + return ( + { + if (elementScrollIsAtBottom(e.target as HTMLUListElement)) { + result.loadNextPage(); + } + }} + > + + All {entity} + + {activeItem ? ( + renderOption(activeItem, lastOptionRef) + ) : ( + <> + {result.data?.map((item, index, list) => + renderOption( + item, + list.length - 1 === index ? lastOptionRef : undefined, + ), + )} + {result.isLoading && } + {result.error && ( + { + // Focus last option before retrying, to avoid the listbox to + // be closed: + // - Starting the fetch retry will cause the result to no + // longer be in the error state, hence the Retry button will + // be umounted. + // - If the retry button had focus when unmounted, the focus + // would revert to the document body. + // - Since the body is outside the select dropdown, this would + // cause the select dropdown to auto-close. + // - To avoid this we focus a different element just before + // initiating the retry. + lastOptionRef.current?.focus(); + result.retry(); + }} + /> + )} + + )} + + ); +} diff --git a/lms/static/scripts/frontend_apps/components/dashboard/test/DashboardActivityFilters-test.js b/lms/static/scripts/frontend_apps/components/dashboard/test/DashboardActivityFilters-test.js index 28a4e070f7..0ac2bc45e3 100644 --- a/lms/static/scripts/frontend_apps/components/dashboard/test/DashboardActivityFilters-test.js +++ b/lms/static/scripts/frontend_apps/components/dashboard/test/DashboardActivityFilters-test.js @@ -172,7 +172,7 @@ describe('DashboardActivityFilters', () => { } function getSelect(wrapper, id) { - return wrapper.find(`MultiSelect[data-testid="${id}"]`); + return wrapper.find(`PaginatedMultiSelect[data-testid="${id}"]`); } function getSelectContent(wrapper, id) { @@ -204,43 +204,58 @@ describe('DashboardActivityFilters', () => { }); [ + // Course { id: 'courses-select', - expectedOptions: ['All courses', ...courses.map(c => c.title)], + entity: courses[0], + expectedText: courses[0].title, }, + // Assignment { id: 'assignments-select', - expectedOptions: [ - 'All assignments', - ...assignments.map(a => `${a.title}${formatDateTime(a.created)}`), - ], + entity: assignments[0], + expectedText: `${assignments[0].title}${formatDateTime(assignments[0].created)}`, }, + // Student with name { id: 'students-select', - expectedOptions: [ - 'All students', - ...studentsWithName.map(s => s.display_name), - 'Student name unavailable (ID: 12345)', - ], - expectedTitles: [undefined, undefined, undefined, 'User ID: 123456789'], + entity: students[0], + expectedText: students[0].display_name, }, - ].forEach(({ id, expectedOptions, expectedTitles = [] }) => { - it('renders corresponding options', () => { + // Student without name + { + id: 'students-select', + entity: students[students.length - 1], + expectedText: 'Student name unavailable (ID: 12345)', + expectedTitle: 'User ID: 123456789', + }, + ].forEach(({ id, entity, expectedText, expectedTitle }) => { + it('formats select options', () => { const wrapper = createComponent(); const select = getSelect(wrapper, id); - const options = select.find(MultiSelect.Option); - assert.equal(options.length, expectedOptions.length); - options.forEach((option, index) => { - assert.equal(option.text(), expectedOptions[index]); + // The option needs to be wrapped in a select, otherwise it throws. + const tempSelect = mount( + + {select.props().renderOption(entity)} + , + ); + const option = tempSelect.find(MultiSelect.Option); - if (expectedTitles[index]) { + try { + assert.equal(option.text(), expectedText); + + if (expectedTitle) { assert.equal( option.find('[data-testid="option-content-wrapper"]').prop('title'), - expectedTitles[index], + expectedTitle, ); } - }); + } finally { + // We need to unmount the temp select, to avoid a disconnected popover + // to be left in the DOM + tempSelect.unmount(); + } }); }); @@ -270,72 +285,6 @@ describe('DashboardActivityFilters', () => { }); }); - [true, false].forEach(isLoading => { - it('shows page loading indicators when loading', () => { - configureFakeAPIFetch({ isLoading }); - - const wrapper = createComponent(); - - assert.equal( - wrapper.exists('[data-testid="loading-more-courses"]'), - isLoading, - ); - assert.equal( - wrapper.exists('[data-testid="loading-more-assignments"]'), - isLoading, - ); - assert.equal( - wrapper.exists('[data-testid="loading-more-students"]'), - isLoading, - ); - }); - }); - - context('when scrolling listboxes down', () => { - [ - { - id: 'courses-select', - getExpectedCallback: () => fakeLoadNextCoursesPage, - }, - { - id: 'assignments-select', - getExpectedCallback: () => fakeLoadNextAssignmentsPage, - }, - { - id: 'students-select', - getExpectedCallback: () => fakeLoadNextStudentsPage, - }, - ].forEach(({ id, getExpectedCallback }) => { - it('loads next page when scroll is at the bottom', () => { - const wrapper = createComponent(); - const select = getSelect(wrapper, id); - - select.props().onListboxScroll({ - target: { - scrollTop: 100, - clientHeight: 50, - scrollHeight: 160, - }, - }); - assert.called(getExpectedCallback()); - }); - - it('does nothing when scroll is not at the bottom', () => { - const wrapper = createComponent(); - const select = getSelect(wrapper, id); - - select.props().onListboxScroll({ - target: { - scrollTop: 100, - clientHeight: 50, - scrollHeight: 250, - }, - }); - assert.notCalled(getExpectedCallback()); - }); - }); - }); - context('when items are selected', () => { [0, 1].forEach(index => { it('shows item name when only one is selected', () => { @@ -503,9 +452,7 @@ describe('DashboardActivityFilters', () => { students: emptySelection, }, selectId: 'courses-select', - allOption: 'All courses', skippedAPIFetchIndex: 0, - expectedOptionTitle: 'The active title', }, { props: { @@ -514,93 +461,25 @@ describe('DashboardActivityFilters', () => { students: emptySelection, }, selectId: 'assignments-select', - allOption: 'All assignments', skippedAPIFetchIndex: 1, - expectedOptionTitle: `The active title${formatDateTime(created)}`, }, - ].forEach( - ({ - props, - selectId, - allOption, - skippedAPIFetchIndex, - expectedOptionTitle, - }) => { - it('displays active item', () => { - const wrapper = createComponentWithProps(props); - const select = getSelectContent(wrapper, selectId); - - assert.equal(select, 'The active title'); - }); - - it('displays only two options in select', () => { - const wrapper = createComponentWithProps(props); - const select = getSelect(wrapper, selectId); - const options = select.find(MultiSelect.Option); - - assert.equal(options.length, 2); - assert.equal(options.at(0).text(), allOption); - assert.equal(options.at(1).text(), expectedOptionTitle); - }); + ].forEach(({ props, selectId, skippedAPIFetchIndex }) => { + it('displays active item', () => { + const wrapper = createComponentWithProps(props); + const select = getSelectContent(wrapper, selectId); - it('does not load list of items', () => { - createComponentWithProps(props); - - assert.calledWith( - fakeUsePaginatedAPIFetch.getCall(skippedAPIFetchIndex), - sinon.match.string, - null, // The path should be null - sinon.match.any, - ); - }); - }, - ); - }); - - describe('error handling', () => { - let fakeRetry; - - beforeEach(() => { - fakeRetry = sinon.stub(); - configureFakeAPIFetch({ - isLoading: false, - error: new Error('An error occurred'), - retry: fakeRetry, + assert.equal(select, 'The active title'); }); - }); - function clickRetry(wrapper, entity) { - wrapper - .find(`[data-testid="${entity}-error"]`) - .find('button') - .simulate('click'); - } - - ['courses', 'assignments', 'students'].forEach(entity => { - it('shows error message when loading filters fails', () => { - const wrapper = createComponent(); - assert.isTrue(wrapper.exists(`[data-testid="${entity}-error"]`)); - }); + it('does not load list of items', () => { + createComponentWithProps(props); - it('retries loading when retry button is clicked', () => { - const wrapper = createComponent(); - - clickRetry(wrapper, entity); - assert.called(fakeRetry); - }); - - it('focuses last option when retry button is clicked', () => { - const wrapper = createComponent(); - - const lastOption = wrapper - .find(`[data-testid="${entity}-select"]`) - .find('[role="option"]') - .last() - .getDOMNode(); - const focusOption = sinon.stub(lastOption, 'focus'); - - clickRetry(wrapper, entity); - assert.called(focusOption); + assert.calledWith( + fakeUsePaginatedAPIFetch.getCall(skippedAPIFetchIndex), + sinon.match.string, + null, // The path should be null + sinon.match.any, + ); }); }); }); diff --git a/lms/static/scripts/frontend_apps/components/dashboard/test/PaginatedMultiSelect-test.js b/lms/static/scripts/frontend_apps/components/dashboard/test/PaginatedMultiSelect-test.js new file mode 100644 index 0000000000..2a6dd6a46e --- /dev/null +++ b/lms/static/scripts/frontend_apps/components/dashboard/test/PaginatedMultiSelect-test.js @@ -0,0 +1,167 @@ +import { MultiSelect } from '@hypothesis/frontend-shared'; +import { mount } from 'enzyme'; + +import PaginatedMultiSelect from '../PaginatedMultiSelect'; + +describe('PaginatedMultiSelect', () => { + let wrappers; + + beforeEach(() => { + wrappers = []; + }); + + afterEach(() => { + wrappers.forEach(w => w.unmount()); + }); + + function createComponent(props) { + const wrapper = mount( + ( + {o} + )} + {...props} + />, + ); + wrappers.push(wrapper); + + return wrapper; + } + + function getSelect(wrapper, id = 'courses') { + return wrapper.find(`MultiSelect[data-testid="${id}-select"]`); + } + + it('renders options based on result data', () => { + const resultData = ['foo', 'bar', 'baz']; + const expectedOptions = ['All courses', ...resultData]; + const wrapper = createComponent({ + result: { + data: resultData, + }, + }); + const select = getSelect(wrapper); + const options = select.find(MultiSelect.Option); + + assert.equal(options.length, expectedOptions.length); + options.forEach((option, index) => { + assert.equal(option.text(), expectedOptions[index]); + }); + }); + + [true, false].forEach(isLoading => { + it('shows page loading indicators when loading', () => { + const wrapper = createComponent({ + result: { isLoading }, + }); + + assert.equal(wrapper.exists('LoadingOption'), isLoading); + }); + }); + + context('when scrolling listboxes down', () => { + function getScrollableSelect() { + const fakeLoadNextPage = sinon.stub(); + const wrapper = createComponent({ + result: { + loadNextPage: fakeLoadNextPage, + data: ['foo', 'bar', 'baz'], + }, + }); + const select = getSelect(wrapper); + + return { select, fakeLoadNextPage }; + } + + function scrollTo(select, scrollHeight) { + select.props().onListboxScroll({ + target: { + scrollTop: 100, + clientHeight: 50, + scrollHeight, + }, + }); + } + + it('loads next page when scroll is at the bottom', () => { + const { select, fakeLoadNextPage } = getScrollableSelect(); + + scrollTo(select, 160); + assert.called(fakeLoadNextPage); + }); + + it('does nothing when scroll is not at the bottom', () => { + const { select, fakeLoadNextPage } = getScrollableSelect(); + + scrollTo(select, 250); + assert.notCalled(fakeLoadNextPage); + }); + }); + + it('displays only active item if provided', () => { + const wrapper = createComponent({ + activeItem: 'This is displayed', + // Result is ignored + result: { + data: ['foo', 'bar', 'baz'], + }, + }); + const select = getSelect(wrapper); + const options = select.find(MultiSelect.Option); + + assert.equal(options.length, 2); + assert.equal(options.at(0).text(), 'All courses'); + assert.equal(options.at(1).text(), 'This is displayed'); + }); + + describe('error handling', () => { + let fakeRetry; + + beforeEach(() => { + fakeRetry = sinon.stub(); + }); + + function createComponentWithError() { + return createComponent({ + result: { + isLoading: false, + error: new Error('An error occurred'), + retry: fakeRetry, + }, + }); + } + + function clickRetry(wrapper) { + wrapper.find('button[data-testid="retry-button"]').simulate('click'); + } + + it('shows error message when loading filters fails', () => { + const wrapper = createComponentWithError(); + assert.isTrue(wrapper.exists('LoadingError')); + }); + + it('retries loading when retry button is clicked', () => { + const wrapper = createComponentWithError(); + + clickRetry(wrapper); + assert.called(fakeRetry); + }); + + it('focuses last option when retry button is clicked', () => { + const wrapper = createComponentWithError(); + + const lastOption = wrapper + .find(`[data-testid="courses-select"]`) + .find('[role="option"]') + .last() + .getDOMNode(); + const focusOption = sinon.stub(lastOption, 'focus'); + + clickRetry(wrapper); + assert.called(focusOption); + }); + }); +});