From 8e32eeae934896aec8e6fc9930faa1f69de07941 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= Date: Fri, 30 May 2025 23:15:02 -0400 Subject: [PATCH] fix(ui): add button is covered when adding to a playlist (#4156) * refactor: fix SelectPlaylistInput layout and improve readability - Replace dropdown with fixed list to prevent button overlay - Break down into smaller focused components - Add comprehensive test coverage - Reduce spacing for compact layout * refactor: update playlist input translations Signed-off-by: Deluan * fix: format code with prettier - Fix formatting issues in AddToPlaylistDialog.test.jsx --------- Signed-off-by: Deluan --- resources/i18n/pt-br.json | 10 +- ui/src/dialogs/AddToPlaylistDialog.test.jsx | 26 +- ui/src/dialogs/SelectPlaylistInput.jsx | 436 +++++++++++++--- ui/src/dialogs/SelectPlaylistInput.test.jsx | 548 ++++++++++++++++---- ui/src/i18n/en.json | 10 +- 5 files changed, 840 insertions(+), 190 deletions(-) diff --git a/resources/i18n/pt-br.json b/resources/i18n/pt-br.json index cc771e8fa..cfb3c8485 100644 --- a/resources/i18n/pt-br.json +++ b/resources/i18n/pt-br.json @@ -197,11 +197,17 @@ "export": "Exportar", "makePublic": "Pública", "makePrivate": "Pessoal", - "saveQueue": "Salvar fila em nova Playlist" + "saveQueue": "Salvar fila em nova Playlist", + "searchOrCreate": "Buscar playlists ou criar nova...", + "pressEnterToCreate": "Pressione Enter para criar nova playlist", + "removeFromSelection": "Remover da seleção", + "removeSymbol": "×" }, "message": { "duplicate_song": "Adicionar músicas duplicadas", - "song_exist": "Algumas destas músicas já existem na playlist. Você quer adicionar as duplicadas ou ignorá-las?" + "song_exist": "Algumas destas músicas já existem na playlist. Você quer adicionar as duplicadas ou ignorá-las?", + "noPlaylistsFound": "Nenhuma playlist encontrada", + "noPlaylists": "Nenhuma playlist disponível" } }, "radio": { diff --git a/ui/src/dialogs/AddToPlaylistDialog.test.jsx b/ui/src/dialogs/AddToPlaylistDialog.test.jsx index a019ee03c..60d3cca0d 100644 --- a/ui/src/dialogs/AddToPlaylistDialog.test.jsx +++ b/ui/src/dialogs/AddToPlaylistDialog.test.jsx @@ -88,12 +88,18 @@ describe('AddToPlaylistDialog', () => { createTestUtils(mockDataProvider) + // Filter to see sample playlists let textBox = screen.getByRole('textbox') fireEvent.change(textBox, { target: { value: 'sample' } }) - fireEvent.keyDown(textBox, { key: 'ArrowDown' }) - fireEvent.keyDown(textBox, { key: 'Enter' }) - fireEvent.keyDown(textBox, { key: 'ArrowDown' }) - fireEvent.keyDown(textBox, { key: 'Enter' }) + + // Click on first playlist + const firstPlaylist = screen.getByText('sample playlist 1') + fireEvent.click(firstPlaylist) + + // Click on second playlist + const secondPlaylist = screen.getByText('sample playlist 2') + fireEvent.click(secondPlaylist) + await waitFor(() => { expect(screen.getByTestId('playlist-add')).not.toBeDisabled() }) @@ -133,12 +139,11 @@ describe('AddToPlaylistDialog', () => { createTestUtils(mockDataProvider) + // Type a new playlist name and press Enter to create it let textBox = screen.getByRole('textbox') fireEvent.change(textBox, { target: { value: 'sample' } }) - fireEvent.keyDown(textBox, { key: 'ArrowDown' }) - fireEvent.keyDown(textBox, { key: 'ArrowDown' }) - fireEvent.keyDown(textBox, { key: 'ArrowDown' }) fireEvent.keyDown(textBox, { key: 'Enter' }) + await waitFor(() => { expect(screen.getByTestId('playlist-add')).not.toBeDisabled() }) @@ -171,14 +176,15 @@ describe('AddToPlaylistDialog', () => { createTestUtils(mockDataProvider) + // Create first playlist let textBox = screen.getByRole('textbox') fireEvent.change(textBox, { target: { value: 'sample' } }) - fireEvent.keyDown(textBox, { key: 'ArrowDown' }) - fireEvent.keyDown(textBox, { key: 'ArrowDown' }) - fireEvent.keyDown(textBox, { key: 'ArrowDown' }) fireEvent.keyDown(textBox, { key: 'Enter' }) + + // Create second playlist fireEvent.change(textBox, { target: { value: 'new playlist' } }) fireEvent.keyDown(textBox, { key: 'Enter' }) + await waitFor(() => { expect(screen.getByTestId('playlist-add')).not.toBeDisabled() }) diff --git a/ui/src/dialogs/SelectPlaylistInput.jsx b/ui/src/dialogs/SelectPlaylistInput.jsx index 372d5f408..effbf5416 100644 --- a/ui/src/dialogs/SelectPlaylistInput.jsx +++ b/ui/src/dialogs/SelectPlaylistInput.jsx @@ -1,26 +1,251 @@ -import React from 'react' +import React, { useState } from 'react' import TextField from '@material-ui/core/TextField' import Checkbox from '@material-ui/core/Checkbox' import CheckBoxIcon from '@material-ui/icons/CheckBox' import CheckBoxOutlineBlankIcon from '@material-ui/icons/CheckBoxOutlineBlank' -import Autocomplete, { - createFilterOptions, -} from '@material-ui/lab/Autocomplete' +import { + List, + ListItem, + ListItemIcon, + ListItemText, + Typography, + Box, + InputAdornment, + IconButton, +} from '@material-ui/core' +import AddIcon from '@material-ui/icons/Add' import { useGetList, useTranslate } from 'react-admin' import PropTypes from 'prop-types' import { isWritable } from '../common' import { makeStyles } from '@material-ui/core' -const filter = createFilterOptions() - -const useStyles = makeStyles({ +const useStyles = makeStyles((theme) => ({ root: { width: '100%' }, - checkbox: { marginRight: 8 }, -}) + searchField: { + marginBottom: theme.spacing(2), + width: '100%', + }, + playlistList: { + maxHeight: 200, + overflow: 'auto', + border: `1px solid ${theme.palette.divider}`, + borderRadius: theme.shape.borderRadius, + backgroundColor: theme.palette.background.paper, + }, + listItem: { + paddingTop: 0, + paddingBottom: 0, + }, + selectedPlaylistsContainer: { + marginTop: theme.spacing(2), + }, + selectedPlaylist: { + display: 'inline-flex', + alignItems: 'center', + margin: theme.spacing(0.5), + padding: theme.spacing(0.5, 1), + backgroundColor: theme.palette.primary.main, + color: theme.palette.primary.contrastText, + borderRadius: theme.shape.borderRadius, + fontSize: '0.875rem', + }, + removeButton: { + marginLeft: theme.spacing(0.5), + padding: 2, + color: 'inherit', + }, + emptyMessage: { + padding: theme.spacing(2), + textAlign: 'center', + color: theme.palette.text.secondary, + }, +})) + +const PlaylistSearchField = ({ + searchText, + onSearchChange, + onCreateNew, + onKeyDown, + canCreateNew, +}) => { + const classes = useStyles() + const translate = useTranslate() + + return ( + onSearchChange(e.target.value)} + onKeyDown={onKeyDown} + placeholder={translate('resources.playlist.actions.searchOrCreate')} + InputProps={{ + endAdornment: canCreateNew && ( + + + + + + ), + }} + /> + ) +} + +const EmptyPlaylistMessage = ({ searchText, canCreateNew }) => { + const classes = useStyles() + const translate = useTranslate() + + return ( +
+ + {searchText + ? translate('resources.playlist.message.noPlaylistsFound') + : translate('resources.playlist.message.noPlaylists')} + + {canCreateNew && ( + + {translate('resources.playlist.actions.pressEnterToCreate')} + + )} +
+ ) +} + +const PlaylistListItem = ({ playlist, isSelected, onToggle }) => { + const classes = useStyles() + + return ( + onToggle(playlist)} + dense + > + + } + checkedIcon={} + checked={isSelected} + tabIndex={-1} + disableRipple + /> + + + + ) +} + +const CreatePlaylistItem = ({ searchText, onCreateNew }) => { + const classes = useStyles() + const translate = useTranslate() + + return ( + + + + + + + ) +} + +const PlaylistList = ({ + filteredOptions, + selectedPlaylists, + onPlaylistToggle, + searchText, + canCreateNew, + onCreateNew, +}) => { + const classes = useStyles() + + const isPlaylistSelected = (playlist) => + selectedPlaylists.some((p) => p.id === playlist.id) + + return ( + + {filteredOptions.length === 0 ? ( + + ) : ( + filteredOptions.map((playlist) => ( + + )) + )} + {canCreateNew && filteredOptions.length > 0 && ( + + )} + + ) +} + +const SelectedPlaylistChip = ({ playlist, onRemove }) => { + const classes = useStyles() + const translate = useTranslate() + + return ( + + {playlist.name} + onRemove(playlist)} + title={translate('resources.playlist.actions.removeFromSelection')} + > + {translate('resources.playlist.actions.removeSymbol')} + + + ) +} + +const SelectedPlaylistsDisplay = ({ selectedPlaylists, onRemoveSelected }) => { + const classes = useStyles() + const translate = useTranslate() + + if (selectedPlaylists.length === 0) { + return null + } + + return ( + + + {selectedPlaylists.map((playlist, index) => ( + + ))} + + + ) +} export const SelectPlaylistInput = ({ onChange }) => { const classes = useStyles() - const translate = useTranslate() + const [searchText, setSearchText] = useState('') + const [selectedPlaylists, setSelectedPlaylists] = useState([]) + const { ids, data } = useGetList( 'playlist', { page: 1, perPage: -1 }, @@ -32,92 +257,131 @@ export const SelectPlaylistInput = ({ onChange }) => { ids && ids.map((id) => data[id]).filter((option) => isWritable(option.ownerId)) - const handleOnChange = (event, newValue) => { - let newState = [] - if (newValue && newValue.length) { - newValue.forEach((playlistObject) => { - if (playlistObject.inputValue) { - newState.push({ - name: playlistObject.inputValue, - }) - } else if (typeof playlistObject === 'string') { - newState.push({ - name: playlistObject, - }) - } else { - newState.push(playlistObject) - } - }) + // Filter playlists based on search text + const filteredOptions = + options?.filter((option) => + option.name.toLowerCase().includes(searchText.toLowerCase()), + ) || [] + + const handlePlaylistToggle = (playlist) => { + const isSelected = selectedPlaylists.some((p) => p.id === playlist.id) + let newSelection + + if (isSelected) { + newSelection = selectedPlaylists.filter((p) => p.id !== playlist.id) + } else { + newSelection = [...selectedPlaylists, playlist] } - onChange(newState) + + setSelectedPlaylists(newSelection) + onChange(newSelection) } - const icon = - const checkedIcon = + const handleRemoveSelected = (playlistToRemove) => { + const newSelection = selectedPlaylists.filter( + (p) => p.id !== playlistToRemove.id, + ) + setSelectedPlaylists(newSelection) + onChange(newSelection) + } + + const handleCreateNew = () => { + if (searchText.trim()) { + const newPlaylist = { name: searchText.trim() } + const newSelection = [...selectedPlaylists, newPlaylist] + setSelectedPlaylists(newSelection) + onChange(newSelection) + setSearchText('') + } + } + + const handleKeyDown = (e) => { + if (e.key === 'Enter' && searchText.trim()) { + e.preventDefault() + handleCreateNew() + } + } + + const canCreateNew = Boolean( + searchText.trim() && + !filteredOptions.some( + (option) => + option.name.toLowerCase() === searchText.toLowerCase().trim(), + ) && + !selectedPlaylists.some((p) => p.name === searchText.trim()), + ) return ( - { - const filtered = filter(options, params) +
+ - // Suggest the creation of a new value - if (params.inputValue !== '') { - filtered.push({ - inputValue: params.inputValue, - name: translate('resources.playlist.actions.addNewPlaylist', { - name: params.inputValue, - }), - }) - } + - return filtered - }} - clearOnBlur - handleHomeEndKeys - openOnFocus - selectOnFocus - id="select-playlist-input" - options={options} - getOptionLabel={(option) => { - // Value selected with enter, right from the input - if (typeof option === 'string') { - return option - } - // Add "xxx" option created dynamically - if (option.inputValue) { - return option.inputValue - } - // Regular option - return option.name - }} - renderOption={(option, { selected }) => ( - - - {option.name} - - )} - className={classes.root} - freeSolo - renderInput={(params) => ( - - )} - /> + +
) } SelectPlaylistInput.propTypes = { onChange: PropTypes.func.isRequired, } + +// PropTypes for sub-components +PlaylistSearchField.propTypes = { + searchText: PropTypes.string.isRequired, + onSearchChange: PropTypes.func.isRequired, + onCreateNew: PropTypes.func.isRequired, + onKeyDown: PropTypes.func.isRequired, + canCreateNew: PropTypes.bool.isRequired, +} + +EmptyPlaylistMessage.propTypes = { + searchText: PropTypes.string.isRequired, + canCreateNew: PropTypes.bool.isRequired, +} + +PlaylistListItem.propTypes = { + playlist: PropTypes.object.isRequired, + isSelected: PropTypes.bool.isRequired, + onToggle: PropTypes.func.isRequired, +} + +CreatePlaylistItem.propTypes = { + searchText: PropTypes.string.isRequired, + onCreateNew: PropTypes.func.isRequired, +} + +PlaylistList.propTypes = { + filteredOptions: PropTypes.array.isRequired, + selectedPlaylists: PropTypes.array.isRequired, + onPlaylistToggle: PropTypes.func.isRequired, + searchText: PropTypes.string.isRequired, + canCreateNew: PropTypes.bool.isRequired, + onCreateNew: PropTypes.func.isRequired, +} + +SelectedPlaylistChip.propTypes = { + playlist: PropTypes.object.isRequired, + onRemove: PropTypes.func.isRequired, +} + +SelectedPlaylistsDisplay.propTypes = { + selectedPlaylists: PropTypes.array.isRequired, + onRemoveSelected: PropTypes.func.isRequired, +} diff --git a/ui/src/dialogs/SelectPlaylistInput.test.jsx b/ui/src/dialogs/SelectPlaylistInput.test.jsx index 727126965..93a14b325 100644 --- a/ui/src/dialogs/SelectPlaylistInput.test.jsx +++ b/ui/src/dialogs/SelectPlaylistInput.test.jsx @@ -11,115 +11,483 @@ import { import { SelectPlaylistInput } from './SelectPlaylistInput' import { describe, beforeAll, afterEach, it, expect, vi } from 'vitest' -describe('SelectPlaylistInput', () => { - beforeAll(() => localStorage.setItem('userId', 'admin')) - afterEach(cleanup) - const onChangeHandler = vi.fn() +const mockPlaylists = [ + { id: 'playlist-1', name: 'Rock Classics', ownerId: 'admin' }, + { id: 'playlist-2', name: 'Jazz Collection', ownerId: 'admin' }, + { id: 'playlist-3', name: 'Electronic Beats', ownerId: 'admin' }, + { id: 'playlist-4', name: 'Chill Vibes', ownerId: 'user2' }, // Not writable by admin +] - it('should call the handler with the selections', async () => { - const mockData = [ - { id: 'sample-id1', name: 'sample playlist 1', ownerId: 'admin' }, - { id: 'sample-id2', name: 'sample playlist 2', ownerId: 'admin' }, - ] - const mockIndexedData = { - 'sample-id1': { - id: 'sample-id1', - name: 'sample playlist 1', - ownerId: 'admin', - }, - 'sample-id2': { - id: 'sample-id2', - name: 'sample playlist 2', - ownerId: 'admin', - }, - } +const mockIndexedData = { + 'playlist-1': { id: 'playlist-1', name: 'Rock Classics', ownerId: 'admin' }, + 'playlist-2': { id: 'playlist-2', name: 'Jazz Collection', ownerId: 'admin' }, + 'playlist-3': { + id: 'playlist-3', + name: 'Electronic Beats', + ownerId: 'admin', + }, + 'playlist-4': { id: 'playlist-4', name: 'Chill Vibes', ownerId: 'user2' }, +} - const mockDataProvider = { - getList: vi - .fn() - .mockResolvedValue({ data: mockData, total: mockData.length }), - } +const createTestComponent = ( + mockDataProvider = null, + onChangeMock = vi.fn(), + playlists = mockPlaylists, + indexedData = mockIndexedData, +) => { + const dataProvider = mockDataProvider || { + getList: vi.fn().mockResolvedValue({ + data: playlists, + total: playlists.length, + }), + } - render( - - + - - - , - ) + }, + }} + > + + + , + ) +} - await waitFor(() => { - expect(mockDataProvider.getList).toHaveBeenCalledWith('playlist', { - filter: { smart: false }, - pagination: { page: 1, perPage: -1 }, - sort: { field: 'name', order: 'ASC' }, +describe('SelectPlaylistInput', () => { + beforeAll(() => localStorage.setItem('userId', 'admin')) + afterEach(cleanup) + + describe('Basic Functionality', () => { + it('should render search field and playlist list', async () => { + const onChangeMock = vi.fn() + createTestComponent(null, onChangeMock) + + await waitFor(() => { + expect(screen.getByRole('textbox')).toBeInTheDocument() + expect(screen.getByText('Rock Classics')).toBeInTheDocument() + expect(screen.getByText('Jazz Collection')).toBeInTheDocument() + expect(screen.getByText('Electronic Beats')).toBeInTheDocument() + }) + + // Should not show playlists not owned by admin (not writable) + expect(screen.queryByText('Chill Vibes')).not.toBeInTheDocument() + }) + + it('should filter playlists based on search input', async () => { + const onChangeMock = vi.fn() + createTestComponent(null, onChangeMock) + + await waitFor(() => { + expect(screen.getByText('Rock Classics')).toBeInTheDocument() + }) + + const searchInput = screen.getByRole('textbox') + fireEvent.change(searchInput, { target: { value: 'rock' } }) + + await waitFor(() => { + expect(screen.getByText('Rock Classics')).toBeInTheDocument() + expect(screen.queryByText('Jazz Collection')).not.toBeInTheDocument() + expect(screen.queryByText('Electronic Beats')).not.toBeInTheDocument() }) }) - let textBox = screen.getByRole('textbox') - fireEvent.change(textBox, { target: { value: 'sample' } }) - fireEvent.keyDown(textBox, { key: 'ArrowDown' }) - fireEvent.keyDown(textBox, { key: 'Enter' }) - await waitFor(() => { - expect(onChangeHandler).toHaveBeenCalledWith([ - { id: 'sample-id1', name: 'sample playlist 1', ownerId: 'admin' }, - ]) + it('should handle case-insensitive search', async () => { + const onChangeMock = vi.fn() + createTestComponent(null, onChangeMock) + + await waitFor(() => { + expect(screen.getByText('Jazz Collection')).toBeInTheDocument() + }) + + const searchInput = screen.getByRole('textbox') + fireEvent.change(searchInput, { target: { value: 'JAZZ' } }) + + await waitFor(() => { + expect(screen.getByText('Jazz Collection')).toBeInTheDocument() + expect(screen.queryByText('Rock Classics')).not.toBeInTheDocument() + }) + }) + }) + + describe('Playlist Selection', () => { + it('should select and deselect playlists by clicking', async () => { + const onChangeMock = vi.fn() + createTestComponent(null, onChangeMock) + + await waitFor(() => { + expect(screen.getByText('Rock Classics')).toBeInTheDocument() + }) + + // Select first playlist + const rockPlaylist = screen.getByText('Rock Classics') + fireEvent.click(rockPlaylist) + + await waitFor(() => { + expect(onChangeMock).toHaveBeenCalledWith([ + { id: 'playlist-1', name: 'Rock Classics', ownerId: 'admin' }, + ]) + }) + + // Select second playlist + const jazzPlaylist = screen.getByText('Jazz Collection') + fireEvent.click(jazzPlaylist) + + await waitFor(() => { + expect(onChangeMock).toHaveBeenCalledWith([ + { id: 'playlist-1', name: 'Rock Classics', ownerId: 'admin' }, + { id: 'playlist-2', name: 'Jazz Collection', ownerId: 'admin' }, + ]) + }) + + // Deselect first playlist + fireEvent.click(rockPlaylist) + + await waitFor(() => { + expect(onChangeMock).toHaveBeenCalledWith([ + { id: 'playlist-2', name: 'Jazz Collection', ownerId: 'admin' }, + ]) + }) }) - fireEvent.keyDown(textBox, { key: 'ArrowDown' }) - fireEvent.keyDown(textBox, { key: 'Enter' }) - await waitFor(() => { - expect(onChangeHandler).toHaveBeenCalledWith([ - { id: 'sample-id1', name: 'sample playlist 1', ownerId: 'admin' }, - { id: 'sample-id2', name: 'sample playlist 2', ownerId: 'admin' }, - ]) + it('should show selected playlists as chips', async () => { + const onChangeMock = vi.fn() + createTestComponent(null, onChangeMock) + + await waitFor(() => { + expect(screen.getByText('Rock Classics')).toBeInTheDocument() + }) + + // Select a playlist + const rockPlaylist = screen.getByText('Rock Classics') + fireEvent.click(rockPlaylist) + + await waitFor(() => { + // Should show the selected playlist as a chip + const chips = screen.getAllByText('Rock Classics') + expect(chips.length).toBeGreaterThan(1) // One in list, one in chip + }) }) - fireEvent.change(textBox, { - target: { value: 'new playlist' }, + it('should remove selected playlists via chip remove button', async () => { + const onChangeMock = vi.fn() + createTestComponent(null, onChangeMock) + + await waitFor(() => { + expect(screen.getByText('Rock Classics')).toBeInTheDocument() + }) + + // Select a playlist + const rockPlaylist = screen.getByText('Rock Classics') + fireEvent.click(rockPlaylist) + + await waitFor(() => { + // Should show selected playlist as chip + const chips = screen.getAllByText('Rock Classics') + expect(chips.length).toBeGreaterThan(1) + }) + + // Find and click the remove button (translation key) + const removeButton = screen.getByText( + 'resources.playlist.actions.removeSymbol', + ) + fireEvent.click(removeButton) + + await waitFor(() => { + expect(onChangeMock).toHaveBeenCalledWith([]) + // Should only have one instance (in the list) after removal + const remainingChips = screen.getAllByText('Rock Classics') + expect(remainingChips.length).toBe(1) + }) }) - fireEvent.keyDown(textBox, { key: 'ArrowDown' }) - fireEvent.keyDown(textBox, { key: 'Enter' }) - await waitFor(() => { - expect(onChangeHandler).toHaveBeenCalledWith([ - { id: 'sample-id1', name: 'sample playlist 1', ownerId: 'admin' }, - { id: 'sample-id2', name: 'sample playlist 2', ownerId: 'admin' }, - { name: 'new playlist' }, - ]) + }) + + describe('Create New Playlist', () => { + it('should create new playlist by pressing Enter', async () => { + const onChangeMock = vi.fn() + createTestComponent(null, onChangeMock) + + await waitFor(() => { + expect(screen.getByRole('textbox')).toBeInTheDocument() + }) + + const searchInput = screen.getByRole('textbox') + fireEvent.change(searchInput, { target: { value: 'My New Playlist' } }) + fireEvent.keyDown(searchInput, { key: 'Enter' }) + + await waitFor(() => { + expect(onChangeMock).toHaveBeenCalledWith([{ name: 'My New Playlist' }]) + }) + + // Input should be cleared after creating + expect(searchInput.value).toBe('') }) - fireEvent.change(textBox, { - target: { value: 'another new playlist' }, + it('should create new playlist by clicking add button', async () => { + const onChangeMock = vi.fn() + createTestComponent(null, onChangeMock) + + await waitFor(() => { + expect(screen.getByRole('textbox')).toBeInTheDocument() + }) + + const searchInput = screen.getByRole('textbox') + fireEvent.change(searchInput, { target: { value: 'Another Playlist' } }) + + // Find the add button by the translation key title + const addButton = screen.getByTitle( + 'resources.playlist.actions.addNewPlaylist', + ) + fireEvent.click(addButton) + + await waitFor(() => { + expect(onChangeMock).toHaveBeenCalledWith([ + { name: 'Another Playlist' }, + ]) + }) }) - fireEvent.keyDown(textBox, { key: 'Enter' }) - await waitFor(() => { - expect(onChangeHandler).toHaveBeenCalledWith([ - { id: 'sample-id1', name: 'sample playlist 1', ownerId: 'admin' }, - { id: 'sample-id2', name: 'sample playlist 2', ownerId: 'admin' }, - { name: 'new playlist' }, - { name: 'another new playlist' }, - ]) + + it('should not show create option for existing playlist names', async () => { + const onChangeMock = vi.fn() + createTestComponent(null, onChangeMock) + + await waitFor(() => { + expect(screen.getByRole('textbox')).toBeInTheDocument() + }) + + const searchInput = screen.getByRole('textbox') + fireEvent.change(searchInput, { target: { value: 'Rock Classics' } }) + + await waitFor(() => { + expect( + screen.queryByText('resources.playlist.actions.addNewPlaylist'), + ).not.toBeInTheDocument() + }) + }) + + it('should not create playlist with empty name', async () => { + const onChangeMock = vi.fn() + createTestComponent(null, onChangeMock) + + await waitFor(() => { + expect(screen.getByRole('textbox')).toBeInTheDocument() + }) + + const searchInput = screen.getByRole('textbox') + fireEvent.change(searchInput, { target: { value: ' ' } }) // Only spaces + fireEvent.keyDown(searchInput, { key: 'Enter' }) + + // Should not call onChange + expect(onChangeMock).not.toHaveBeenCalled() + }) + + it('should show create options in appropriate contexts', async () => { + const onChangeMock = vi.fn() + createTestComponent(null, onChangeMock) + + await waitFor(() => { + expect(screen.getByRole('textbox')).toBeInTheDocument() + }) + + const searchInput = screen.getByRole('textbox') + + // When typing a new name, should show create options + fireEvent.change(searchInput, { target: { value: 'My New Playlist' } }) + + await waitFor(() => { + // Should show the add button in the search field + expect( + screen.getByTitle('resources.playlist.actions.addNewPlaylist'), + ).toBeInTheDocument() + // Should also show hint in empty message when no matches + expect( + screen.getByText('resources.playlist.actions.pressEnterToCreate'), + ).toBeInTheDocument() + }) + }) + }) + + describe('Mixed Operations', () => { + it('should handle selecting existing playlists and creating new ones', async () => { + const onChangeMock = vi.fn() + createTestComponent(null, onChangeMock) + + await waitFor(() => { + expect(screen.getByText('Rock Classics')).toBeInTheDocument() + }) + + // Select existing playlist + const rockPlaylist = screen.getByText('Rock Classics') + fireEvent.click(rockPlaylist) + + await waitFor(() => { + expect(onChangeMock).toHaveBeenCalledWith([ + { id: 'playlist-1', name: 'Rock Classics', ownerId: 'admin' }, + ]) + }) + + // Create new playlist + const searchInput = screen.getByRole('textbox') + fireEvent.change(searchInput, { target: { value: 'New Mix' } }) + fireEvent.keyDown(searchInput, { key: 'Enter' }) + + await waitFor(() => { + expect(onChangeMock).toHaveBeenCalledWith([ + { id: 'playlist-1', name: 'Rock Classics', ownerId: 'admin' }, + { name: 'New Mix' }, + ]) + }) + }) + + it('should maintain selections when searching', async () => { + const onChangeMock = vi.fn() + createTestComponent(null, onChangeMock) + + await waitFor(() => { + expect(screen.getByText('Rock Classics')).toBeInTheDocument() + }) + + // Select a playlist + const rockPlaylist = screen.getByText('Rock Classics') + fireEvent.click(rockPlaylist) + + // Filter the list + const searchInput = screen.getByRole('textbox') + fireEvent.change(searchInput, { target: { value: 'jazz' } }) + + await waitFor(() => { + // Should still show selected playlists section + // Rock Classics should still be visible as a selected chip even though filtered out + expect(screen.getByText('Rock Classics')).toBeInTheDocument() // In selected chips + expect(screen.getByText('Jazz Collection')).toBeInTheDocument() + }) + }) + }) + + describe('Empty States', () => { + it('should show empty message when no playlists exist', async () => { + const onChangeMock = vi.fn() + createTestComponent(null, onChangeMock, [], {}) + + await waitFor(() => { + expect( + screen.getByText('resources.playlist.message.noPlaylists'), + ).toBeInTheDocument() + }) + }) + + it('should show "no results" message when search returns no matches', async () => { + const onChangeMock = vi.fn() + createTestComponent(null, onChangeMock) + + await waitFor(() => { + expect(screen.getByRole('textbox')).toBeInTheDocument() + }) + + const searchInput = screen.getByRole('textbox') + fireEvent.change(searchInput, { + target: { value: 'NonExistentPlaylist' }, + }) + + await waitFor(() => { + expect( + screen.getByText('resources.playlist.message.noPlaylistsFound'), + ).toBeInTheDocument() + expect( + screen.getByText('resources.playlist.actions.pressEnterToCreate'), + ).toBeInTheDocument() + }) + }) + }) + + describe('Keyboard Navigation', () => { + it('should not create playlist on Enter if input is empty', async () => { + const onChangeMock = vi.fn() + createTestComponent(null, onChangeMock) + + await waitFor(() => { + expect(screen.getByRole('textbox')).toBeInTheDocument() + }) + + const searchInput = screen.getByRole('textbox') + fireEvent.keyDown(searchInput, { key: 'Enter' }) + + expect(onChangeMock).not.toHaveBeenCalled() + }) + + it('should handle other keys without side effects', async () => { + const onChangeMock = vi.fn() + createTestComponent(null, onChangeMock) + + await waitFor(() => { + expect(screen.getByRole('textbox')).toBeInTheDocument() + }) + + const searchInput = screen.getByRole('textbox') + fireEvent.change(searchInput, { target: { value: 'test' } }) + fireEvent.keyDown(searchInput, { key: 'ArrowDown' }) + fireEvent.keyDown(searchInput, { key: 'Tab' }) + fireEvent.keyDown(searchInput, { key: 'Escape' }) + + // Should not create playlist or trigger onChange + expect(onChangeMock).not.toHaveBeenCalled() + expect(searchInput.value).toBe('test') + }) + }) + + describe('Integration Scenarios', () => { + it('should handle complex workflow: search, select, create, remove', async () => { + const onChangeMock = vi.fn() + createTestComponent(null, onChangeMock) + + await waitFor(() => { + expect(screen.getByText('Rock Classics')).toBeInTheDocument() + }) + + // Search and select existing playlist + const searchInput = screen.getByRole('textbox') + fireEvent.change(searchInput, { target: { value: 'rock' } }) + + const rockPlaylist = screen.getByText('Rock Classics') + fireEvent.click(rockPlaylist) + + // Clear search and create new playlist + fireEvent.change(searchInput, { target: { value: 'My Custom Mix' } }) + fireEvent.keyDown(searchInput, { key: 'Enter' }) + + await waitFor(() => { + expect(onChangeMock).toHaveBeenCalledWith([ + { id: 'playlist-1', name: 'Rock Classics', ownerId: 'admin' }, + { name: 'My Custom Mix' }, + ]) + }) + + // Remove the first selected playlist via chip + const removeButtons = screen.getAllByText( + 'resources.playlist.actions.removeSymbol', + ) + fireEvent.click(removeButtons[0]) + + await waitFor(() => { + expect(onChangeMock).toHaveBeenCalledWith([{ name: 'My Custom Mix' }]) + }) }) }) }) diff --git a/ui/src/i18n/en.json b/ui/src/i18n/en.json index a8b026ddf..52363a350 100644 --- a/ui/src/i18n/en.json +++ b/ui/src/i18n/en.json @@ -198,11 +198,17 @@ "export": "Export", "saveQueue": "Save Queue to Playlist", "makePublic": "Make Public", - "makePrivate": "Make Private" + "makePrivate": "Make Private", + "searchOrCreate": "Search playlists or type to create new...", + "pressEnterToCreate": "Press Enter to create new playlist", + "removeFromSelection": "Remove from selection", + "removeSymbol": "×" }, "message": { "duplicate_song": "Add duplicated songs", - "song_exist": "There are duplicates being added to the playlist. Would you like to add the duplicates or skip them?" + "song_exist": "There are duplicates being added to the playlist. Would you like to add the duplicates or skip them?", + "noPlaylistsFound": "No playlists found", + "noPlaylists": "No playlists available" } }, "radio": {