Skip to content

Conversation

@flooxo
Copy link
Contributor

@flooxo flooxo commented Jan 16, 2024

This PR closes #2461

on

Description

I added a slider for the sound effect volume similar to the implementation of the animation speed. The slider only gets displayed when the setting is enabled

Alternatives

  • It would also have been possible to only increase the volume of the file, so that the user then has to use the windows volume mixer -> imo not that user friendly
  • It would also have been possible to increase the volume of the audio file on the go -> imo not that efficient

Changes

Increase the volume so that it can be increased overall
Changed SoundPlayer to MediaPlayer as the SoundPlayer does not allow to modify the volume

Signed-off-by: Florian Grabmeier <[email protected]>
@flooxo
Copy link
Contributor Author

flooxo commented Jan 16, 2024

as a little feature it would also be possible to change the volume icon depending on the volume, but i didn't know how to look up which icon is what

@github-actions

This comment has been minimized.

@taooceros
Copy link
Member

as a little feature it would also be possible to change the volume icon depending on the volume, but i didn't know how to look up which icon is what

check here https://learn.microsoft.com/en-us/windows/apps/design/style/segoe-fluent-icons-font

but I feel like two icon is a bit duplicate. @onesounds do you want to take a look?

@onesounds
Copy link
Contributor

onesounds commented Jan 17, 2024

as a little feature it would also be possible to change the volume icon depending on the volume, but i didn't know how to look up which icon is what

check here https://learn.microsoft.com/en-us/windows/apps/design/style/segoe-fluent-icons-font

but I feel like two icon is a bit duplicate. @onesounds do you want to take a look?

Yeah, Duplicates are best avoided.

I think simply its ok to remove icon for volume. (because it's sub category of sound effect)

But if it has align issue, you can change sound effect's icon to e7f5 Speakers icon. Volume icon is fine.

Signed-off-by: Florian Grabmeier <[email protected]>
@github-actions

This comment has been minimized.

Signed-off-by: Florian Grabmeier <[email protected]>
@github-actions

This comment has been minimized.

@flooxo
Copy link
Contributor Author

flooxo commented Jan 17, 2024

Oke thanks, unfortunately i don't know how to replace the icon with a blank space so that everything is still aligned correctly. imo i think it looks better if each setting has an icon, because otherwise it would look like the icon is missing

@taooceros
Copy link
Member

taooceros commented Mar 9, 2024

@jjw24 this seems to be a nice little feature to have. Maybe we add it to 1.17.3 as well? 1.18 doesn't seems to be near.

@jjw24
Copy link
Member

jjw24 commented Mar 9, 2024

Keep it for 1.18.0, we have another feature that can be released so once we get couple more we can just go straight to 1.18.0 instead.

@jjw24
Copy link
Member

jjw24 commented Mar 9, 2024

@taooceros is this one approved or still need review?

@jjw24 jjw24 added this to the 1.18.0 milestone Mar 9, 2024
@jjw24 jjw24 added the enhancement New feature or request label Mar 9, 2024
@VictoriousRaptor
Copy link
Contributor

Why the open.wav is also modified?

@flooxo
Copy link
Contributor Author

flooxo commented Mar 10, 2024

I have changed the audio file so that you can make it louder than it is right now. With the media player, you can only select a value between 0 and 1 as the volume.
The current volume corresponds to 0.5 using the new volume slider

@taooceros
Copy link
Member

@flooxo mind fix the conflict?

@jjw24 i think it is a relatively simple change and the code looks good to me.

@github-actions

This comment has been minimized.

@taooceros
Copy link
Member

@flooxo seems like the pipeline fails.

C:\projects\flow-launcher\Flow.Launcher\MainWindow.xaml.cs(43,9): error CS0246: The type or namespace name 'MediaPlayer' could not be found (are you missing a using directive or an assembly reference?) [C:\projects\flow-launcher\Flow.Launcher\Flow.Launcher_l1jpuugg_wpftmp.csproj]

Signed-off-by: Florian Grabmeier <[email protected]>
@github-actions
Copy link

@check-spelling-bot Report

🔴 Please review

See the 📂 files view, the 📜action log, or 📝 job summary for details.

Unrecognized words (34)
ASponsor
clearlogfolder
clocksb
ddd
dlg
dopusrt
firefox
flowlauncher
fullscreen
gamemode
hotkeys
iconsb
Img
imo
installbtn
listbox
mainwindow
mscorlib
msedge
Mvvm
Noresult
positionreset
pythonw
scm
searchplugin
Segoe
totalcmd
Txb
uninstallbtn
updatebtn
windowsb
Wpf
wpftk
ZIndex
To accept these unrecognized words as correct, you could run the following commands

... in a clone of the [email protected]:flooxo/Flow.Launcher.git repository
on the volume branch (ℹ️ how do I use this?):

curl -s -S -L 'https://raw.githubusercontent.com/check-spelling/check-spelling/prerelease/apply.pl' |
perl - 'https://github.com/Flow-Launcher/Flow.Launcher/actions/runs/8264227544/attempts/1'
Warnings (1)

See the 📂 files view, the 📜action log, or 📝 job summary for details.

ℹ️ Warnings Count
ℹ️ non-alpha-in-dictionary 10

See ℹ️ Event descriptions for more information.

If the flagged items are 🤯 false positives

If items relate to a ...

  • binary file (or some other file you wouldn't want to check at all).

    Please add a file path to the excludes.txt file matching the containing file.

    File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

    ^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

  • well-formed pattern.

    If you can write a pattern that would match it,
    try adding it to the patterns.txt file.

    Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

    Note that patterns can't match multiline strings.

Copy link
Contributor

@VictoriousRaptor VictoriousRaptor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@jjw24 jjw24 merged commit 0cb80c4 into Flow-Launcher:dev Mar 31, 2024
@onesounds
Copy link
Contributor

onesounds commented Apr 24, 2024

@flooxo

image

After the update, the issue has been reported that there is no sound, and it is not showing in the app volume control list in windows setting (win11. Users cannot hear the sound when set in flow, nor can they find flow in a Windows mixer.). Is there a part that is presumed to be the cause?

+) I've tested this and that, but there's nothing particularly wrong with this pr. I've guessed that if I've already adjusted the sound through the mixer, it will affect after this patch, but there was no particular issue with the bar I tried. Why...

@onesounds
Copy link
Contributor

@flooxo

image

After the update, the issue has been reported that there is no sound, and it is not showing in the app volume control list in windows setting (win11. Users cannot hear the sound when set in flow, nor can they find flow in a Windows mixer.). Is there a part that is presumed to be the cause?

+) I've tested this and that, but there's nothing particularly wrong with this pr. I've guessed that if I've already adjusted the sound through the mixer, it will affect after this patch, but there was no particular issue with the bar I tried. Why...

This issue occurred because the user deleted the Windows Media Player game. We should consider including separate checks and guidance on future media player presence and version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Open Sound Effect volume

5 participants