D27792: [Applet]Fix traffic monitor labels width

[George Vogiatzis]

This revision was automatically updated to reflect the committed changes.
Closed by commit R116:986341a3fae8: [Applet]Fix traffic monitor labels width 
(authored by gvgeo).

REPOSITORY
  R116 Plasma Network Management Applet

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27792?vs=76797=77018

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

AFFECTED FILES
  applet/contents/ui/TrafficMonitor.qml

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

D27792: [Applet]Fix traffic monitor labels width

[Nathaniel Graham]

ngraham added a comment.


  Yeah, if you want to keep the plotter dimensions constant, then there's no 
way around measuring the width of the labels.
  
  Regarding not opening any new patches, I hope you didn't have a bad 
experience in KDE that's left a bad taste in your mouth. If anything's gone 
wrong, don't hesitate to let me know and I'll do what I can.

REPOSITORY
  R116 Plasma Network Management Applet

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

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

D27792: [Applet]Fix traffic monitor labels width

[George Vogiatzis]

gvgeo added a comment.


  I don't plan to make more patches, for now. But if the problem would not 
exist with layouts, prefer to do it than make quick bugfixes.
  
  But can't see how the Layouts would help with that. Everything can be done 
without Layouts too AFAIK, just more messy.
  I want to keep the plotter dimensions constant. It would not be nice to 
change width constantly.

REPOSITORY
  R116 Plasma Network Management Applet

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

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

D27792: [Applet]Fix traffic monitor labels width

[Nathaniel Graham]

ngraham accepted this revision.
ngraham added a comment.


  If you need to measure the width of text, then you're doing it the right way.
  
  However this problem wouldn't be happening in the first place if this UI used 
Layouts, which handle this kind of case more elegantly so you don't need to 
manually set margins and measure the width of items.
  
  So I say feel free to land this now since it's a correct bugfix and the 
current code is obviously wrong. And I think we should move towards porting to 
Layouts in another patch. I can do that if you're sick of plasma-nm. :)

REPOSITORY
  R116 Plasma Network Management Applet

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

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

D27792: [Applet]Fix traffic monitor labels width

[George Vogiatzis]

gvgeo added a comment.


  #VDG  Does a better way exist, to 
measure the width of transfer speed text?

REPOSITORY
  R116 Plasma Network Management Applet

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

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

D27792: [Applet]Fix traffic monitor labels width

[George Vogiatzis]

gvgeo added a comment.


  Is it?

INLINE COMMENTS

> TrafficMonitor.qml:107
> +// Measure 888.8 KiB/s
> +text: KCoreAddons.Format.formatByteSize(910131) + i18n("/s")
> +}

Biggest number it can display?

REPOSITORY
  R116 Plasma Network Management Applet

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

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

D27792: [Applet]Fix traffic monitor labels width

[David Edmundson]

davidedmundson added inline comments.

INLINE COMMENTS

> TrafficMonitor.qml:107
> +// Measure 888.8 KiB/s
> +text: KCoreAddons.Format.formatByteSize(910131) + i18n("/s")
> +}

what is this number about?

REPOSITORY
  R116 Plasma Network Management Applet

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

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

D27792: [Applet]Fix traffic monitor labels width

[George Vogiatzis]

gvgeo added a comment.


  @ngraham Is there a better way to fix this?
  
  In D27199#620730 , @jgrulich wrote:
  
  > Looks this changed introduced an issue. The traffic monitor doesn't fit 
into the applet.
  >
  > See picture below:
  >  F8145280: plasma-nm.png 

REPOSITORY
  R116 Plasma Network Management Applet

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

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

D27792: [Applet]Fix traffic monitor labels width

[George Vogiatzis]

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

REVISION SUMMARY
  Use text metrics to set a proper space for labels.
  And remove unused plotter width.

TEST PLAN
  Change default font styles and family.
  Change small font size.
  Space available must follow the font.

REPOSITORY
  R116 Plasma Network Management Applet

BRANCH
  traffic (branched from master)

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

AFFECTED FILES
  applet/contents/ui/TrafficMonitor.qml

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