Skip to content

Commit

Permalink
fix: make filter dropdown closable by clicking outside + revert to si…
Browse files Browse the repository at this point in the history
…ngle-value video filter (#414)
  • Loading branch information
CefBoud authored Nov 30, 2023
1 parent bad7635 commit 8928d35
Show file tree
Hide file tree
Showing 7 changed files with 59 additions and 79 deletions.
29 changes: 11 additions & 18 deletions src/editors/containers/VideoGallery/hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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),
Expand All @@ -38,7 +30,9 @@ export const useSearchAndSortProps = () => {
sortKeys,
sortMessages,
filterBy,
onFilterClick: handleFilter,
onFilterClick: (key) => () => setFilterBy(key),
filterKeys,
filterMessages,
showSwitch: false,
hideSelectedVideos,
switchMessage: messages.hideSelectedCourseVideosSwitchLabel,
Expand All @@ -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 }) => (
Expand Down Expand Up @@ -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:
Expand Down
11 changes: 5 additions & 6 deletions src/editors/containers/VideoGallery/index.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}));
});
Expand Down
6 changes: 3 additions & 3 deletions src/editors/containers/VideoGallery/messages.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
3 changes: 2 additions & 1 deletion src/editors/containers/VideoGallery/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,15 @@ export const sortMessages = StrictDict({
});

export const filterKeys = StrictDict({
anyStatus: 'anyStatus',
uploading: 'Uploading',
processing: 'In Progress',
ready: 'Ready',
failed: 'Failed',
});

export const filterMessages = StrictDict({
title: messages[messageKeys.filterByVideoStatusNone],
anyStatus: messages[messageKeys.filterByVideoStatusAny],
uploading: messages[messageKeys.filterByVideoStatusUploading],
processing: messages[messageKeys.filterByVideoStatusProcessing],
ready: messages[messageKeys.filterByVideoStatusReady],
Expand Down

This file was deleted.

31 changes: 27 additions & 4 deletions src/editors/sharedComponents/SelectionModal/SearchSort.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 = ({
Expand All @@ -22,6 +21,8 @@ export const SearchSort = ({
onSortClick,
filterBy,
onFilterClick,
filterKeys,
filterMessages,
showSwitch,
switchMessage,
onSwitchClick,
Expand Down Expand Up @@ -62,7 +63,25 @@ export const SearchSort = ({
))}
</SelectMenu>

{onFilterClick && <MultiSelectFilterDropdown selected={filterBy} onSelectionChange={onFilterClick} />}
{ onFilterClick && (
<Dropdown>
<Dropdown.Toggle
data-testid="dropdown-filter"
className="text-gray-700"
id="gallery-filter-button"
variant="tertiary"
>
<FormattedMessage {...filterMessages[filterBy]} />
</Dropdown.Toggle>
<Dropdown.Menu>
{Object.keys(filterKeys).map(key => (
<Dropdown.Item data-testid={`dropdown-filter-${key}`} key={key} onClick={onFilterClick(key)}>
<FormattedMessage {...filterMessages[key]} />
</Dropdown.Item>
))}
</Dropdown.Menu>
</Dropdown>
)}

{ showSwitch && (
<>
Expand All @@ -86,6 +105,8 @@ export const SearchSort = ({
SearchSort.defaultProps = {
filterBy: '',
onFilterClick: null,
filterKeys: null,
filterMessages: null,
showSwitch: false,
onSwitchClick: null,
};
Expand All @@ -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,
Expand Down
20 changes: 11 additions & 9 deletions src/editors/sharedComponents/SelectionModal/SearchSort.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -32,7 +32,10 @@ describe('SearchSort component', () => {
id: 'test.id',
defaultMessage: 'test message',
},
filterBy: filterKeys.anyStatus,
onFilterClick: jest.fn(),
filterKeys,
filterMessages,
showSwitch: true,
};

Expand Down Expand Up @@ -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 () => {
Expand Down

0 comments on commit 8928d35

Please sign in to comment.