Review Request: battery: change brightness on mouse wheel

John Layt johnlayt at googlemail.com
Sun Aug 1 16:05:41 CEST 2010



> On 2010-08-01 01:53:17, Aaron Seigo wrote:
> > you don't need to propagate wheel events (or most other events, for that matter, unless there is an underlying implementation that also needs to be called). i don't know why it would be crashing with looking at the backtrace.
> > 
> > that said, however, i don't see the connection between the battery icon and the brightness of the screen. screen brightness affects power usage, sure, but it's a little like scrolling on the app menu and having it switch between windows :)
> > 
> > as such, i don't think this is something that should go into svn.

Drive-by bike-shedding :-)

Scrolling over an indicator icon suggests changing the primary function of that indicator, so scrolling over the battery suggests to me changing the Power Profile.  Progressively switching from one profile to the next as the user scrolls would not be a good thing, so scrolling would just show a pop-up with a list of all the profiles with a highlight around the selection that the scrolling would move, then once the scrolling stops after a slight delay the profile would switch.

For easily changing the brightness I would suggest a new Screen Brightness plasmoid in kdebase that works like the Volume plasmoid.  It would display the fairly standard sunshine icon that you can scroll over, with the sun's ray changing in size accordingly and clicking on it would pop-up a slider.  Bonus points for integrating the NVDimmer screen brightness functionality, although that probably needs to be done further down the stack in Solid :-)

It always seemed a little strange to me to have the screen brightness slider and sleep and hibernate buttons in the battery/power management plasmoid pop-up, it's not really obvious to users and trying to use any of them was always 2-3 clicks away when 1-2 would be better.  The battery should be purely about power profile management.  With a separate brightness plasmoid, and the lock plasmoid now also providing the sleep/hibernate buttons, the battery pop-up on click could be simplified to just choosing the Power Profile using the same pop-up as the scrolling action suggested above.  I think this would give the indicators a more consistent look-and-feel, and work better for the mobile/netbook containments.  Tying it all together would be an 'Add Panel' profile for 'Laptop' that includes the battery and brightness plasmoids and the lock plasmoid with the sleep/hibernate buttons enabled, then on plasma first run try make an intelligent choice between using the normal default Desktop panel profile or the Laptop profile.  Sound elegant? :-)


- John


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


On 2010-08-01 01:01:33, Rafa? Mi?ecki wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/4810/
> -----------------------------------------------------------
> 
> (Updated 2010-08-01 01:01:33)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> This implements feature requested in bug 230888 . Scrolling over battery plasmoid changes brightness.
> 
> In (uncommon) case of multiple brightness devices it affects the first registered device. We may want to make it configurable in the future.
> 
> 
> This addresses bug 230888.
>     https://bugs.kde.org/show_bug.cgi?id=230888
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdebase/workspace/plasma/generic/applets/battery/battery.h 1157714 
>   /trunk/KDE/kdebase/workspace/plasma/generic/applets/battery/battery.cpp 1157714 
> 
> Diff: http://reviewboard.kde.org/r/4810/diff
> 
> 
> Testing
> -------
> 
> It works fine for my notebook, doesn't crash on machine without brightness device.
> 
> The part I am not sure about is:
> Applet::wheelEvent(e);
> I though we need to propagate events so tried to use this call. However using it causes crash. Help?
> 
> 
> Thanks,
> 
> Rafa?
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/plasma-devel/attachments/20100801/80e3bd94/attachment.htm 


More information about the Plasma-devel mailing list