fix(ui): prevent disabled Show in Playlist menu item from triggering actions (#4356)

* fix: prevent disabled Show in Playlist menu item from triggering actions

Fixed bug where clicking on the disabled 'Show in Playlist' menu item would unintentionally trigger music playback and replace the queue. The menu item now properly prevents event propagation when disabled and takes no action.

This resolves the issue where users would accidentally start playing music when clicking on the greyed-out menu option. The fix includes:
- Custom onClick handler that stops event propagation for disabled state
- Proper styling to maintain visual disabled state while allowing event handling
- Comprehensive test coverage for the disabled behavior

* style: clean up disabled menu item styling code

Simplified the arrow function for disabled onClick handler and changed inline style from empty object to undefined when not needed. Also added a CSS class for disabled menu items for potential future use.

These changes improve code readability and follow React best practices by using undefined instead of empty objects for conditional styles.
This commit is contained in:
Deluan Quintão
2025-07-17 11:00:12 -04:00
committed by GitHub
parent 3c1e5603d0
commit 445880c006
2 changed files with 47 additions and 9 deletions

View File

@@ -31,6 +31,9 @@ const useStyles = makeStyles({
noWrap: {
whiteSpace: 'nowrap',
},
disabledMenuItem: {
pointerEvents: 'auto',
},
})
const MoreButton = ({ record, onClick, info }) => {
@@ -233,19 +236,29 @@ export const SongContextMenu = ({
open={open}
onClose={handleMainMenuClose}
>
{Object.keys(options).map(
(key) =>
{Object.keys(options).map((key) => {
const showInPlaylistDisabled =
key === 'showInPlaylist' && !playlists.length
return (
options[key].enabled && (
<MenuItem
value={key}
key={key}
onClick={handleItemClick}
disabled={key === 'showInPlaylist' && !playlists.length}
onClick={
showInPlaylistDisabled
? (e) => e.stopPropagation()
: handleItemClick
}
disabled={showInPlaylistDisabled}
style={
showInPlaylistDisabled ? { pointerEvents: 'auto' } : undefined
}
>
{options[key].label}
</MenuItem>
),
)}
)
)
})}
</Menu>
<Menu
anchorEl={playlistAnchorEl}

View File

@@ -10,6 +10,8 @@ vi.mock('../dataProvider', () => ({
vi.mock('react-redux', () => ({ useDispatch: () => vi.fn() }))
const getPlaylistsMock = vi.fn()
vi.mock('react-admin', async (importOriginal) => {
const actual = await importOriginal()
return {
@@ -18,9 +20,7 @@ vi.mock('react-admin', async (importOriginal) => {
window.location.hash = `#${url}`
},
useDataProvider: () => ({
getPlaylists: vi.fn().mockResolvedValue({
data: [{ id: 'pl1', name: 'Pl 1' }],
}),
getPlaylists: getPlaylistsMock,
inspect: vi.fn().mockResolvedValue({
data: { rawTags: {} },
}),
@@ -32,6 +32,9 @@ describe('SongContextMenu', () => {
beforeEach(() => {
vi.clearAllMocks()
window.location.hash = ''
getPlaylistsMock.mockResolvedValue({
data: [{ id: 'pl1', name: 'Pl 1' }],
})
})
it('navigates to playlist when selected', async () => {
@@ -79,4 +82,26 @@ describe('SongContextMenu', () => {
expect(mockOnClick).not.toHaveBeenCalled()
})
it('does nothing when "Show in Playlist" is disabled', async () => {
getPlaylistsMock.mockResolvedValue({ data: [] })
const mockOnClick = vi.fn()
render(
<TestContext>
<div onClick={mockOnClick}>
<SongContextMenu record={{ id: 'song1', size: 1 }} resource="song" />
</div>
</TestContext>,
)
fireEvent.click(screen.getAllByRole('button')[1])
await waitFor(() =>
screen.getByText(/resources\.song\.actions\.showInPlaylist/),
)
fireEvent.click(
screen.getByText(/resources\.song\.actions\.showInPlaylist/),
)
expect(mockOnClick).not.toHaveBeenCalled()
})
})