From b3e3620650d3f71a1c02aa2e0ecc5d172f2477e8 Mon Sep 17 00:00:00 2001 From: Moncef Abboud Date: Mon, 23 Oct 2023 10:42:48 +0200 Subject: [PATCH] fix: make filter dropdown closable by clicking outside + revert to single-value video filter --- src/editors/containers/VideoGallery/hooks.js | 29 ++++++-------- .../containers/VideoGallery/index.test.jsx | 11 +++--- .../containers/VideoGallery/messages.js | 6 +-- src/editors/containers/VideoGallery/utils.js | 3 +- .../MultiSelectFilterDropdown.jsx | 38 ------------------- .../SelectionModal/SearchSort.jsx | 31 +++++++++++++-- .../SelectionModal/SearchSort.test.jsx | 20 +++++----- 7 files changed, 59 insertions(+), 79 deletions(-) delete mode 100644 src/editors/sharedComponents/SelectionModal/MultiSelectFilterDropdown.jsx diff --git a/src/editors/containers/VideoGallery/hooks.js b/src/editors/containers/VideoGallery/hooks.js index 3c8c3ec24..2842976ad 100644 --- a/src/editors/containers/VideoGallery/hooks.js +++ b/src/editors/containers/VideoGallery/hooks.js @@ -7,7 +7,7 @@ import * as appHooks from '../../hooks'; import { selectors } from '../../data/redux'; import analyticsEvt from '../../data/constants/analyticsEvt'; import { - filterKeys, sortFunctions, sortKeys, sortMessages, + filterKeys, filterMessages, sortFunctions, sortKeys, sortMessages, } from './utils'; export const { @@ -18,17 +18,9 @@ export const { export const useSearchAndSortProps = () => { const [searchString, setSearchString] = React.useState(''); const [sortBy, setSortBy] = React.useState(sortKeys.dateNewest); - const [filterBy, setFilterBy] = React.useState([]); + const [filterBy, setFilterBy] = React.useState(filterKeys.anyStatus); const [hideSelectedVideos, setHideSelectedVideos] = React.useState(false); - const handleFilter = (key) => () => { - if (filterBy.includes(key)) { - setFilterBy(filterBy.filter(item => item !== key)); - } else { - setFilterBy([...filterBy, key]); - } - }; - return { searchString, onSearchChange: (e) => setSearchString(e.target.value), @@ -38,7 +30,9 @@ export const useSearchAndSortProps = () => { sortKeys, sortMessages, filterBy, - onFilterClick: handleFilter, + onFilterClick: (key) => () => setFilterBy(key), + filterKeys, + filterMessages, showSwitch: false, hideSelectedVideos, switchMessage: messages.hideSelectedCourseVideosSwitchLabel, @@ -54,15 +48,13 @@ export const filterListBySearch = ({ .includes(searchString.toLowerCase())) ); -export const filterListByStatus = ({ - statusFilter, - videoList, -}) => { - if (statusFilter.length === 0) { +export const filterListByStatus = ({ statusFilter, videoList }) => { + if (statusFilter === filterKeys.anyStatus) { return videoList; } - return videoList.filter(({ status }) => statusFilter.map(key => filterKeys[key]) - .includes(status)); + // TODO deal with translation mismatch because the video status is + // already translated in the backend + return videoList.filter(({ status }) => filterKeys[statusFilter] === status); }; export const filterListByHideSelectedCourse = ({ videoList }) => ( @@ -181,6 +173,7 @@ export const buildVideos = ({ rawVideos }) => { export const getstatusBadgeVariant = ({ status }) => { switch (status) { + // TODO deal with translation mismatch case filterKeys.failed: return 'danger'; case filterKeys.uploading: diff --git a/src/editors/containers/VideoGallery/index.test.jsx b/src/editors/containers/VideoGallery/index.test.jsx index 1ac21014d..26d1a46e3 100644 --- a/src/editors/containers/VideoGallery/index.test.jsx +++ b/src/editors/containers/VideoGallery/index.test.jsx @@ -163,13 +163,12 @@ describe('VideoGallery', () => { }], }); - await act(async () => { - fireEvent.click(screen.getByRole('button', { - name: 'Video status', - })); + act(() => { + fireEvent.click(screen.getByTestId('dropdown-filter')); }); - await act(async () => { - fireEvent.click(screen.getByRole('checkbox', { + + act(() => { + fireEvent.click(screen.getByRole('button', { name: filterBy, })); }); diff --git a/src/editors/containers/VideoGallery/messages.js b/src/editors/containers/VideoGallery/messages.js index a846b3956..5d293eee6 100644 --- a/src/editors/containers/VideoGallery/messages.js +++ b/src/editors/containers/VideoGallery/messages.js @@ -55,10 +55,10 @@ export const messages = { }, // Filter Dropdown - filterByVideoStatusNone: { + filterByVideoStatusAny: { id: 'authoring.selectvideomodal.filter.videostatusnone.label', - defaultMessage: 'Video status', - description: 'Dropdown label for filter by video status (none)', + defaultMessage: 'Any status', + description: 'Dropdown label for no filter (any status)', }, filterByVideoStatusUploading: { id: 'authoring.selectvideomodal.filter.videostatusuploading.label', diff --git a/src/editors/containers/VideoGallery/utils.js b/src/editors/containers/VideoGallery/utils.js index 8a1373948..94a60e487 100644 --- a/src/editors/containers/VideoGallery/utils.js +++ b/src/editors/containers/VideoGallery/utils.js @@ -22,6 +22,7 @@ export const sortMessages = StrictDict({ }); export const filterKeys = StrictDict({ + anyStatus: 'anyStatus', uploading: 'Uploading', processing: 'In Progress', ready: 'Ready', @@ -29,7 +30,7 @@ export const filterKeys = StrictDict({ }); export const filterMessages = StrictDict({ - title: messages[messageKeys.filterByVideoStatusNone], + anyStatus: messages[messageKeys.filterByVideoStatusAny], uploading: messages[messageKeys.filterByVideoStatusUploading], processing: messages[messageKeys.filterByVideoStatusProcessing], ready: messages[messageKeys.filterByVideoStatusReady], diff --git a/src/editors/sharedComponents/SelectionModal/MultiSelectFilterDropdown.jsx b/src/editors/sharedComponents/SelectionModal/MultiSelectFilterDropdown.jsx deleted file mode 100644 index b6ec2fd6c..000000000 --- a/src/editors/sharedComponents/SelectionModal/MultiSelectFilterDropdown.jsx +++ /dev/null @@ -1,38 +0,0 @@ -import React from 'react'; - -import { useIntl } from '@edx/frontend-platform/i18n'; -import { Dropdown, DropdownToggle, Form } from '@edx/paragon'; - -import PropTypes from 'prop-types'; -import { filterKeys, filterMessages } from '../../containers/VideoGallery/utils'; - -const MultiSelectFilterDropdown = ({ - selected, onSelectionChange, -}) => { - const intl = useIntl(); - return ( - - - {intl.formatMessage(filterMessages.title)} - - - {Object.keys(filterKeys).map(key => ( - - {intl.formatMessage(filterMessages[key])} - - ))} - - - ); -}; - -MultiSelectFilterDropdown.propTypes = { - selected: PropTypes.arrayOf(PropTypes.string).isRequired, - onSelectionChange: PropTypes.func.isRequired, -}; -export default MultiSelectFilterDropdown; diff --git a/src/editors/sharedComponents/SelectionModal/SearchSort.jsx b/src/editors/sharedComponents/SelectionModal/SearchSort.jsx index d9b00e554..15a5422e7 100644 --- a/src/editors/sharedComponents/SelectionModal/SearchSort.jsx +++ b/src/editors/sharedComponents/SelectionModal/SearchSort.jsx @@ -2,7 +2,7 @@ import React from 'react'; import PropTypes from 'prop-types'; import { - ActionRow, Form, Icon, IconButton, SelectMenu, MenuItem, + ActionRow, Dropdown, Form, Icon, IconButton, SelectMenu, MenuItem, } from '@edx/paragon'; import { Close, Search } from '@edx/paragon/icons'; import { @@ -11,7 +11,6 @@ import { } from '@edx/frontend-platform/i18n'; import messages from './messages'; -import MultiSelectFilterDropdown from './MultiSelectFilterDropdown'; import { sortKeys, sortMessages } from '../../containers/VideoGallery/utils'; export const SearchSort = ({ @@ -22,6 +21,8 @@ export const SearchSort = ({ onSortClick, filterBy, onFilterClick, + filterKeys, + filterMessages, showSwitch, switchMessage, onSwitchClick, @@ -62,7 +63,25 @@ export const SearchSort = ({ ))} - {onFilterClick && } + { onFilterClick && ( + + + + + + {Object.keys(filterKeys).map(key => ( + + + + ))} + + + )} { showSwitch && ( <> @@ -86,6 +105,8 @@ export const SearchSort = ({ SearchSort.defaultProps = { filterBy: '', onFilterClick: null, + filterKeys: null, + filterMessages: null, showSwitch: false, onSwitchClick: null, }; @@ -96,8 +117,10 @@ SearchSort.propTypes = { clearSearchString: PropTypes.func.isRequired, sortBy: PropTypes.string.isRequired, onSortClick: PropTypes.func.isRequired, - filterBy: PropTypes.arrayOf(PropTypes.string), + filterBy: PropTypes.string, onFilterClick: PropTypes.func, + filterKeys: PropTypes.shape({}), + filterMessages: PropTypes.shape({}), showSwitch: PropTypes.bool, switchMessage: PropTypes.shape({}).isRequired, onSwitchClick: PropTypes.func, diff --git a/src/editors/sharedComponents/SelectionModal/SearchSort.test.jsx b/src/editors/sharedComponents/SelectionModal/SearchSort.test.jsx index 74899d3a2..d7e0395c1 100644 --- a/src/editors/sharedComponents/SelectionModal/SearchSort.test.jsx +++ b/src/editors/sharedComponents/SelectionModal/SearchSort.test.jsx @@ -7,7 +7,7 @@ import { import { IntlProvider } from '@edx/frontend-platform/i18n'; import { sortKeys, sortMessages } from '../ImageUploadModal/SelectImageModal/utils'; -import { filterMessages } from '../../containers/VideoGallery/utils'; +import { filterKeys, filterMessages } from '../../containers/VideoGallery/utils'; import { SearchSort } from './SearchSort'; import messages from './messages'; @@ -32,7 +32,10 @@ describe('SearchSort component', () => { id: 'test.id', defaultMessage: 'test message', }, + filterBy: filterKeys.anyStatus, onFilterClick: jest.fn(), + filterKeys, + filterMessages, showSwitch: true, }; @@ -68,17 +71,16 @@ describe('SearchSort component', () => { .toBeInTheDocument(); }); }); - test('adds a filter option for each filterKet', async () => { - const { getByRole } = getComponent(); - await act(() => { - fireEvent.click(screen.getByRole('button', { name: /video status/i })); + test('adds a filter option for each filter key', async () => { + const { getByTestId } = getComponent(); + act(() => { + fireEvent.click(getByTestId('dropdown-filter')); }); + Object.keys(filterMessages) .forEach((key) => { - if (key !== 'title') { - expect(getByRole('checkbox', { name: filterMessages[key].defaultMessage })) - .toBeInTheDocument(); - } + expect(getByTestId(`dropdown-filter-${key}`)) + .toBeInTheDocument(); }); }); test('searchbox should show clear message button when not empty', async () => {