! Please note that this is a snapshot of our old Bugzilla server, which is read only since May 29, 2020. Please go to gitlab.xfce.org for our new server !
Patch to show values of traffic on a panel plugin
Status:
CLOSED: FIXED
Product:
Xfce4-netload-plugin
Component:
General

Comments

Description Ivan Romanov 2011-07-11 12:24:39 CEST
Created attachment 3781 
patch

Hello. I wrote patch for this feature. Can you add it to repo?
Comment 1 Ivan Romanov 2011-07-11 12:42:30 CEST
Created attachment 3782 
screenshot
Comment 2 Ivan Romanov 2011-08-12 05:12:17 CEST
Created attachment 3819 
patch-v2

First version of this patch haven't Show_Bars option in config file. I fixed this.
Comment 3 Mike Massonnet editbugs 2011-12-20 21:02:47 CET
Patch works fine, but it takes a lot of space in the panel.
Comment 4 Ivan Romanov 2011-12-21 14:43:23 CET
Maybe do you can guess a more good solution?
Comment 5 Harald Judt 2011-12-21 15:04:34 CET
Problems:

* It does not show the label anymore, even when unselecting "Show values". What if you got two interfaces (eth0, wlan0)?

* The values are already shown in the tooltip.

> Patch works fine, but it takes a lot of space in the panel.

Suggestions:

* Make unit names shorter (kByte => kiB).

* Use smaller horizontal spaces.

* Similar to the panel date and time plugin, make the font a little bit smaller. Then put both values on the right side of the bars and on top of each other, like this (it's a bit hard to do in ASCII, but I think you'll get the idea):

[    ] || [ D: 0.0kiB ]
[eth0] || [           ]
[    ] || [ U: 0.0kiB ]

* Another idea: Use colors for down and up, like those for the bars.
Comment 6 Ivan Romanov 2011-12-21 15:10:49 CET
i will think
Comment 7 Mike Massonnet editbugs 2011-12-25 17:12:01 CET
Created attachment 4042 
0001-Add-option-to-show-text-values-in-panel.patch

after looking at the patch:
- re-added the option to show the interface name
  - this bug is about adding an option, there is no reason the attached patch should remove existing options
- moved the "Show values" option under "Show bars" for aesthetic reason
- changed "kByte" against "KiB/s"

please test, diff will be pushed in the coming days
Comment 8 Ivan Romanov 2011-12-25 18:18:41 CET
Hm ... with your version of the patch applet growing and shrinking. It hasn't constant size.
Comment 9 Ivan Romanov 2011-12-25 18:27:10 CET
I reverted 
+    gtk_label_set_width_chars(GTK_LABEL(global->monitor->rcv_label), 13);
and
+    gtk_label_set_width_chars(GTK_LABEL(global->monitor->snd_label), 13);
for myself. Maybe we can use width less than 13?
Comment 10 Harald Judt 2011-12-27 15:23:12 CET
Created attachment 4045 
Updated patch to show values in panel.

* changed format_with_thousandssep into format_byte_humanreadable, used in labels and tooltip
* set size of label (set to 10 chars for now)
* set alignment of labels
* fixed spelling mistakes in code

Problems/annoyances left:
* check boxes are not a good choice anymore for bars/labels; a combobox would be much better here (only bars, only values, both)
* do the new byte units (Byte, KiB,...) need translation?
Comment 11 Ivan Romanov 2011-12-27 15:47:50 CET
Check your patch. You used tabs. I think it should to use spaces.
Comment 12 Harald Judt 2011-12-27 16:04:57 CET
Created attachment 4046 
Untabified patch to show values in panel.

* Untabified patch
* Added german translation (I did not chang anything related to unit names though).
* Set "show values" option to false by default, as to not mess up existing user's configuration. I hope that I did the right thing when reading in the config file, setting a parameter from TRUE to FALSE, please check this.
Comment 13 Mike Massonnet editbugs 2011-12-27 16:21:12 CET
(In reply to comment #9)
> I reverted 
> +    gtk_label_set_width_chars(GTK_LABEL(global->monitor->rcv_label), 13);
> and
> +    gtk_label_set_width_chars(GTK_LABEL(global->monitor->snd_label), 13);
> for myself. Maybe we can use width less than 13?

It didn't bug me, but you are right, it's better to avoid this.


(In reply to comment #10)
> Created attachment 4045 
> Updated patch to show values in panel.
> 
> * changed format_with_thousandssep into format_byte_humanreadable, used in
> labels and tooltip
> * set size of label (set to 10 chars for now)
> * set alignment of labels
> * fixed spelling mistakes in code
> 
> Problems/annoyances left:
> * check boxes are not a good choice anymore for bars/labels; a combobox would
> be much better here (only bars, only values, both)
> * do the new byte units (Byte, KiB,...) need translation?

One issue per bug, with attachment 4046 , there is a change in the code with regard to 'format_with_thousandssep' (with untranslatable strings). Please open a new bug for this different issue.

This bug should only add the option "Show values". Once it will be pushed, the commit will be marked in the log history with 'bug XXX: message'. This will keep the history clean.
Comment 14 Harald Judt 2011-12-27 16:44:57 CET
Created attachment 4047 
Patch to add and implement option "Show Values".
Comment 15 Ivan Romanov 2011-12-27 17:04:10 CET
Created attachment 4048 
Russian translation
Comment 16 Harald Judt 2011-12-27 17:52:12 CET
Opened bug 8280 with separate follow-up patches for the byte units/space issue.
Comment 17 Harald Judt 2011-12-28 17:35:23 CET
Created attachment 4053 
Use combobox instead of checkboxes.

This patch should apply on top of my previous patch 4047. If not, only small alterations will be needed.

It removes the checkbox controls "show bars" and "show values" with a combobox providing three choices to present data.

I think this looks more clean and has better and more uncomplicated behaviour than activating/deactivating other widgets.
Comment 18 Harald Judt 2011-12-28 17:52:03 CET
Created attachment 4054 
Updated german translation for patch 4053.

Here's a patch to update the german translation. Again, I don't know if it applies cleanly, but theoretically it should.

BTW: Are there documentation or guidelines on how to deal with po files? I have no experience modifying those, I usually change it and see if that works.

Another thing I'd like to do is to update the mnemonics for the other widgets. Modifying the source code is trivial. However, this would force updates for most translations. What do you think?
Comment 19 Ivan Romanov 2011-12-28 18:07:19 CET
I think we should before decide which patch we will apply. Only then do anything with translations file. And sure it's good idea to update all translations.
Comment 20 Harald Judt 2011-12-28 19:11:14 CET
I think this can be done independently of this bug. Therefore, I opened a bug #8283 which adds mnemonics and updates msgids in translation files. It does not touch the translation strings so everything should look like before.
Comment 21 Ivan Romanov 2011-12-29 17:40:45 CET
(In reply to comment #17)
> Created attachment 4053 
> Use combobox instead of checkboxes.
> 
> This patch should apply on top of my previous patch 4047. If not, only small
> alterations will be needed.
> 
> It removes the checkbox controls "show bars" and "show values" with a combobox
> providing three choices to present data.
> 
> I think this looks more clean and has better and more uncomplicated behaviour
> than activating/deactivating other widgets.

Cool! It looks very more prefer for me. Only one thing. Maybe should use 'Present data as' instead of 'Present Data As'? 'Only' word in combobox also should write with lower-case first letter. Hm ... what about semicolon? Did you passed it? Or it is true?
Comment 22 Ivan Romanov 2011-12-29 17:43:20 CET
Oh. My English. I meant 'colon' when I was saying 'semicolon'.
Comment 23 Ivan Romanov 2011-12-29 17:57:30 CET
*said. ((( Today I am on 'top'.
Comment 24 Harald Judt 2011-12-29 23:17:44 CET
I took the combobox choices from the places plugin and there they are capitalized ('Only Icon', 'Only Label', 'Icon and Label'). The Englishmen seem to capitalize everything these days, claiming it's easier to read that way. Since I use the German translation where it would be gramatically incorrect, it does not bother me and I try to match their preferences.

About the colon: I did not notice, so feel free to add it if you miss it.
Comment 25 Ivan Romanov 2011-12-30 03:39:30 CET
Have a look at another labels. It capitalized only first word.
Comment 26 Ivan Romanov 2011-12-30 06:37:15 CET
I've looked places plugin. Really it capitalize each word. Also places plugin doesn't use colons.
Comment 27 Mike Massonnet editbugs 2011-12-30 08:04:53 CET
(In reply to comment #26)
> I've looked places plugin. Really it capitalize each word. Also places plugin
> doesn't use colons.

There is a wiki page that resumes the way strings should be inserted:
http://wiki.xfce.org/style/strings
Comment 28 Ivan Romanov 2011-12-30 08:30:28 CET
Labels Before Controls

    Like other text, the first word should be capitalized, while the rest are not.
    When the label is before a control, such as descriptive text to the left of a dropdown box, append a colon to the string, with NO SPACE between the last letter of the string and the colon.

It's our case. Should use 'Present data as:'. So?
Comment 29 Harald Judt 2012-01-02 12:43:56 CET
Created attachment 4069 
Use combobox instead of checkboxes.

Use string adhering to http://wiki.xfce.org/style/strings.
Comment 30 Harald Judt 2012-01-02 12:46:01 CET
Created attachment 4070 
Updated german translation for patch 4069.
Comment 31 Harald Judt 2012-01-02 12:47:33 CET
Created attachment 4071 
Updated german translation for patch 4069.
Comment 32 Ivan Romanov 2012-01-02 12:55:25 CET
I don't sure about 'Bars Only'. What does Mike say?
Comment 33 Harald Judt 2012-01-02 13:33:39 CET
We could remove 'only' completely:

Present data as: 'Bars', 'Values', 'Bars and Values'
Comment 34 Harald Judt 2012-01-02 14:07:13 CET
In vertical mode, the alignment of the upload label should be changed from left to right.

For my taste, it still wastes too much space, except when you change the panel's orientation to vertical mode. Is there a reason not to put the labels on top of each other?

I wonder how it copes with the new deskbar mode.
Comment 35 Ivan Romanov 2012-01-02 14:16:46 CET
For me layout is very fine. I don't want to change something.
Comment 36 Harald Judt 2012-01-02 14:31:52 CET
Created attachment 4072 
Use combobox instead of checkboxes.

Remove 'Only'.
Comment 37 Harald Judt 2012-01-02 14:32:36 CET
Created attachment 4073 
Updated german translation for patch 4072.
Comment 38 Ivan Romanov 2012-01-02 14:34:06 CET
Oh ... 'Only' word good for me. But I don't like uppercase.
Comment 39 Harald Judt 2012-01-02 14:39:19 CET
Created attachment 4074 
Updated german translation for patch 4072.

Forgot to save in previous version.
Comment 40 Harald Judt 2012-01-02 14:40:30 CET
Created attachment 4075 
Fix vertical alignment of sent label in vertical mode.

This makes the "sent" label look better in vertical mode.
Comment 41 Harald Judt 2012-01-02 14:52:21 CET
(In reply to comment #38)
> Oh ... 'Only' word good for me. But I don't like uppercase.

First, it's capitalized in xfce4-places-plugin. Second, I don't think it is needed. 'Only' can be deduced from the other choices and the fact that it is a combobox. Anyway, I feel a bit this is just nit-picking, and it would be great if the current maintainer could decide whether the patch is good enough now or tell us how to improve it.
Comment 42 Harald Judt 2012-01-02 16:25:47 CET
Created attachment 4076 
Add option to colorize values using bar colors

Follow-up patch adding an option to colorize the values using the bar colors.
Comment 43 Mike Massonnet editbugs 2012-01-07 15:19:13 CET
Pushed patches in git.
Comment 44 Mike Massonnet editbugs 2012-01-07 15:22:38 CET
Translations will need to be uploaded through Transifex[1] or sent to me.

[1] https://translations.xfce.org/projects/p/xfce4-netload-plugin/c/master/
Comment 45 Ivan Romanov 2012-01-07 15:28:25 CET
Thanks! It's best present on New Year days for me.

Bug #7804

Reported by:
Ivan Romanov
Reported on: 2011-07-11
Last modified on: 2012-01-13

People

Assignee:
Mike Massonnet
CC List:
3 users

Version

Version:
unspecified

Attachments

patch (18.98 KB, patch)
2011-07-11 12:24 CEST , Ivan Romanov
no flags
screenshot (33.49 KB, image/png)
2011-07-11 12:42 CEST , Ivan Romanov
no flags
patch-v2 (19.20 KB, patch)
2011-08-12 05:12 CEST , Ivan Romanov
no flags
0001-Add-option-to-show-text-values-in-panel.patch (14.92 KB, patch)
2011-12-25 17:12 CET , Mike Massonnet
no flags
Updated patch to show values in panel. (18.75 KB, patch)
2011-12-27 15:23 CET , Harald Judt
no flags
Untabified patch to show values in panel. (20.64 KB, patch)
2011-12-27 16:04 CET , Harald Judt
no flags
Patch to add and implement option "Show Values". (16.15 KB, patch)
2011-12-27 16:44 CET , Harald Judt
no flags
Russian translation (505 bytes, patch)
2011-12-27 17:04 CET , Ivan Romanov
no flags
Use combobox instead of checkboxes. (7.88 KB, patch)
2011-12-28 17:35 CET , Harald Judt
no flags
Updated german translation for patch 4053. (686 bytes, patch)
2011-12-28 17:52 CET , Harald Judt
no flags
Use combobox instead of checkboxes. (7.88 KB, patch)
2012-01-02 12:43 CET , Harald Judt
no flags
Updated german translation for patch 4069. (687 bytes, patch)
2012-01-02 12:46 CET , Harald Judt
no flags
Updated german translation for patch 4069. (688 bytes, patch)
2012-01-02 12:47 CET , Harald Judt
no flags
Use combobox instead of checkboxes. (7.87 KB, patch)
2012-01-02 14:31 CET , Harald Judt
no flags
Updated german translation for patch 4072. (675 bytes, patch)
2012-01-02 14:32 CET , Harald Judt
no flags
Updated german translation for patch 4072. (670 bytes, patch)
2012-01-02 14:39 CET , Harald Judt
no flags
Fix vertical alignment of sent label in vertical mode. (1.48 KB, patch)
2012-01-02 14:40 CET , Harald Judt
no flags
Add option to colorize values using bar colors (5.11 KB, patch)
2012-01-02 16:25 CET , Harald Judt
no flags

Additional information