diff --git a/ui/src/common/SelectLibraryInput.jsx b/ui/src/common/SelectLibraryInput.jsx index 4d34fd822..4825693ea 100644 --- a/ui/src/common/SelectLibraryInput.jsx +++ b/ui/src/common/SelectLibraryInput.jsx @@ -8,8 +8,9 @@ import { ListItemIcon, ListItemText, Typography, + Box, } from '@material-ui/core' -import { useGetList } from 'react-admin' +import { useGetList, useTranslate } from 'react-admin' import PropTypes from 'prop-types' import { makeStyles } from '@material-ui/core' @@ -18,6 +19,17 @@ const useStyles = makeStyles((theme) => ({ width: '960px', // MD breakpoint width maxWidth: '100%', }, + headerContainer: { + display: 'flex', + alignItems: 'center', + marginBottom: theme.spacing(1), + paddingLeft: theme.spacing(1), + }, + masterCheckbox: { + padding: '7px', // Match the default Material-UI checkbox padding + marginLeft: '-9px', // Negative margin to align with FormControlLabel checkboxes + marginRight: theme.spacing(1), + }, libraryList: { height: '120px', // 3 rows of libraries overflow: 'auto', @@ -72,6 +84,7 @@ const LibraryListItem = ({ library, isSelected, onToggle }) => { export const SelectLibraryInput = ({ onChange, value = [] }) => { const classes = useStyles() + const translate = useTranslate() const [selectedLibraryIds, setSelectedLibraryIds] = useState([]) const { ids, data } = useGetList( @@ -109,8 +122,35 @@ export const SelectLibraryInput = ({ onChange, value = [] }) => { onChange(newSelection) } + const handleMasterCheckboxChange = () => { + const isAllSelected = selectedLibraryIds.length === options.length + const newSelection = isAllSelected ? [] : options.map((lib) => lib.id) + + setSelectedLibraryIds(newSelection) + onChange(newSelection) + } + + const selectedCount = selectedLibraryIds.length + const totalCount = options.length + const isAllSelected = selectedCount === totalCount && totalCount > 0 + const isIndeterminate = selectedCount > 0 && selectedCount < totalCount + return (
+ {options.length > 1 && ( + + + + {translate('resources.user.message.selectAllLibraries')} + + + )} {options.length === 0 ? ( diff --git a/ui/src/common/SelectLibraryInput.test.jsx b/ui/src/common/SelectLibraryInput.test.jsx index 706ece428..35e640591 100644 --- a/ui/src/common/SelectLibraryInput.test.jsx +++ b/ui/src/common/SelectLibraryInput.test.jsx @@ -4,9 +4,50 @@ import { SelectLibraryInput } from './SelectLibraryInput' import { useGetList } from 'react-admin' import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest' +// Mock Material-UI components +vi.mock('@material-ui/core', () => ({ + List: ({ children }) =>
{children}
, + ListItem: ({ children, button, onClick, dense, className }) => ( + + ), + ListItemIcon: ({ children }) => {children}, + ListItemText: ({ primary }) => {primary}, + Typography: ({ children, variant }) => {children}, + Box: ({ children, className }) =>
{children}
, + Checkbox: ({ + checked, + indeterminate, + onChange, + size, + className, + ...props + }) => ( + { + if (el) el.indeterminate = indeterminate + }} + onChange={onChange} + className={className} + {...props} + /> + ), + makeStyles: () => () => ({}), +})) + +// Mock Material-UI icons +vi.mock('@material-ui/icons', () => ({ + CheckBox: () => ☑, + CheckBoxOutlineBlank: () => ☐, +})) + // Mock the react-admin hook vi.mock('react-admin', () => ({ useGetList: vi.fn(), + useTranslate: vi.fn(() => (key) => key), // Simple translation mock })) describe('', () => { @@ -60,8 +101,9 @@ describe('', () => { }) render() - // First test - click to select - fireEvent.click(screen.getAllByRole('button')[0]) // Click first list item + // Find the library buttons by their text content + const library1Button = screen.getByText('Library 1').closest('button') + fireEvent.click(library1Button) expect(mockOnChange).toHaveBeenCalledWith(['1']) // Clean up to avoid DOM duplication @@ -75,8 +117,11 @@ describe('', () => { }) render() - // Second test - click to deselect - fireEvent.click(screen.getAllByRole('button')[0]) // Click first list item again + // Find the library button again and click to deselect + const library1ButtonDeselect = screen + .getByText('Library 1') + .closest('button') + fireEvent.click(library1ButtonDeselect) expect(mockOnChange).toHaveBeenCalledWith([]) }) @@ -96,8 +141,9 @@ describe('', () => { // Check that checkbox for Library 1 is checked const checkboxes = screen.getAllByRole('checkbox') - expect(checkboxes[0].checked).toBe(true) - expect(checkboxes[1].checked).toBe(false) + // With master checkbox, individual checkboxes start at index 1 + expect(checkboxes[1].checked).toBe(true) // Library 1 + expect(checkboxes[2].checked).toBe(false) // Library 2 }) it('should handle value as array of objects', () => { @@ -116,7 +162,101 @@ describe('', () => { // Check that checkbox for Library 2 is checked const checkboxes = screen.getAllByRole('checkbox') - expect(checkboxes[0].checked).toBe(false) - expect(checkboxes[1].checked).toBe(true) + // With master checkbox, index shifts by 1 + expect(checkboxes[1].checked).toBe(false) // Library 1 + expect(checkboxes[2].checked).toBe(true) // Library 2 + }) + + it('should render master checkbox when there are multiple libraries', () => { + // Mock libraries + const mockLibraries = { + 1: { id: '1', name: 'Library 1' }, + 2: { id: '2', name: 'Library 2' }, + } + useGetList.mockReturnValue({ + ids: ['1', '2'], + data: mockLibraries, + }) + + render() + + // Should render master checkbox plus individual checkboxes + const checkboxes = screen.getAllByRole('checkbox') + expect(checkboxes).toHaveLength(3) // 1 master + 2 individual + expect( + screen.getByText('resources.user.message.selectAllLibraries'), + ).not.toBeNull() + }) + + it('should not render master checkbox when there is only one library', () => { + // Mock single library + const mockLibraries = { + 1: { id: '1', name: 'Library 1' }, + } + useGetList.mockReturnValue({ + ids: ['1'], + data: mockLibraries, + }) + + render() + + // Should render only individual checkbox + const checkboxes = screen.getAllByRole('checkbox') + expect(checkboxes).toHaveLength(1) // Only 1 individual checkbox + }) + + it('should handle master checkbox selection and deselection', () => { + // Mock libraries + const mockLibraries = { + 1: { id: '1', name: 'Library 1' }, + 2: { id: '2', name: 'Library 2' }, + } + useGetList.mockReturnValue({ + ids: ['1', '2'], + data: mockLibraries, + }) + + render() + + const checkboxes = screen.getAllByRole('checkbox') + const masterCheckbox = checkboxes[0] // Master is first + + // Click master checkbox to select all + fireEvent.click(masterCheckbox) + expect(mockOnChange).toHaveBeenCalledWith(['1', '2']) + + // Clean up and test deselect all + cleanup() + mockOnChange.mockClear() + + render() + const checkboxes2 = screen.getAllByRole('checkbox') + const masterCheckbox2 = checkboxes2[0] + + // Click master checkbox to deselect all + fireEvent.click(masterCheckbox2) + expect(mockOnChange).toHaveBeenCalledWith([]) + }) + + it('should show master checkbox as indeterminate when some libraries are selected', () => { + // Mock libraries + const mockLibraries = { + 1: { id: '1', name: 'Library 1' }, + 2: { id: '2', name: 'Library 2' }, + } + useGetList.mockReturnValue({ + ids: ['1', '2'], + data: mockLibraries, + }) + + render() + + const checkboxes = screen.getAllByRole('checkbox') + const masterCheckbox = checkboxes[0] // Master is first + + // Master checkbox should not be checked when only some libraries are selected + expect(masterCheckbox.checked).toBe(false) + // Note: Testing indeterminate property directly through JSDOM can be unreliable + // The important behavior is that it's not checked when only some are selected }) }) diff --git a/ui/src/i18n/en.json b/ui/src/i18n/en.json index 9525b878d..72cc3adb9 100644 --- a/ui/src/i18n/en.json +++ b/ui/src/i18n/en.json @@ -166,9 +166,8 @@ }, "message": { "listenBrainzToken": "Enter your ListenBrainz user token.", - "clickHereForToken": "Click here to get your token" - }, - "messages": { + "clickHereForToken": "Click here to get your token", + "selectAllLibraries": "Select all libraries", "adminAutoLibraries": "Admin users automatically have access to all libraries" } }, diff --git a/ui/src/user/UserCreate.jsx b/ui/src/user/UserCreate.jsx index 182f66ab5..ee1fa5f79 100644 --- a/ui/src/user/UserCreate.jsx +++ b/ui/src/user/UserCreate.jsx @@ -95,7 +95,7 @@ const UserCreate = (props) => { color="textSecondary" style={{ marginTop: 16, marginBottom: 16 }} > - {translate('resources.user.messages.adminAutoLibraries')} + {translate('resources.user.message.adminAutoLibraries')} )} diff --git a/ui/src/user/UserEdit.jsx b/ui/src/user/UserEdit.jsx index 9401f8f7c..3406aa4c4 100644 --- a/ui/src/user/UserEdit.jsx +++ b/ui/src/user/UserEdit.jsx @@ -171,7 +171,7 @@ const UserEdit = (props) => { color="textSecondary" style={{ marginTop: 16, marginBottom: 16 }} > - {translate('resources.user.messages.adminAutoLibraries')} + {translate('resources.user.message.adminAutoLibraries')} )}