Review Request 118272: batterymonitor visual cleanups

[Sebastian Kügler]

> On May 22, 2014, 11:08 p.m., Mark Gaiser wrote:
> > applets/batterymonitor/contents/ui/PopupDialog.qml, lines 78-81
> > <https://git.reviewboard.kde.org/r/118272/diff/1/?file=274300#file274300line78>
> >
> >     I don't know this code so i'm not going to comment on how it should or should not work. But this early return effectively makes this function useless.
> >     
> >     I think you meant to remove this function?

Ah, right, thanks.

At first, I wanted to, but it's used for keyboard navigation inside the popup. I'll re-add the function in BatteryItem and make it a no-op (since it doesn't need to do anything anymore in the battery items, but are useful in the brightness ones), so the code doesn't struggle over it. 


- Sebastian


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118272/#review58335
-----------------------------------------------------------


On May 22, 2014, 10:36 p.m., Sebastian Kügler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118272/
> -----------------------------------------------------------
> 
> (Updated May 22, 2014, 10:36 p.m.)
> 
> 
> Review request for Plasma and Kai Uwe Broulik.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> -------
> 
> Clean up Battery Monitor
> 
> This patch simplifies the battery's ui, and brings spacing in line with
> Plasma 5 standards.
> 
> In detail:
> 
> - removing the selection item on batteries: it's already visible by the
>   text shown that the battery is selected. It also doesn't need the
>   selection semantics across the ui, because -- for what?
> - remove separator lines, in Plasma 5, we use spacing for this case
> - align extra battery info to the slider, improves logical grouping
> - Improve HighDPI by removing hard-coded layout hints, use units.gridUnit
>   throughout
> - Fix batteryitem's status: it would show the wrong icon and time label,
>   because AC Adapter can be empty. Checking charging is semantically
>   correct here, since it uses the charge state, not the adapter state.
> - remove a bunch of SVGs that were used internally to get margins -- use
>   gridUnit for layout internal margins instead
> - fix slider's right alignment, this would jump based on the percentage
>   label's width, which varies per item. We know the rough length of the
>   percentage label from the context, and can align the labels to that.
> 
> Code is also in kde-workspace[sebas/batterycleanup].
> 
> 
> Diffs
> -----
> 
>   applets/batterymonitor/contents/ui/BatteryItem.qml 431aa9ad40435e331cfff494cbe6f9f1e409ce35 
>   applets/batterymonitor/contents/ui/BrightnessItem.qml a91ccdeebd7136becfb9d78bfade0e6189f6221a 
>   applets/batterymonitor/contents/ui/PopupDialog.qml 0444199a7d34deac0472bb50961a34b8717c9f5a 
>   applets/batterymonitor/contents/ui/PowerManagementItem.qml 530fce30e7e81ebbe6e394d7c8927806e3b9d08b 
> 
> Diff: https://git.reviewboard.kde.org/r/118272/diff/
> 
> 
> Testing
> -------
> 
> Tested with plasmoidviewer and in the shell, plugging ac in and out, tried different combinations of values.
> 
> 
> File Attachments
> ----------------
> 
> before (collapsed)
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/05/22/abc1d750-6eb7-4728-a5f2-e9f8566b5b44__battery-before-collapsed.png
> before (expanded)
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/05/22/2a0b9ad3-03e8-4f07-a158-ceb0d3d0e23c__battery-before-expanded.png
> after (collapsed)
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/05/22/4439093c-345c-44af-afa1-a05cb0973990__battery-after-collapsed.png
> after (expanded)
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/05/22/b18cf61f-0ff1-4e91-93c2-ebd6b1dab38d__battery-after-expanded.png
> 
> 
> Thanks,
> 
> Sebastian Kügler
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20140523/53ab91dc/attachment.html>