Review Request 112721: Volume step should not be affected by wheelScrollLines

[Eike Hein]

> On Sept. 13, 2013, 10:46 p.m., Eike Hein wrote:
> > (Also, just FWIW, not that the code matters given the above, but: You're caching a global setting and not handling the case that it can be changed at runtime, so the patch would have needed work anyway, even if its underlying assumption had been correct -- which as mentioned it isn't, the 3 is just a coincidence.)
> 
> Albert Vaca Cintora wrote:
>     I already discarded the patch but, about this point, I think that lots of kde applications that read global settings have the same 'problem'. Actually for some of these settings there is already a warning saying that the running apps should be restarted, and I think we can live with that.
> 
> Eike Hein wrote:
>     I'm afraid I don't agree with that. Other things being broken isn't a good reason to write more broken things.
>     
>     In most cases, changes to global settings can be adapted to at runtime by listening to the appropriate signals from KGlobalSettings (or Qt in KF5, since we've now eliminated KGlobalSettings and upstreamed its notification signals), and that's true here as well: KGlobalSettings::settingsChanged() is emitted with SETTINGS_QT when the wheel lines preference is changed, and the new value is subsequently available via QApplication:wheelScrollLines() (i.e. opening the kdeglobals config file was also unnecessary).
>     
>     If you find cases where this isn't done correctly, please fix them. If there's a reason they haven't been fixed, let's fix the reason. We shouldn't "live with that".

(Note that of course we have review exactly so we can help each other catch these sorts of things, so the process worked just fine and no harm was done. :)


- Eike


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112721/#review40005
-----------------------------------------------------------


On Sept. 13, 2013, 10:51 p.m., Albert Vaca Cintora wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/112721/
> -----------------------------------------------------------
> 
> (Updated Sept. 13, 2013, 10:51 p.m.)
> 
> 
> Review request for KDE Multimedia.
> 
> 
> Description
> -------
> 
> I noticed that changing the volume by scrolling over the KMix icon changed it in very big steps. I also realized that killing KDED made the step smaller and 'nicer' so I realized it might be a bug. I found that the scroll event was being called as many times as set in the setting "wheelScrollLines" ("Mouse wheel scrolls by [X] lines" in system settings). It makes sense when scrolling on documents, but not here, so I'm scaling the steps by that value so it always change in steps the same (smaller) size.
> 
> 
> Diffs
> -----
> 
>   gui/kmixdockwidget.h 0109086 
>   gui/kmixdockwidget.cpp e84338e 
> 
> Diff: http://git.reviewboard.kde.org/r/112721/diff/
> 
> 
> Testing
> -------
> 
> Manual testing.
> 
> 
> Thanks,
> 
> Albert Vaca Cintora
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-multimedia/attachments/20130913/2251dd5a/attachment.htm>
-------------- next part --------------
_______________________________________________
kde-multimedia mailing list
kde-multimedia at kde.org
https://mail.kde.org/mailman/listinfo/kde-multimedia