Review Request 118272: batterymonitor visual cleanups

[Kai Uwe Broulik]

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


Looks great, thanks!

I was actually thinking of removing this expanding thing altogether and just move this information to a tooltip. Since in Plasma 2 the popup cannot change its size, the expanding looks weird, and also that thing should be wrapped inside a ScrollView.

You can find my work in the broulik/batterymonitorsolidimport branch of plasma-workspace. I didn't push it forward that much as I also migrated it to the Solid PM import which is not yet ready and we're already past Beta.

- Kai Uwe Broulik


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/bde15a26/attachment.html>