Extend configuration knobs for visual feedback by adding separate ones
for output volume, input volume and mute state.
BUG: 363983
FIXED-IN: 5.19.0
No Linters Available |
No Unit Test Coverage |
Buildable 24233 | |
Build 24251: arc lint + arc unit |
Not sure about the actual wording and whether we would want all 3 of them.
Probably would use "input volume", "output volume" but i tried to match the existing "aural feedback" string.
Also i actually tried to implement the various qml functions a single one taking a QML enum but failed to do so. If that or a string based version is preferred, just tell me (how :) ).
applet/contents/ui/main.qml | ||
---|---|---|
198 | mutecOsd? :-) |
This works great!
In terms of the UI, it feels like we need a bit more logical separation between the groups now. I would adjust things (in this same patch) to be like this:
Play audio feedback for changes to: [x] Audio volume Show visual feedback for changes to: [x] Audio volume [x] Microphone sensitivity [x] Mute state [x] default output
You can add the spacing between groups with an item like this:
Item { Kirigami.FormData.isSection: true }
This looks great to me!
I have to admit I was skeptical when I first saw the patch, but I think making the options very granular like this actually helps to explain just what they'll do, and also disambiguates the visual vs audible feedback settings too.
Yeah, was unsure too if it's a good idea but now with your sanitized wording i'd prefer it that way :)
But if i should go for a single OSD on/off i wouldn't mind either.
Code looks good, bar one minor issue—instead of using global showOsdX functions, it would be more idiomatic to declare these functions on the OSD object itself. Other than that, this looks good.
applet/contents/ui/main.qml | ||
---|---|---|
179 ↗ | (On Diff #78495) | These functions would probably be better declared on the OSD object like so: VolumeOSD { id: osd function showVolume(text) { if (!Plasmoid.configuration.volumeOsd) return show(text) } } |
Cool thanks!
As this is my first contribution via phabricator, can you give me a short advice on how to proceed?
Do i "arc land" it myself? To master?
IIRC you can only do that if you get an accepted application for becoming a "KDE Developer", otherwise you'll need a KDE Developer to land it for you, which will most likely happen anyway.
Since there's two pending reviews, you probably want to wait on them first.
However, you need to have a developer account in order to land your patches, and you most likely don't have one yet considering that this is your first contribution. When someone's landing your patch for you, they'll ask for authorship information (name, email) if it's missing and then land it for you.
EDIT: Speaking of authorship information, you should probably add yourself to the copyright headers. It's not for "credit me" purposes, but for keeping track of who's added copyrightable stuff to a file without needing git.