feat: add mouse scroll to incease/decrease volume

[Alimedhat000]

Adds mouse scroll support to increase/decrease volume.

Resolves #446.

In app.toml:

enable_mouse_scroll_volume = true # enable mouse scrolling to change volume
volume_scroll_step = 5  # percent to change per scroll (default)

[aome510]

there is already VolumeChange command, why did you add new ones?

[Alimedhat000]

oh my bad! i will use VolumeChange instead

[Copilot]

Pull request overview

This PR adds mouse scroll support to control playback volume in response to issue #446, which requested the ability to bind mouse scroll events to volume control. The implementation introduces two new commands (VolumeUp and VolumeDown), adds corresponding player requests, and implements mouse scroll event handling with a configurable volume step.

Changes:

• Added VolumeUp and VolumeDown commands with default key bindings to "m s u" and "m s d"
• Implemented mouse scroll event handlers (ScrollUp/ScrollDown) that trigger volume changes
• Added volume_scroll_step configuration option (default: 5%) to control the volume increment/decrement size

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
spotify_player/src/event/mod.rs	Added mouse scroll event handlers and VolumeUp/VolumeDown command handlers
spotify_player/src/config/mod.rs	Added volume_scroll_step configuration field with default value of 5
spotify_player/src/config/keymap.rs	Added default keymaps for VolumeUp ("m s u") and VolumeDown ("m s d")
spotify_player/src/command.rs	Added VolumeUp and VolumeDown command enum variants with descriptions
spotify_player/src/client/request.rs	Added VolumeUp and VolumeDown request enum variants
spotify_player/src/client/mod.rs	Implemented VolumeUp and VolumeDown request handlers with volume bounds checking

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The mouse scroll handler hardcodes the key sequences "m s u" and the VolumeUp command. This implementation checks if "m s u" exists in the keymap config but then always sends VolumeUp regardless of what command is actually mapped to that sequence. Instead, the handler should retrieve the command from the keymap using find_command_or_action_from_key_sequence and execute whatever command is mapped to "m s u", similar to how handle_key_event works (lines 149-157). This would allow users to customize what mouse scroll does, not just limit it to volume control.

Suggested change

	if keymap_config
	.find_command_or_action_from_key_sequence(&key_sequence)
	.is_some()
	{
	let _ = client_pub.try_send(ClientRequest::Player(PlayerRequest::VolumeUp));
	}
	}
	crossterm::event::MouseEventKind::ScrollDown => {
	let key_sequence: KeySequence = "m s d".into();
	let keymap_config = &config::get_config().keymap_config;
	if keymap_config
	.find_command_or_action_from_key_sequence(&key_sequence)
	.is_some()
	{
	let _ = client_pub.try_send(ClientRequest::Player(PlayerRequest::VolumeDown));
	if let Some(command_or_action) =
	keymap_config.find_command_or_action_from_key_sequence(&key_sequence)
	{
	// Execute whatever command or action is mapped to "m s u"
	handle_command_or_action(command_or_action, client_pub, state)?;
	}
	}
	crossterm::event::MouseEventKind::ScrollDown => {
	let key_sequence: KeySequence = "m s d".into();
	let keymap_config = &config::get_config().keymap_config;
	if let Some(command_or_action) =
	keymap_config.find_command_or_action_from_key_sequence(&key_sequence)
	{
	// Execute whatever command or action is mapped to "m s d"
	handle_command_or_action(command_or_action, client_pub, state)?;

[Copilot]

Same issue as ScrollUp handler: this hardcodes the key sequence "m s d" and always sends VolumeDown. The handler should retrieve and execute the actual command mapped to "m s d" from the keymap config using find_command_or_action_from_key_sequence, allowing users to customize mouse scroll behavior beyond just volume control.

Suggested change

	if keymap_config
	.find_command_or_action_from_key_sequence(&key_sequence)
	.is_some()
	{
	let _ = client_pub.try_send(ClientRequest::Player(PlayerRequest::VolumeUp));
	}
	}
	crossterm::event::MouseEventKind::ScrollDown => {
	let key_sequence: KeySequence = "m s d".into();
	let keymap_config = &config::get_config().keymap_config;
	if keymap_config
	.find_command_or_action_from_key_sequence(&key_sequence)
	.is_some()
	{
	let _ = client_pub.try_send(ClientRequest::Player(PlayerRequest::VolumeDown));
	if let Some(command_or_action) =
	keymap_config.find_command_or_action_from_key_sequence(&key_sequence)
	{
	// Execute the command or action mapped to "m s u" so users can customize
	// scroll-up behavior (e.g. volume up or something else).
	command::handle_command_or_action(command_or_action, &state, client_pub)?;
	}
	}
	crossterm::event::MouseEventKind::ScrollDown => {
	let key_sequence: KeySequence = "m s d".into();
	let keymap_config = &config::get_config().keymap_config;
	if let Some(command_or_action) =
	keymap_config.find_command_or_action_from_key_sequence(&key_sequence)
	{
	// Execute the command or action mapped to "m s d" so users can customize
	// scroll-down behavior (e.g. volume down or something else).
	command::handle_command_or_action(command_or_action, &state, client_pub)?;

[Copilot]

Error handling is silently discarded with the underscore pattern. This is inconsistent with the rest of the function which uses ? to propagate errors (see line 103-105). If the VolumeUp request fails to send, the user should be notified. Consider using the same error handling pattern as the SeekTrack request below.

[Copilot]

Error handling is silently discarded with the underscore pattern. This is inconsistent with the rest of the function which uses ? to propagate errors (see line 103-105). If the VolumeDown request fails to send, the user should be notified. Consider using the same error handling pattern as the SeekTrack request below.

[Copilot]

The PR description states users can configure keymaps in keymap.toml to bind VolumeUp/VolumeDown commands to key sequences "m s u" and "m s d". However, the mouse scroll implementation hardcodes these key sequences and always executes VolumeUp/VolumeDown regardless of what users actually configure. This creates a discrepancy between what the PR description promises (configurability) and what the code delivers (hardcoded behavior). The implementation should honor user-configured keymaps by retrieving and executing the mapped command, not bypassing the keymap system.

[Alimedhat000]

would changing it to use a bool like enable_mouse_scroll_volume instead of the keymap option be better?
cause it's a scrolling option why would some one remap that?

[Alimedhat000]

one problem i faced is that i had to reuse some of the of VolumeChange logic because it's not accessible from the mouse handler.

A potential fix would be to add a small helper function that the mouse handler could use.
i'm happy to implement this but i'd wait for your approval.