D24070: Don't use toolTipMainText to show info, rather use the second line

2020-01-05 Thread Nathaniel Graham 
ngraham updated this revision to Diff 72811.
ngraham added a comment.


  Update according to review comments and tweak presentation

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24070?vs=71222=72811

BRANCH
  arcpatch-D24070

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

AFFECTED FILES
  applets/batterymonitor/package/contents/ui/batterymonitor.qml

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2020-01-05 Thread Nathaniel Graham 
ngraham planned changes to this revision.
ngraham added a comment.


  I'm not done yet. ;-)

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2020-01-04 Thread Nathaniel Graham 
ngraham commandeered this revision.
ngraham edited reviewers, added: mthw; removed: ngraham.
ngraham added a comment.


  Yoink. :)

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2020-01-01 Thread Nathaniel Graham 
ngraham added a comment.


  Yeah sorry, I need to block out some time to build a mental model of all the 
possible combinations currently, and what this changes them to, to make sure 
nothing regresses. Don't worry, this can go in after the soft feature freeze 
since it was already in progress, as long as it gets in before the hard feature 
freeze/string freeze.

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2020-01-01 Thread Matej Mrenica 
mthw added a comment.


  @ngraham Sorry to bother you again, but tomorrow is repo freeze and it would 
be unfortunate if this doesn't make it in. I mean it's finished and it works 
correctly, right? If needed we can still do small polishing during the Beta 
period, correct?

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-12-19 Thread Nathaniel Graham 
ngraham added a comment.


  Oh you know me, I'll be doing KDE stuff on Christmas eve. :)

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-12-19 Thread Matej Mrenica 
mthw added a comment.


  @ngraham could you please look at this? I would say it's finished and there 
isn't much time left until the next freeze. I also don't expect you to do this 
during the holidays, so it would be nice to have it finished, before them. I am 
sorry for being impatient.

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-12-17 Thread Björn Feber 
GB_2 added a comment.


  Ping :-)

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-12-12 Thread Nathaniel Graham 
ngraham added a comment.


  Yeah, I'll take over the review. Working through some other stuff first, will 
resume later today or tomorrow.

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-12-12 Thread Matej Mrenica 
mthw added a comment.


  What does it mean? Can this patch still go forward?

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-12-10 Thread Matej Mrenica 
mthw updated this revision to Diff 71222.
mthw added a comment.


  Tried to implement your suggestion, hopefully correctly, works correctly so 
far.

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24070?vs=71175=71222

BRANCH
  master

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

AFFECTED FILES
  applets/batterymonitor/package/contents/ui/batterymonitor.qml

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-12-10 Thread Kai Uwe Broulik 
broulik added a comment.


  You can just do something like
  
var parts = [];
if (powerManagementDisabled) {
parts.push(i18n("Powermanagement disabled"));
}
if (pmSource.data["AC Adapter"] && pmSource.data["AC Adapter"]["Plugged 
in"]) {
parts.push(blabla);
} else {
parts.push(blablabla);
}
parts.push(whatever other texts);
return parts.join("\n");

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-12-10 Thread Matej Mrenica 
mthw updated this revision to Diff 71175.
mthw added a comment.


  The second line is everywhere now. I guessed it would make more sense. But if 
you dont't like it that way, I can remove it.

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24070?vs=69910=71175

BRANCH
  master

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

AFFECTED FILES
  applets/batterymonitor/package/contents/ui/batterymonitor.qml

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-12-10 Thread Kai Uwe Broulik 
broulik added a comment.


  Probably:
  
  > MainText: Battery at X%
  >  SubText: Powermanagement disabled\n
  >  HH:MM left

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-12-10 Thread Matej Mrenica 
mthw added inline comments.

INLINE COMMENTS

> broulik wrote in batterymonitor.qml:56
> You still haven't addressed this. Don't just `return` when 
> `powermanagementDisabled`, I would still want to see my battery state when 
> this is the case?

OK, would you like to see:
MainText: Power management disabled
SubText: Battery at X%, HH:MM left
Or:
MainText: Battery at X% 
SubText:  HH:MM left
Or something else?

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-12-10 Thread Kai Uwe Broulik 
broulik requested changes to this revision.
broulik added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> broulik wrote in batterymonitor.qml:56
> You can probably just return `plasmoid.title` in the default case.
> Also, what's the reason for not showing percentage when power management is 
> disabled? This isn't about the service not running or being a desktop PC, 
> this is true when screens are forced on.

You still haven't addressed this. Don't just `return` when 
`powermanagementDisabled`, I would still want to see my battery state when this 
is the case?

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-12-10 Thread Matej Mrenica 
mthw added a comment.


  Is everything fine now?

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-11-18 Thread Matej Mrenica 
mthw updated this revision to Diff 69910.
mthw added a comment.


  Done

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24070?vs=69869=69910

BRANCH
  master

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

AFFECTED FILES
  applets/batterymonitor/package/contents/ui/batterymonitor.qml

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-11-18 Thread Kai Uwe Broulik 
broulik added inline comments.

INLINE COMMENTS

> batterymonitor.qml:72
> +if (powermanagementDisabled) {
> +return i18n("Power management is disabled");
> +}

This early return still hasn't been addressed.
When I keep the screen on I still want to see my remaining battery time.

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-11-17 Thread Nathaniel Graham 
ngraham added a comment.


  All good now, @broulik?

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-11-17 Thread Matej Mrenica 
mthw updated this revision to Diff 69869.
mthw added a comment.


  Added i18n()

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24070?vs=69854=69869

BRANCH
  master

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

AFFECTED FILES
  applets/batterymonitor/package/contents/ui/batterymonitor.qml

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-11-16 Thread Nathaniel Graham 
ngraham accepted this revision.
ngraham added a comment.


  Perfect.

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-11-16 Thread Matej Mrenica 
mthw updated this revision to Diff 69854.
mthw added a comment.


  Secondly if you wait long enough, the description will change, meaning the 
remaining time isn't calculated imediately after plug/un-plug which looks like 
a bug somewhere else. This change only adds a message that remaining time isn't 
available yet.

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24070?vs=69737=69854

BRANCH
  master

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

AFFECTED FILES
  applets/batterymonitor/package/contents/ui/batterymonitor.qml

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-11-16 Thread Matej Mrenica 
mthw added a comment.


  In D24070#563390 , @ndavis wrote:
  
  > Here's what I see when the power cable is plugged in: F7766890: 
Screenshot_20191116_125846.png 
  >
  > Here's when I unplug it: F7766893: Screenshot_20191116_125912.png 

  
  
  But if you plug it back again then it work correctly, right? I was also able 
to reproduce this issue but only by plugging cable first and then adding 
battery.

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-11-16 Thread Noah Davis 
ndavis added a comment.


  Here's what I see when the power cable is plugged in: F7766890: 
Screenshot_20191116_125846.png 
  
  Here's when I unplug it: F7766893: Screenshot_20191116_125912.png 


REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-11-14 Thread Matej Mrenica 
mthw added a comment.


  From previous comments:
  
  In D24070#534832 , @ngraham wrote:
  
  > The logic looks sane to me and the UI is good, but the 
time-to-empty/time-to-full values still don't always update correctly for me 
after a state change. I suspect this is something that @broulik will need to 
advise on. Maybe it's just not auto-updating itself because it was never needed 
to before this patch?
  
  
  
  
  In D24070#534876 , @ndavis wrote:
  
  > Works just as well as it did before the patch for me.
  
  
  @ndavis Could you check if everything still works fine?
  For me it does show the remaining time imediately after cable plug/unplug.
  @broulik Is everything fine with the code now?

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-11-14 Thread Nathaniel Graham 
ngraham added a comment.


  Yes, when the battery is being charged, right after I unplug the cord. I have 
a feeling that there is something odd going on with the power subsystem on my 
computer, since sometimes it doesn't show the time remaining. Most of the time 
it looks correct, like this: F7763860: Screenshot_20191114_120955.png 

  
  But the current code handles the non-ideal situation a bit more gracefully.

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-11-14 Thread Matej Mrenica 
mthw added a comment.


  @ngraham When does that happen? I am guessing, when batery is being charged, 
right?
  
  This is how it looks for me: https://imgur.com/a/vSkdDmo

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-11-14 Thread Nathaniel Graham 
ngraham added a comment.


  Erm, actually, I just saw this...
  
  F7763635: Screenshot_20191114_094001.png 


REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-11-14 Thread Nathaniel Graham 
ngraham accepted this revision.
ngraham added a comment.


  LGTM now!

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-11-14 Thread Matej Mrenica 
mthw updated this revision to Diff 69737.
mthw added a comment.


  Issue above fixed and some stuff removed. I do have a question though, what 
info should be shown when "Power management" is disabled? With this patch it 
shows:
  
  - Battery level + managemet disabled (with battery)
  - Widget name + management disabled (without battery)
  
  Is that OK, or would you like to have something different there?

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24070?vs=69693=69737

BRANCH
  master

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

AFFECTED FILES
  applets/batterymonitor/package/contents/ui/batterymonitor.qml

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-11-13 Thread Nathaniel Graham 
ngraham requested changes to this revision.
ngraham added a comment.
This revision now requires changes to proceed.


  Hmm
  
  F7761926: Screenshot_20191113_151950.png 

  
  Looks like the condition where `remainingTime` is 0 or undefined needs better 
handling.

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-11-13 Thread Matej Mrenica 
mthw updated this revision to Diff 69693.
mthw added a comment.


  I think I made all the changes you wanted. Hopefully I didn't break anything. 
It works correctly on first glance.

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24070?vs=66959=69693

BRANCH
  master

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

AFFECTED FILES
  applets/batterymonitor/package/contents/ui/batterymonitor.qml

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-11-13 Thread Kai Uwe Broulik 
broulik added a comment.


  There's also a lot of code duplication making the diff hard to read. Please 
group if statements together where it makes sense, e.g. avpod things like
  
if (foo) {
return plasmoid.title;
} else if (foo) {
return plasmoid.title;
  
  or
  
if (foo) {
if (bar) {
baz();
}
} else {
if (bar) {
baz();
}
}

INLINE COMMENTS

> batterymonitor.qml:85
> +if (powermanagementDisabled) {
> +return i18n("Power management is disabled")
> +}

When power management is disabled the subtext shows no useful information now.

Powermanagement disabled means disabled screen powermanagement (i.e. keep 
screen always on), has nothing to do with the battery percentage, which is 
still useful to have.

> batterymonitor.qml:95
>  if (remainingTime > 0) {
> -return i18nc("%1 is remaining time, %2 is percentage", "%1 
> Remaining (%2%)",
> - 
> KCoreAddons.Format.formatDuration(remainingTime, 
> KCoreAddons.FormatTypes.HideSeconds),
> - pmSource.data["Battery"]["Percent"])
> -} else {
> -return i18n("%1% Battery Remaining", 
> pmSource.data["Battery"]["Percent"]);
> +return i18n("%1 until fully charged", 
> KCoreAddons.Format.formatDuration(remainingTime, 
> KCoreAddons.FormatTypes.HideSeconds))
> +}

Keep the `i18nc` explaining that %1 is remaining time

> batterymonitor.qml:102
>  }
>  }
> +

What in the `else` case? Perhaps at least explicitly `return "";`

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-11-13 Thread Matej Mrenica 
mthw added a comment.


  @ndavis Unless this needs a rebase, all I need is review/approval.

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-11-12 Thread Noah Davis 
ndavis added a comment.


  @broulik @mthw ping :)

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-10-11 Thread Matej Mrenica 
mthw added a comment.


  @broulik Any other changes needed?

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-09-27 Thread Matej Mrenica 
mthw updated this revision to Diff 66959.
mthw marked an inline comment as done.
mthw added a comment.


  > Also, what's the reason for not showing percentage when power management is 
disabled? This isn't about the service not running or being a desktop PC, this 
is true when screens are forced on.
  
  Makes sense, but it was like this before, so I just kept it that way. 
Secondly will the string still be translated using "plasmoid.title"?

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24070?vs=66958=66959

BRANCH
  master

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

AFFECTED FILES
  applets/batterymonitor/package/contents/ui/batterymonitor.qml

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-09-27 Thread Kai Uwe Broulik 
broulik added inline comments.

INLINE COMMENTS

> batterymonitor.qml:56
> +if (powermanagementDisabled) {
> +return i18n("Battery and Brightness")
> +}

You can probably just return `plasmoid.title` in the default case.
Also, what's the reason for not showing percentage when power management is 
disabled? This isn't about the service not running or being a desktop PC, this 
is true when screens are forced on.

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-09-27 Thread Nathaniel Graham 
ngraham added a comment.


  In D24070#538806 , @mthw wrote:
  
  > Like this? Don't we need to return something when "remainingTime <= 0"?
  
  
  No, because in this case it just doesn't show a subtitle, which I think is 
fine.

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-09-27 Thread Matej Mrenica 
mthw updated this revision to Diff 66958.
mthw added a comment.


  Like this? Don't we need to return something when "remainingTime <= 0"?

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24070?vs=66487=66958

BRANCH
  master

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

AFFECTED FILES
  applets/batterymonitor/package/contents/ui/batterymonitor.qml

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-09-27 Thread Nathaniel Graham 
ngraham requested changes to this revision.
ngraham added a comment.
This revision now requires changes to proceed.


  From what I can tell, the problem with  `remainingTime` being 0 sometimes is 
a pre-existing bug that the current code is hiding fairly well by not showing 
the string that uses that value when the time is 0. We should fix that later, 
but for now what we should do is replicate the hiding logic for all cases in 
the new style too (see inline comment). Than I think this can go in, pending 
@broulik's approval.

INLINE COMMENTS

> batterymonitor.qml:93
> +}
> +if (pmSource.data["AC Adapter"] && pmSource.data["AC 
> Adapter"]["Plugged in"]) {
> +return i18n("%1 until fully charged", 
> KCoreAddons.Format.formatDuration(remainingTime, 
> KCoreAddons.FormatTypes.HideSeconds))

This block also needs` if (remainingTime > 0) {` like the one below it

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-09-27 Thread Matej Mrenica 
mthw added a comment.


  Is there still something that needs to be fixed, or can this go in?

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-09-19 Thread Noah Davis 
ndavis added a comment.


  Works just as well as it did before the patch for me.

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-09-19 Thread Nathaniel Graham 
ngraham accepted this revision.
ngraham added a reviewer: broulik.
ngraham added a comment.


  The logic looks sane to me and the UI is good, but the 
time-to-empty/time-to-full values still don't always update correctly for me 
after a state change. I suspect this is something that @broulik will need to 
advise on. Maybe it's just not auto-updating itself because it was never needed 
to before this patch?

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-09-19 Thread Matej Mrenica 
mthw added a comment.


  @ngraham Do you still have issues with this? @ndavis could you try this too? 
Or anyone else?

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-09-19 Thread Matej Mrenica 
mthw updated this revision to Diff 66487.
mthw added a comment.


  Re-order again

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24070?vs=66485=66487

BRANCH
  master

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

AFFECTED FILES
  applets/batterymonitor/package/contents/ui/batterymonitor.qml

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-09-19 Thread Matej Mrenica 
mthw added a comment.


  In D24070#534771 , @ngraham wrote:
  
  > Heh, how I get this every time my battery is discharging: F7387261: 
Screenshot_20190919_102717.png 
  
  
  Not here: F7387375: Screenshot_20190919_183545.png 


REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-09-19 Thread Matej Mrenica 
mthw updated this revision to Diff 66485.
mthw added a comment.


  Remove ";", reorder and simplify if clauses

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24070?vs=66482=66485

BRANCH
  master

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

AFFECTED FILES
  applets/batterymonitor/package/contents/ui/batterymonitor.qml

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-09-19 Thread Nathaniel Graham 
ngraham added a comment.


  Heh, how I get this every time my battery is discharging: F7387261: 
Screenshot_20190919_102717.png 

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-09-19 Thread Matej Mrenica 
mthw marked 6 inline comments as done.
mthw added a comment.


  Is the formatting OK?

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-09-19 Thread Matej Mrenica 
mthw updated this revision to Diff 66482.
mthw added a comment.


  Switch lines

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24070?vs=66480=66482

BRANCH
  master

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

AFFECTED FILES
  applets/batterymonitor/package/contents/ui/batterymonitor.qml

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-09-19 Thread Nathaniel Graham 
ngraham added a comment.


  In D24070#534721 , @mthw wrote:
  
  > I can do that if @ngraham agrees.
  
  
  Yeah alright, I don't have any strong negative preference. Consistency with 
the volume applet's tooltip is also another argument in favor.

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-09-19 Thread Matej Mrenica 
mthw updated this revision to Diff 66480.
mthw added a comment.


  Use battery at X%

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24070?vs=66476=66480

BRANCH
  master

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

AFFECTED FILES
  applets/batterymonitor/package/contents/ui/batterymonitor.qml

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-09-19 Thread Noah Davis 
ndavis added a comment.


  I think it's redundant to say "change level" since the percentage is always 
the charge level. I also think keeping the language consistent is a good idea. 
The volume tooltip says "Volume at %", which is why I originally suggested 
"Battery at %".

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-09-19 Thread Matej Mrenica 
mthw added a comment.


  I can do that if @ngraham agrees.

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-09-19 Thread Noah Davis 
ndavis added a comment.


  In D24070#534719 , @mthw wrote:
  
  > @ndavis So, generally, you would like to switch the two lines?
  
  
  Yes

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-09-19 Thread Matej Mrenica 
mthw added a comment.


  @ndavis So, generally, you would like to switch the two lines?

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-09-19 Thread Noah Davis 
ndavis added a comment.


  In D24070#534585 , @ngraham wrote:
  
  > In D24070#534447 , @ndavis wrote:
  >
  > > In D24070#534378 , @broulik 
wrote:
  > >
  > > > Volume shows the percentage in the main title so this is inconsistent 
now.
  > > >  Perhaps instead show some other information like remaining time in the 
subtext?
  > >
  > >
  > > Yes, perhaps it should be like this:
  > >
  > >   Battery at 100%
  > >   Plugged in, not charging
  > >
  >
  >
  > Yeah, that makes sense to me. Put the primary information in the first 
line, and additional information in the second line. For example:
  >
  >   4:58 remaining
  >   55% Battery charge level
  >
  >
  >
  >
  >   2:08 until fully charged
  >   78% Battery charge level
  >
  >
  >
  >
  >   2:08 until fully charged
  >   78% Battery charge level
  >
  >
  >
  >
  >   Battery 100% charged
  >   Plugged in
  >
  >
  > (not totally sure about that last one, just an idea)
  
  
  I think the percentage is more important than the timer because the timer 
might be misleading. For instance, my own laptop says my battery can last 10 
hours on a full battery, but that seems awfully suspicious to me. I don't think 
I've ever actually gotten 10 hours out of my battery.

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-09-19 Thread Matej Mrenica 
mthw updated this revision to Diff 66476.
mthw added a comment.


  un-capitalize

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24070?vs=66474=66476

BRANCH
  master

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

AFFECTED FILES
  applets/batterymonitor/package/contents/ui/batterymonitor.qml

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-09-19 Thread Nathaniel Graham 
ngraham added inline comments.

INLINE COMMENTS

> batterymonitor.qml:70
> +if (remainingTime > 0) {
> +return i18n("%1 Remaining", 
> KCoreAddons.Format.formatDuration(remainingTime, 
> KCoreAddons.FormatTypes.HideSeconds))
> +}

un-capitalize the R

> batterymonitor.qml:70
> +if (remainingTime > 0) {
> +return i18n("%1 Remaining", 
> KCoreAddons.Format.formatDuration(remainingTime, 
> KCoreAddons.FormatTypes.HideSeconds))
> +}

Un-capitalize the R

> batterymonitor.qml:89
>  if (state === "Charging") {
> -return i18n("%1% Charging", percent)
> +return i18n("%1% Battery charge level", percent)
>  } else if (state === "NoCharge") {

Un-capitalize the B

> batterymonitor.qml:93
>  } else {
> -return i18n("%1% Plugged in", percent)
> +return i18n("%1% Battery charge level", percent)
>  }

Un-capitalize the B

> batterymonitor.qml:96
>  } else {
> -if (remainingTime > 0) {
> -return i18nc("%1 is remaining time, %2 is percentage", "%1 
> Remaining (%2%)",
> - 
> KCoreAddons.Format.formatDuration(remainingTime, 
> KCoreAddons.FormatTypes.HideSeconds),
> - pmSource.data["Battery"]["Percent"])
> -} else {
> -return i18n("%1% Battery Remaining", 
> pmSource.data["Battery"]["Percent"]);
> -}
> +return i18n("%1% Battery charge level", 
> pmSource.data.Battery.Percent)
>  }

Un-capitalize the B

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-09-19 Thread Nathaniel Graham 
ngraham added a comment.


  In D24070#534687 , @mthw wrote:
  
  > In D24070#534676 , @ngraham 
wrote:
  >
  > > Uh-oh, looks like there's a binding that isn't getting updated properly; 
the "time to charge/time to empty" number always reads as 0:00 for me: 
F7386917: Screenshot_20190919_092713.png 
  > >
  > > Also when I unplug the cable, the tooltip's first line doesn't change: 
F7386912: Screenshot_20190919_092632.png 
  >
  >
  > I had this problem too but it fixes itself after a while, don't know why.
  
  
  Probably a broken binding. perhaps @broulik can help.

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-09-19 Thread Matej Mrenica 
mthw added a comment.


  In D24070#534676 , @ngraham wrote:
  
  > Uh-oh, looks like there's a binding that isn't getting updated properly; 
the "time to charge/time to empty" number always reads as 0:00 for me: 
F7386917: Screenshot_20190919_092713.png 
  >
  > Also when I unplug the cable, the tooltip's first line doesn't change: 
F7386912: Screenshot_20190919_092632.png 
  
  
  I had this problem too but it fixes itself after a while, don't know why.
  
  Now it works correctly:
  
  F7386988: Screenshot_20190919_173412.png 

  
  F7386989: Screenshot_20190919_173438.png 


REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-09-19 Thread Nathaniel Graham 
ngraham requested changes to this revision.
ngraham added a comment.
This revision now requires changes to proceed.


  Uh-oh, looks like there's a binding that isn't getting updated properly; the 
"time to charge/time to empty" number always reads as 0:00 for me: F7386917: 
Screenshot_20190919_092713.png 
  
  Also when I unplug the cable, the tooltip's first line doesn't change: 
F7386912: Screenshot_20190919_092632.png 

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-09-19 Thread Matej Mrenica 
mthw updated this revision to Diff 66474.
mthw added a comment.


  re-added powermanagementDisabled and fixed white spaces

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24070?vs=66470=66474

BRANCH
  master

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

AFFECTED FILES
  applets/batterymonitor/package/contents/ui/batterymonitor.qml

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-09-19 Thread Matej Mrenica 
mthw updated this revision to Diff 66470.
mthw added a comment.


  Made changes, according to instructions, tested, should work correctly

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24070?vs=66431=66470

BRANCH
  master

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

AFFECTED FILES
  applets/batterymonitor/package/contents/ui/batterymonitor.qml

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-09-19 Thread Nathaniel Graham 
ngraham added a comment.


  In D24070#534447 , @ndavis wrote:
  
  > In D24070#534378 , @broulik 
wrote:
  >
  > > Volume shows the percentage in the main title so this is inconsistent now.
  > >  Perhaps instead show some other information like remaining time in the 
subtext?
  >
  >
  > Yes, perhaps it should be like this:
  >
  >   Battery at 100%
  >   Plugged in, not charging
  >
  
  
  Yeah, that makes sense to me. Put the primary information in the first line, 
and additional information in the second line. For example:
  
4:58 remaining
55% Battery charge level
  
  
  
2:08 until fully charged
78% Battery charge level
  
  
  
2:08 until fully charged
78% Battery charge level
  
  
  
Battery 100% charged
Plugged in
  
  (not totally sure about that last one, just an idea)

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-09-19 Thread Noah Davis 
ndavis added a comment.


  In D24070#534378 , @broulik wrote:
  
  > Volume shows the percentage in the main title so this is inconsistent now.
  >  Perhaps instead show some other information like remaining time in the 
subtext?
  
  
  Yes, perhaps it should be like this:
  
Battery at 100%
Plugged in, not charging

REPOSITORY
  R120 Plasma Workspace

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

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

D24070: Don't use toolTipMainText to show info, rather use the second line

2019-09-19 Thread Kai Uwe Broulik 
broulik added a comment.


  Volume shows the percentage in the main title so this is inconsistent now.
  Perhaps instead show some other information like remaining time in the 
subtext?

INLINE COMMENTS

> batterymonitor.qml:82
>  }
> +if (powermanagementDisabled) {
> +return i18n("Power management is disabled")

This code path will never be reached

REPOSITORY
  R120 Plasma Workspace

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

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