subject:"D26806\: \[Applets\/Power Manager\] Update layout based on T10470"

D26806: [Applets/Power Manager] Update layout based on T10470

2020-03-04 Thread George Vogiatzis

gvgeo abandoned this revision.
gvgeo added a comment.


  Unfortunately I won't be able to continue with this patch about battery 
applet.
  Feel free to use/modify if you want.

REPOSITORY
  R120 Plasma Workspace

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

To: gvgeo, #plasma, #vdg, davidedmundson, manueljlin
Cc: broulik, ngraham, manueljlin, davidedmundson, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, 
ragreen, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, ahiemstra, mart

D26806: [Applets/Power Manager] Update layout based on T10470

2020-02-26 Thread George Vogiatzis

gvgeo added a comment.


  @davidedmundson ping

REPOSITORY
  R120 Plasma Workspace

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

To: gvgeo, #plasma, #vdg, davidedmundson, manueljlin
Cc: broulik, ngraham, manueljlin, davidedmundson, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, 
ragreen, ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, ahiemstra, mart

D26806: [Applets/Power Manager] Update layout based on T10470

2020-02-18 Thread Nathaniel Graham

ngraham added a comment.


  We're waiting for @davidedmundson to re-review it.

REPOSITORY
  R120 Plasma Workspace

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

To: gvgeo, #plasma, #vdg, davidedmundson, manueljlin
Cc: broulik, ngraham, manueljlin, davidedmundson, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, 
ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, ahiemstra, mart

D26806: [Applets/Power Manager] Update layout based on T10470

2020-02-18 Thread George Vogiatzis

gvgeo added a comment.


  Is there any problem with this patch?

REPOSITORY
  R120 Plasma Workspace

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

To: gvgeo, #plasma, #vdg, davidedmundson, manueljlin
Cc: broulik, ngraham, manueljlin, davidedmundson, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, 
ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, ahiemstra, mart

D26806: [Applets/Power Manager] Update layout based on T10470

2020-02-14 Thread George Vogiatzis

gvgeo added a comment.


  No need to worry. The flip of the toolbar is a small proportion of the patch, 
most of the time I try to solve other problems. And in the end of the line was 
a team decision.

REPOSITORY
  R120 Plasma Workspace

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

To: gvgeo, #plasma, #vdg, davidedmundson, manueljlin
Cc: broulik, ngraham, manueljlin, davidedmundson, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, 
ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, ahiemstra, mart

D26806: [Applets/Power Manager] Update layout based on T10470

2020-02-09 Thread Manuel Jesús de la Fuente

manueljlin added a comment.


  Thanks, and sorry for wasting your time :x

REPOSITORY
  R120 Plasma Workspace

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

To: gvgeo, #plasma, #vdg, davidedmundson, manueljlin
Cc: broulik, ngraham, manueljlin, davidedmundson, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, 
ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, ahiemstra, mart

D26806: [Applets/Power Manager] Update layout based on T10470

2020-02-09 Thread George Vogiatzis

gvgeo updated this revision to Diff 75315.
gvgeo added a comment.


  Up it goes again

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26806?vs=75297=75315

BRANCH
  b4 (branched from master)

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

AFFECTED FILES
  applets/batterymonitor/package/contents/ui/BatteryItem.qml
  applets/batterymonitor/package/contents/ui/BrightnessItem.qml
  applets/batterymonitor/package/contents/ui/InhibitionHint.qml
  applets/batterymonitor/package/contents/ui/PopupDialog.qml
  applets/batterymonitor/package/contents/ui/PowerManagementItem.qml
  applets/batterymonitor/package/contents/ui/batterymonitor.qml
  applets/batterymonitor/package/contents/ui/logic.js

To: gvgeo, #plasma, #vdg, davidedmundson, manueljlin
Cc: broulik, ngraham, manueljlin, davidedmundson, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, 
ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, ahiemstra, mart

D26806: [Applets/Power Manager] Update layout based on T10470

2020-02-09 Thread George Vogiatzis

gvgeo updated this revision to Diff 75297.
gvgeo added a comment.


  Small improvement of Power Managment checkbox's tooltip code.

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26806?vs=74347=75297

BRANCH
  b3 (branched from master)

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

AFFECTED FILES
  applets/batterymonitor/package/contents/ui/BatteryItem.qml
  applets/batterymonitor/package/contents/ui/BrightnessItem.qml
  applets/batterymonitor/package/contents/ui/InhibitionHint.qml
  applets/batterymonitor/package/contents/ui/PopupDialog.qml
  applets/batterymonitor/package/contents/ui/PowerManagementItem.qml
  applets/batterymonitor/package/contents/ui/batterymonitor.qml
  applets/batterymonitor/package/contents/ui/logic.js

To: gvgeo, #plasma, #vdg, davidedmundson, manueljlin
Cc: broulik, ngraham, manueljlin, davidedmundson, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, 
ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, ahiemstra, mart

D26806: [Applets/Power Manager] Update layout based on T10470

2020-02-05 Thread George Vogiatzis

gvgeo added a comment.


  I 'll wait for response about the alignment before I update test plan too.

REPOSITORY
  R120 Plasma Workspace

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

To: gvgeo, #plasma, #vdg, davidedmundson, manueljlin
Cc: broulik, ngraham, manueljlin, davidedmundson, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, 
ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, ahiemstra, mart

D26806: [Applets/Power Manager] Update layout based on T10470

2020-01-27 Thread Manuel Jesús de la Fuente

manueljlin added a comment.


  Might be better to be aligned to the left since it looks similar now to the 
battery percentage, but aside from that it's great! Also, sorry for replying 
late :P

REPOSITORY
  R120 Plasma Workspace

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

To: gvgeo, #plasma, #vdg, davidedmundson, manueljlin
Cc: broulik, ngraham, manueljlin, davidedmundson, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, ahiemstra, mart

D26806: [Applets/Power Manager] Update layout based on T10470

2020-01-24 Thread George Vogiatzis

gvgeo updated this revision to Diff 74347.
gvgeo added a comment.


  Need specific width for columns in FLow. Changed maximumWidth to 
preferredWidth.

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26806?vs=74322=74347

BRANCH
  b3 (branched from master)

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

AFFECTED FILES
  applets/batterymonitor/package/contents/ui/BatteryItem.qml
  applets/batterymonitor/package/contents/ui/BrightnessItem.qml
  applets/batterymonitor/package/contents/ui/InhibitionHint.qml
  applets/batterymonitor/package/contents/ui/PopupDialog.qml
  applets/batterymonitor/package/contents/ui/PowerManagementItem.qml
  applets/batterymonitor/package/contents/ui/batterymonitor.qml
  applets/batterymonitor/package/contents/ui/logic.js

To: gvgeo, #plasma, #vdg, davidedmundson, manueljlin
Cc: broulik, ngraham, manueljlin, davidedmundson, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, ahiemstra, mart

D26806: [Applets/Power Manager] Update layout based on T10470

2020-01-24 Thread David Edmundson

davidedmundson added inline comments.

INLINE COMMENTS

> gvgeo wrote in BatteryItem.qml:112
> This does not react to preferredWidth, minimumWidth or maximumWidth. Is it 
> really managed by Layout?

It's not Flow isn't a layout.

REPOSITORY
  R120 Plasma Workspace

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

To: gvgeo, #plasma, #vdg, davidedmundson, manueljlin
Cc: broulik, ngraham, manueljlin, davidedmundson, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, ahiemstra, mart

D26806: [Applets/Power Manager] Update layout based on T10470

2020-01-24 Thread George Vogiatzis

gvgeo marked 5 inline comments as done.
gvgeo added a comment.


  After some effort, I reached in these conclusions:
  1.Is not that big of a problem to specify width in a layout. The big problem 
is specifying preferredWidth and height in the same item. Breaks the 
dimensions, this is what got me confused.
  2.Item set as property, does not get managed by layout.
  
  There are still some width set in places. That do not respond to 
preferredWidth, minimumWidth or maximumWidth.

INLINE COMMENTS

> BatteryItem.qml:112
> +height: implicitHeight
> +width: modelData.label ? detailsLayout.leftColumnWidth : 
> detailsLayout.rightColumnWidth
> +wrapMode: Text.NoWrap

This does not react to preferredWidth, minimumWidth or maximumWidth. Is it 
really managed by Layout?

REPOSITORY
  R120 Plasma Workspace

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

To: gvgeo, #plasma, #vdg, davidedmundson, manueljlin
Cc: broulik, ngraham, manueljlin, davidedmundson, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, ahiemstra, mart

D26806: [Applets/Power Manager] Update layout based on T10470

2020-01-24 Thread George Vogiatzis

gvgeo updated this revision to Diff 74322.
gvgeo added a comment.


  Remove as much as I could width and height settings.
  Removed loader. Moved instead flow in place.

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26806?vs=74314=74322

BRANCH
  b3 (branched from master)

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

AFFECTED FILES
  applets/batterymonitor/package/contents/ui/BatteryItem.qml
  applets/batterymonitor/package/contents/ui/BrightnessItem.qml
  applets/batterymonitor/package/contents/ui/InhibitionHint.qml
  applets/batterymonitor/package/contents/ui/PopupDialog.qml
  applets/batterymonitor/package/contents/ui/PowerManagementItem.qml
  applets/batterymonitor/package/contents/ui/batterymonitor.qml
  applets/batterymonitor/package/contents/ui/logic.js

To: gvgeo, #plasma, #vdg, davidedmundson, manueljlin
Cc: broulik, ngraham, manueljlin, davidedmundson, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, ahiemstra, mart

D26806: [Applets/Power Manager] Update layout based on T10470

2020-01-24 Thread David Edmundson

davidedmundson added inline comments.

INLINE COMMENTS

> BatteryItem.qml:48
> +spacing: units.smallSpacing
> +width: leftColumnWidth + spacing + rightColumnWidth
>  

width used inside a layout

> BatteryItem.qml:57
>  height: implicitHeight
> +width: modelData.label ? detailsLayout.leftColumnWidth : 
> detailsLayout.rightColumnWidth
> +wrapMode: Text.NoWrap

width/height used inside a layout

> BatteryItem.qml:83
> +id: batteryIcon
> +width: units.iconSizes.smallMedium
> +height: width

here

> PopupDialog.qml:147
> +id: batteryList
> +height: childrenRect.height
> +implicitHeight: height

and here

> PowerManagementItem.qml:34
> +MouseArea {
> +width: childrenRect.width
> +height: childrenRect.height

and here

and you shouldn't need to be using chidrenRect.width 
when your child has

  anchors.fill: parent

that's a loop

REPOSITORY
  R120 Plasma Workspace

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

To: gvgeo, #plasma, #vdg, davidedmundson, manueljlin
Cc: broulik, ngraham, manueljlin, davidedmundson, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, ahiemstra, mart

D26806: [Applets/Power Manager] Update layout based on T10470

2020-01-24 Thread George Vogiatzis

gvgeo updated this revision to Diff 74314.
gvgeo added a comment.


  Don't show suppression messages if PM is disabled.

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26806?vs=74262=74314

BRANCH
  b3 (branched from master)

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

AFFECTED FILES
  applets/batterymonitor/package/contents/ui/BatteryItem.qml
  applets/batterymonitor/package/contents/ui/BrightnessItem.qml
  applets/batterymonitor/package/contents/ui/InhibitionHint.qml
  applets/batterymonitor/package/contents/ui/PopupDialog.qml
  applets/batterymonitor/package/contents/ui/PowerManagementItem.qml
  applets/batterymonitor/package/contents/ui/batterymonitor.qml
  applets/batterymonitor/package/contents/ui/logic.js

To: gvgeo, #plasma, #vdg, davidedmundson, manueljlin
Cc: broulik, ngraham, manueljlin, davidedmundson, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, ahiemstra, mart

D26806: [Applets/Power Manager] Update layout based on T10470

2020-01-23 Thread Nathaniel Graham

ngraham added a comment.


  Thanks!

REPOSITORY
  R120 Plasma Workspace

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

To: gvgeo, #plasma, #vdg, davidedmundson
Cc: broulik, ngraham, manueljlin, davidedmundson, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, ahiemstra, mart

D26806: [Applets/Power Manager] Update layout based on T10470

2020-01-23 Thread Nathaniel Graham

ngraham added a reviewer: manueljlin.
ngraham added a comment.


  Looking pretty good to me, UI-wise.

REPOSITORY
  R120 Plasma Workspace

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

To: gvgeo, #plasma, #vdg, davidedmundson, manueljlin
Cc: broulik, ngraham, manueljlin, davidedmundson, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, ahiemstra, mart

D26806: [Applets/Power Manager] Update layout based on T10470

2020-01-23 Thread George Vogiatzis

gvgeo updated this revision to Diff 74262.
gvgeo added a comment.


  Fix inhibition icon size.

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26806?vs=74228=74262

BRANCH
  b3 (branched from master)

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

AFFECTED FILES
  applets/batterymonitor/package/contents/ui/BatteryItem.qml
  applets/batterymonitor/package/contents/ui/BrightnessItem.qml
  applets/batterymonitor/package/contents/ui/InhibitionHint.qml
  applets/batterymonitor/package/contents/ui/PopupDialog.qml
  applets/batterymonitor/package/contents/ui/PowerManagementItem.qml
  applets/batterymonitor/package/contents/ui/batterymonitor.qml
  applets/batterymonitor/package/contents/ui/logic.js

To: gvgeo, #plasma, #vdg, davidedmundson
Cc: broulik, ngraham, manueljlin, davidedmundson, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, ahiemstra, mart

D26806: [Applets/Power Manager] Update layout based on T10470

2020-01-23 Thread Nathaniel Graham

ngraham added inline comments.

INLINE COMMENTS

> PopupDialog.qml:76
> +visible: inhibitions.length > 0
> +iconSource: inhibitions.length > 0 ? inhibitions[0].Icon 
> || "" : ""
> +text: {

The icon is a bit small: F7923036: Screenshot_20200123_093232.png 


REPOSITORY
  R120 Plasma Workspace

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

To: gvgeo, #plasma, #vdg, davidedmundson
Cc: broulik, ngraham, manueljlin, davidedmundson, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, ahiemstra, mart

D26806: [Applets/Power Manager] Update layout based on T10470

2020-01-23 Thread George Vogiatzis

gvgeo marked 3 inline comments as done.
gvgeo added a comment.


  I marked 3 comments as done that had forgotten, as I removed the old tooltip 
code.
  If there is something else, please be more specific. I can not see a better 
way to address any of your comments.

INLINE COMMENTS

> BatteryItem.qml:64
>  
> -states: [
> -State {
> -when: !!detailsLayout.parent.inListView // HACK
> -PropertyChanges {
> -target: detailsLabel
> -horizontalAlignment: modelData.value ? 
> Text.AlignRight : Text.AlignLeft
> -font.pointSize: theme.smallestFont.pointSize
> -width: parent ? parent.width / 2 : 0
> -elide: Text.ElideNone // eliding and height: 
> implicitHeight causes loops
> -}
> +onPaintedWidthChanged: { // horrible HACK to get a column 
> layout
> +if (modelData.label && paintedWidth > 
> detailsLayout.leftColumnWidth) {

No, it does not. Those two comments were in the same commit, to avoid the 
GridLayout crash.
https://phabricator.kde.org/R120:d12521b149fa7766fbbeb9957bde14b4a6278928

REPOSITORY
  R120 Plasma Workspace

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

To: gvgeo, #plasma, #vdg, davidedmundson
Cc: broulik, ngraham, manueljlin, davidedmundson, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, ahiemstra, mart

D26806: [Applets/Power Manager] Update layout based on T10470

2020-01-23 Thread David Edmundson

davidedmundson added inline comments.

INLINE COMMENTS

> broulik wrote in BatteryItem.qml:64
> That's been in there ever since the initial release 2013.

Well that explains  "// GridLayout crashes with a Repeater in it somehow"

REPOSITORY
  R120 Plasma Workspace

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

To: gvgeo, #plasma, #vdg, davidedmundson
Cc: broulik, ngraham, manueljlin, davidedmundson, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, ahiemstra, mart

D26806: [Applets/Power Manager] Update layout based on T10470

2020-01-23 Thread Kai Uwe Broulik

broulik added inline comments.

INLINE COMMENTS

> davidedmundson wrote in BatteryItem.qml:64
> You can't do that.
> 
> That'll change the column size, which will change our text bounding box which 
> will change our paintedWidth. A loop.

That's been in there ever since the initial release 2013.

REPOSITORY
  R120 Plasma Workspace

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

To: gvgeo, #plasma, #vdg, davidedmundson
Cc: broulik, ngraham, manueljlin, davidedmundson, plasma-devel, Orage, 
LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, ahiemstra, mart

D26806: [Applets/Power Manager] Update layout based on T10470

2020-01-23 Thread David Edmundson

davidedmundson requested changes to this revision.
davidedmundson added a comment.
This revision now requires changes to proceed.


  Few of my initial comments do not seem to be addressed yet.

INLINE COMMENTS

> BatteryItem.qml:64
>  
> -states: [
> -State {
> -when: !!detailsLayout.parent.inListView // HACK
> -PropertyChanges {
> -target: detailsLabel
> -horizontalAlignment: modelData.value ? 
> Text.AlignRight : Text.AlignLeft
> -font.pointSize: theme.smallestFont.pointSize
> -width: parent ? parent.width / 2 : 0
> -elide: Text.ElideNone // eliding and height: 
> implicitHeight causes loops
> -}
> +onPaintedWidthChanged: { // horrible HACK to get a column 
> layout
> +if (modelData.label && paintedWidth > 
> detailsLayout.leftColumnWidth) {

You can't do that.

That'll change the column size, which will change our text bounding box which 
will change our paintedWidth. A loop.

REPOSITORY
  R120 Plasma Workspace

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

To: gvgeo, #plasma, #vdg, davidedmundson
Cc: ngraham, manueljlin, davidedmundson, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D26806: [Applets/Power Manager] Update layout based on T10470

2020-01-23 Thread George Vogiatzis

gvgeo updated this revision to Diff 74228.
gvgeo added a comment.


  Remove detailsLoader visibility

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26806?vs=74222=74228

BRANCH
  b3 (branched from master)

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

AFFECTED FILES
  applets/batterymonitor/package/contents/ui/BatteryItem.qml
  applets/batterymonitor/package/contents/ui/BrightnessItem.qml
  applets/batterymonitor/package/contents/ui/InhibitionHint.qml
  applets/batterymonitor/package/contents/ui/PopupDialog.qml
  applets/batterymonitor/package/contents/ui/PowerManagementItem.qml
  applets/batterymonitor/package/contents/ui/batterymonitor.qml
  applets/batterymonitor/package/contents/ui/logic.js

To: gvgeo, #plasma, #vdg
Cc: ngraham, manueljlin, davidedmundson, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D26806: [Applets/Power Manager] Update layout based on T10470

2020-01-23 Thread George Vogiatzis

gvgeo updated this revision to Diff 74222.
gvgeo added a comment.


  Removed battery tooltip (had no extra info).
  Changed details view.
  
  Would be better without colon and without grid arrangement?

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26806?vs=74158=74222

BRANCH
  b3 (branched from master)

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

AFFECTED FILES
  applets/batterymonitor/package/contents/ui/BatteryItem.qml
  applets/batterymonitor/package/contents/ui/BrightnessItem.qml
  applets/batterymonitor/package/contents/ui/InhibitionHint.qml
  applets/batterymonitor/package/contents/ui/PopupDialog.qml
  applets/batterymonitor/package/contents/ui/PowerManagementItem.qml
  applets/batterymonitor/package/contents/ui/batterymonitor.qml
  applets/batterymonitor/package/contents/ui/logic.js

To: gvgeo, #plasma, #vdg
Cc: ngraham, manueljlin, davidedmundson, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D26806: [Applets/Power Manager] Update layout based on T10470

2020-01-22 Thread George Vogiatzis

gvgeo added a comment.


  First line in the commit message above `Workaround a crash in GridLayout when 
using a Repeater`. But was 5 years ago, don't know if this has been fixed, or 
how to reproduce for testing.
  Rowlayout in ColumnLayout should be fine, instead of repeater. I will look 
into it.

REPOSITORY
  R120 Plasma Workspace

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

To: gvgeo, #plasma, #vdg
Cc: ngraham, manueljlin, davidedmundson, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D26806: [Applets/Power Manager] Update layout based on T10470

2020-01-22 Thread Nathaniel Graham

ngraham added a comment.


  Is there anything stopping a port away from the FlowLayout and using a 
GridLayout, or even just using one Rowlayout per row, all stuck in a 
ColumnLayout?

REPOSITORY
  R120 Plasma Workspace

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

To: gvgeo, #plasma, #vdg
Cc: ngraham, manueljlin, davidedmundson, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D26806: [Applets/Power Manager] Update layout based on T10470

2020-01-22 Thread George Vogiatzis

gvgeo added a comment.


  An other interesting idea from apol D26735#597158 
 was to use checkbox, when a 
keyboard has only 1 setting for brightness like mine, as an on/off switch.
  But haven't found a way to fit it visually.

REPOSITORY
  R120 Plasma Workspace

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

To: gvgeo, #plasma, #vdg
Cc: ngraham, manueljlin, davidedmundson, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D26806: [Applets/Power Manager] Update layout based on T10470

2020-01-22 Thread George Vogiatzis

gvgeo added a comment.


  About the text subtitles
  
  In D26806#598067 , @gvgeo wrote:
  
  > I will not touch Tooltip in `BatteryItem.qml`, has a bug with Grid and uses 
Flow instead. That made very difficult to fix the tooltip and details.
  >  R120:d12521b149fa7766fbbeb9957bde14b4a6278928 

  
  
  If this bug has been fixed from then. I could probably made the changes.

REPOSITORY
  R120 Plasma Workspace

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

To: gvgeo, #plasma, #vdg
Cc: ngraham, manueljlin, davidedmundson, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D26806: [Applets/Power Manager] Update layout based on T10470

2020-01-22 Thread George Vogiatzis

gvgeo updated this revision to Diff 74158.
gvgeo added a comment.


  SmallMedium icons
  remove margin from inhibitor message

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26806?vs=74069=74158

BRANCH
  b3 (branched from master)

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

AFFECTED FILES
  applets/batterymonitor/package/contents/ui/BatteryItem.qml
  applets/batterymonitor/package/contents/ui/BrightnessItem.qml
  applets/batterymonitor/package/contents/ui/InhibitionHint.qml
  applets/batterymonitor/package/contents/ui/PopupDialog.qml
  applets/batterymonitor/package/contents/ui/PowerManagementItem.qml
  applets/batterymonitor/package/contents/ui/batterymonitor.qml

To: gvgeo, #plasma, #vdg
Cc: ngraham, manueljlin, davidedmundson, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D26806: [Applets/Power Manager] Update layout based on T10470

2020-01-22 Thread Nathaniel Graham

ngraham added subscribers: manueljlin, ngraham.
ngraham added a comment.


  Nice!
  
  Couple of UI comments:
  
  - The icons are now blurry, presumably because the plasma theme doesn't 
supply versions for the size you're using. I would recommend increasing the 
size until you find a size that the Plasma theme has icons for
  - This is a pre-existing issue, but the alignment and consistency for text 
subtitles is poor: F7906755: Screenshot_20200122_082510.png 
 We might want to consider right-aligning 
everything and using the same font size and opacity. Thoughts, @manueljlin?

REPOSITORY
  R120 Plasma Workspace

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

To: gvgeo, #plasma, #vdg
Cc: ngraham, manueljlin, davidedmundson, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, jraleigh, zachus, fbampaloukas, GB_2, ragreen, ZrenBot, 
alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D26806: [Applets/Power Manager] Update layout based on T10470

2020-01-21 Thread George Vogiatzis

gvgeo updated this revision to Diff 74069.
gvgeo added a comment.


  Revert 2 lines (unintentional changes)

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26806?vs=74045=74069

BRANCH
  b3 (branched from master)

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

AFFECTED FILES
  applets/batterymonitor/package/contents/ui/BatteryItem.qml
  applets/batterymonitor/package/contents/ui/BrightnessItem.qml
  applets/batterymonitor/package/contents/ui/InhibitionHint.qml
  applets/batterymonitor/package/contents/ui/PopupDialog.qml
  applets/batterymonitor/package/contents/ui/PowerManagementItem.qml
  applets/batterymonitor/package/contents/ui/batterymonitor.qml

To: gvgeo, #plasma, #vdg
Cc: davidedmundson, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D26806: [Applets/Power Manager] Update layout based on T10470

2020-01-21 Thread George Vogiatzis

gvgeo updated this revision to Diff 74045.
gvgeo added a comment.


  Correct previous wrong update

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26806?vs=74044=74045

BRANCH
  b3 (branched from master)

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

AFFECTED FILES
  applets/batterymonitor/package/contents/ui/BatteryItem.qml
  applets/batterymonitor/package/contents/ui/BrightnessItem.qml
  applets/batterymonitor/package/contents/ui/InhibitionHint.qml
  applets/batterymonitor/package/contents/ui/PopupDialog.qml
  applets/batterymonitor/package/contents/ui/PowerManagementItem.qml
  applets/batterymonitor/package/contents/ui/batterymonitor.qml

To: gvgeo, #plasma, #vdg
Cc: davidedmundson, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D26806: [Applets/Power Manager] Update layout based on T10470

2020-01-21 Thread George Vogiatzis

gvgeo updated this revision to Diff 74044.
gvgeo added a comment.


  Fix Layout issues. No changes of the apperance.

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26806?vs=74017=74044

BRANCH
  battery (branched from master)

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

AFFECTED FILES
  applets/batterymonitor/package/contents/ui/BatteryItem.qml
  applets/batterymonitor/package/contents/ui/BrightnessItem.qml
  applets/batterymonitor/package/contents/ui/InhibitionHint.qml
  applets/batterymonitor/package/contents/ui/PopupDialog.qml
  applets/batterymonitor/package/contents/ui/PowerManagementItem.qml
  applets/batterymonitor/package/contents/ui/batterymonitor.qml

To: gvgeo, #plasma, #vdg
Cc: davidedmundson, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D26806: [Applets/Power Manager] Update layout based on T10470

2020-01-21 Thread George Vogiatzis

gvgeo marked an inline comment as done.
gvgeo added a comment.


  I believe scroolbars should look good, but didn't test the new scrollbars. I 
use stable builds, where it touches the scrollbar.
  F7895378: Screenshot_20200121_152405.png 

  Can someone that test this, confirm if it looks okay with the new scrollbars 
changes?

REPOSITORY
  R120 Plasma Workspace

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

To: gvgeo, #plasma, #vdg
Cc: davidedmundson, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D26806: [Applets/Power Manager] Update layout based on T10470

2020-01-21 Thread George Vogiatzis

gvgeo updated this revision to Diff 74017.
gvgeo added a comment.


  Remove translating of new lines

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26806?vs=74014=74017

BRANCH
  b3 (branched from master)

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

AFFECTED FILES
  applets/batterymonitor/package/contents/ui/BatteryItem.qml
  applets/batterymonitor/package/contents/ui/BrightnessItem.qml
  applets/batterymonitor/package/contents/ui/InhibitionHint.qml
  applets/batterymonitor/package/contents/ui/PopupDialog.qml
  applets/batterymonitor/package/contents/ui/PowerManagementItem.qml
  applets/batterymonitor/package/contents/ui/batterymonitor.qml

To: gvgeo, #plasma, #vdg
Cc: davidedmundson, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D26806: [Applets/Power Manager] Update layout based on T10470

2020-01-21 Thread George Vogiatzis

gvgeo added a comment.


  I will not touch any more BatteryItem.qml, has a bug with Grid and uses Flow 
instead. That made very difficult to fix the tooltip and details.
  R120:d12521b149fa7766fbbeb9957bde14b4a6278928 

  Width did not work without specifying it (and needed fillWith too).
  Also wanted to remove the extra column, but sizes broke again. For no reason 
I could find.
  
  I opted to revert everything not needed, than going in blind.
  Maybe can put everything in tooltipArea again, if you prefer.

REPOSITORY
  R120 Plasma Workspace

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

To: gvgeo, #plasma, #vdg
Cc: davidedmundson, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D26806: [Applets/Power Manager] Update layout based on T10470

2020-01-21 Thread David Edmundson

davidedmundson added inline comments.

INLINE COMMENTS

> BatteryItem.qml:150
> +
> +property int _s: units.largeSpacing / 2
> +

can we use units.smallSpacing

If we invent our own margin policy everywhere by multiplying by random factors 
we're not going to have any consistency.

> BatteryItem.qml:152
> +
> +Layout.minimumWidth: implicitWidth + batteryItemToolTip._s
> +Layout.minimumHeight: implicitHeight + batteryItemToolTip._s * 2

you're using Layout. in a child of something that's not a layout

> BatteryItem.qml:162
> +BatteryIcon {
> +x: batteryItemToolTip._s * 2
> +y: batteryItemToolTip._s

Setting X in a row is a bad idea, use a layout

REPOSITORY
  R120 Plasma Workspace

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

To: gvgeo, #plasma, #vdg
Cc: davidedmundson, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D26806: [Applets/Power Manager] Update layout based on T10470

2020-01-21 Thread David Edmundson

davidedmundson added a comment.


  With any layout changes be sure you've tested it:
  
  - in the system tray
  - in the panel
  - in a vertical panel
  - on the desktop

INLINE COMMENTS

> BatteryItem.qml:96
> +RowLayout {
> +width: batteryItem.width
> +

We want to almost never specify a width inside a layout

It's the layout's job to change the item's width, which means when it re does 
the layout this information gets lost.

Instead, we want to set an implicitWidth or Layout.preferredWidth

This applies throughout

> PowerManagementItem.qml:69
> +if (suppressHint.visible) {
> +mainMessage += i18n("\n") + suppressHint.text
> +}

there's no point translating "\n"

REPOSITORY
  R120 Plasma Workspace

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

To: gvgeo, #plasma, #vdg
Cc: davidedmundson, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D26806: [Applets/Power Manager] Update layout based on T10470

2020-01-21 Thread George Vogiatzis

gvgeo updated this revision to Diff 74014.
gvgeo added a comment.


  Clean up

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26806?vs=74013=74014

BRANCH
  b3 (branched from master)

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

AFFECTED FILES
  applets/batterymonitor/package/contents/ui/BatteryItem.qml
  applets/batterymonitor/package/contents/ui/BrightnessItem.qml
  applets/batterymonitor/package/contents/ui/InhibitionHint.qml
  applets/batterymonitor/package/contents/ui/PopupDialog.qml
  applets/batterymonitor/package/contents/ui/PowerManagementItem.qml
  applets/batterymonitor/package/contents/ui/batterymonitor.qml

To: gvgeo, #plasma, #vdg
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D26806: [Applets/Power Manager] Update layout based on T10470

2020-01-21 Thread George Vogiatzis

gvgeo created this revision.
gvgeo added reviewers: Plasma, VDG.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
gvgeo requested review of this revision.

REVISION SUMMARY
  Icons were decreased in sized and placed in row with their labels.
  'Enable power management' was moved to bottom. Suppression messages
  kept on top, but now mirror in the checkbox's tooltip too.
  
  The complete main view can scroll now (only batteries
  could scroll).
  For the 'Enable power management' checkbox, fixed tooltip and switch
  area (tooltip was only on text, and switch was on full width).
  
  Battery stats kept bellow. There are multiple batteries (laptop main
  battery, keyboard, mouse etc.), and include many stats (eg. charge,
  health, charge time).
  
  Issues:
  Tab does not cycle though when on desktop (shift-tab works).
  Needs systemtray or theme update too. Otherwise suppression messages
  look weird.
  Issues with sliders:
  Wheel need 33,3 complete rotations to reach from minimum to maximum.
  Do not scale.
  Leave 1 pixel On at the left, leaving the impression that can lower
  brightness more.

REPOSITORY
  R120 Plasma Workspace

BRANCH
  b3 (branched from master)

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

AFFECTED FILES
  applets/batterymonitor/package/contents/ui/BatteryItem.qml
  applets/batterymonitor/package/contents/ui/BrightnessItem.qml
  applets/batterymonitor/package/contents/ui/InhibitionHint.qml
  applets/batterymonitor/package/contents/ui/PopupDialog.qml
  applets/batterymonitor/package/contents/ui/PowerManagementItem.qml
  applets/batterymonitor/package/contents/ui/batterymonitor.qml

To: gvgeo, #plasma, #vdg
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

42 matches

Mail list logo