[Differential] [Commented On] D3603: Option to show percentage charge in the icon

[mart (Marco Martin)]

mart added a comment.


  F672027: Spectacle.T21326.png 

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D3603

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: mart, #plasma
Cc: broulik, plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas

[Differential] [Commented On] D3603: Option to show percentage charge in the icon

[broulik (Kai Uwe Broulik)]

broulik added a comment.


  You forgot BadgeOverlay.qml

INLINE COMMENTS

> ConfigGeneral.qml:28
> +
> +signal configurationChanged
> +

Unused

> ConfigGeneral.qml:30-31
> +
> +width: childrenRect.width
> +height: childrenRect.height
> +implicitWidth: pageColumn.implicitWidth

Not needed since we have implicitWidth?

> ConfigGeneral.qml:37
> +
> +SystemPalette {
> +id: palette

Unused

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D3603

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: mart, #plasma
Cc: broulik, plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas

[Differential] [Commented On] D3603: Option to show percentage charge in the icon

[mart (Marco Martin)]

mart added a comment.


  eew, sorry, that canvas thing was from an old experiment i did for a possible 
alternative graphics, which i forgot to kill ;)

INLINE COMMENTS

> broulik wrote in main.xml:11
> I think this could even be the default?

not sure it's pretty enough for default (yet?), in the end may give some 
problems (like covering the red x that shows up when the battery is removed, 
that's rare in modern laptops, but still)

> broulik wrote in CompactRepresentation.qml:72
> Can you instead try an OpacityMask / ShaderEffect thing like I did in task 
> manager for the badge so we actually cut out a piece of the icon instead of 
> just overlaying an opaque rectangle?

sure

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D3603

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: mart, #plasma
Cc: broulik, plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas

[Differential] [Commented On] D3603: Option to show percentage charge in the icon

[broulik (Kai Uwe Broulik)]

broulik added a comment.


  Nifty. That I haven't thought of that kind of placement before. I always 
wanted some crappy overlay like we had before.
  
  Can you please check again with opacity mask instead of just overlaying a 
Rectangle. Also, what's up with the Canvas thing?

INLINE COMMENTS

> main.xml:11
> +  If true, the battery will display a little charge percentage 
> label inside.
> +  false
> +

I think this could even be the default?

> CompactRepresentation.qml:72
>  
> -BatteryIcon {
> -id: batteryIcon
> -anchors.horizontalCenter: isConstrained ? undefined 
> : parent.horizontalCenter
> -hasBattery: batteryContainer.hasBattery
> -percent: batteryContainer.percent
> -pluggedIn: batteryContainer.pluggedIn
> -height: isConstrained ? batteryContainer.iconSize : 
> batteryContainer.iconSize - batteryLabel.height
> -width: height
> +Rectangle {
> +anchors {

Can you instead try an OpacityMask / ShaderEffect thing like I did in task 
manager for the badge so we actually cut out a piece of the icon instead of 
just overlaying an opaque rectangle?

> CompactRepresentation.qml:84
>  id: batteryLabel
> -width: parent.width
> -height: visible ? paintedHeight : 0
> +anchors.centerIn: parent
> +height: paintedHeight

for fonts avoid vertically anchoring it, use verticalAlignment instead to 
ensure descenders are properly taken into account

> CompactRepresentationCircle.qml:54
>  property bool hasBattery: view.singleBattery ? 
> view.hasBattery : model["Plugged in"]
> -property int percent: view.singleBattery ? 
> pmSource.data["Battery"]["Percent"] : model["Percent"]
> +property int percent: 72//view.singleBattery ? 
> pmSource.data["Battery"]["Percent"] : model["Percent"]
>  property bool pluggedIn: pmSource.data["AC Adapter"] && 
> pmSource.data["AC Adapter"]["Plugged in"] && (view.singleBattery || model["Is 
> Power Supply"])

;)

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D3603

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: mart, #plasma
Cc: broulik, plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas