! 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 !
Make spacing consistent with other plugins.
Status:
RESOLVED: MOVED
Severity:
enhancement
Product:
Xfce4-systemload-plugin
Component:
General

Comments

Description Harald Judt 2012-05-21 14:01:13 CEST
Created attachment 4444 
xfce4-systemload-plugin-spacing.patch

Other plugins with monitors have a default spacing/border of 8. Although that wastes a bit of space, for a more consistent and cleaner look & feel it might be better to adapt that setting.
Comment 1 Landry Breuil editbugs 2012-05-21 22:13:18 CEST
That's quite ugly, imo..and i'm pretty sure most users wont like it.
Comment 2 Harald Judt 2012-05-21 22:45:37 CEST
Created attachment 4448 
screenshot.png

I don't understand. Here is a screenshot showing the patched systemload plugin besides some diskperf plugin. With the patch, they have the same spacing as in the screenshot, without it, they do not. Does it look different for you? What's the problem?
Comment 3 Harald Judt 2012-05-21 22:49:44 CEST
Created attachment 4449 
screenshot2.png

Here's another screenshot showing the original, unpatched version. As you can see, there is no space between the monitors and the labels, unlike the diskperf or other plugins (netload for example).

That is with systemload-1.1.0.
Comment 4 Landry Breuil editbugs 2012-05-22 09:53:37 CEST
Just an example. I dont use labels, and the progressbars are 'stuck' one next to each other, like netload does too with its two adjacents progressbars. If you want to add some spacing then it might be better to put it on the label.
Comment 5 Landry Breuil editbugs 2012-05-22 09:54:39 CEST
Created attachment 4451 
screenshot of monitors without labels

See how it looks iwht systemload, netload, and your patched systemload. Now which one looks wrong ?
Comment 6 Landry Breuil editbugs 2012-05-22 10:01:32 CEST
Systemload needs an overhaul anyway, i noticed it was using

if (orientation == HORIZONTAL)
create vbox
else 
create hbox

while it should just use hvbox....
Comment 7 Harald Judt 2012-05-22 10:55:21 CEST
Ok, that's a valid reason. I did never use plugins without labels and find it clearer that way. So if I put some spacing to the left of the labels, that would be ok? Without the spacing, they look jammed together, as you may probably agree. We could change the other plugins to do the same. How about adding this to http://wiki.xfce.org/dev/hig/panel-plugins?

> Now which one looks wrong ?
The difference is that systemload plugin combines three plugins into one, while the others are all separate. I wonder if the authors intended a use-case as yours, where you can string together the monitors without space between. Is there a plugin that does? Obviously, they did not. But you're right, my patch would make the problem worse here if you see the three meters as a single component. The other plugins with more than one meter do not allow any spacing between their meters.

In general, systemload plugin could need an overhaul, and it should be made using only one row in vertical mode (see places plugin, diskperf plugin). It would also be an advantage if you could split up the single monitors. I see there is already someone working on a new monitor plugin? -> http://wiki.xfce.org/plugins_wish_list

Code state is not such a mess like in netload plugin, but is not ideal either and suffers from similar problems. Here is a patch I wrote for netload which refactors the widget creation and configuration parts: https://bugzilla.xfce.org/attachment.cgi?id=4437&action=diff
What do you think about that approach? Are there other reasons except historical ones why it is the way it is now?

Finally a personal matter: Telling something is ugly or bad without giving any explanation why is very impolite. Also claiming (most) other users would share your opinion is guesswork and not fair. Remember I could claim the same, and I suppose you did not ask them? So please be careful, you might turn off new contributors and probably annoy existing ones.
Comment 8 Harald Judt 2012-05-22 13:41:57 CEST
Created attachment 4452 
xfce4-systemload-plugin-spacing.patch

After some further investigation, I came to the conclusion that this is not trivial to do - if not impossible - by setting the properties of the present widgets. If you set a border on the labels, it will never look the same as the other plugins because the border will be on all sides.

So here is another patch which adds a spacing box before each label and toggles them on and off together with the corresponding label. Both mine and your configuration should be satisfied.

BTW: Just to avoid confusion, my comment about stringing monitors together was meant spanning different plugins. Unfortunately, that is something neither systemload nor the other plugins can do.
Comment 9 Landry Breuil editbugs 2012-05-22 15:53:09 CEST
(In reply to comment #7)

> > Now which one looks wrong ?
> The difference is that systemload plugin combines three plugins into one,
> while the others are all separate. I wonder if the authors intended a
> use-case as yours, where you can string together the monitors without space
> between. Is there a plugin that does? Obviously, they did not.

Provide an option to users, they'll use it. So that's an intended usecase..

> Code state is not such a mess like in netload plugin, but is not ideal
> either and suffers from similar problems. Here is a patch I wrote for
> netload which refactors the widget creation and configuration parts:
> https://bugzilla.xfce.org/attachment.cgi?id=4437&action=diff
> What do you think about that approach? Are there other reasons except
> historical ones why it is the way it is now?

Historically a lot of code was copied/pasted between plugins, with various maintainers with different code styling, and each code evolved its own way, hence the ui & code differences across all plugins. 

(In reply to comment #8)
> Created attachment 4452 
> xfce4-systemload-plugin-spacing.patch
> 
> After some further investigation, I came to the conclusion that this is not
> trivial to do - if not impossible - by setting the properties of the present
> widgets. If you set a border on the labels, it will never look the same as
> the other plugins because the border will be on all sides.

Yeah, but if you look at netload, the border is on all sides of the label. I (_personally_) think it's better ui-wise that if a label is displayed it shouldn't be 'stuck' with the corresponding progressbar.. battery & diskperf plugin have this 'problem' too. (well for battery plugin it's a bit different, since there can be the icon between label and progressbar, and it adds spacing if displayed..)

Anyway, look'n'feel unification will be far simpler with a single monitor plugin providing the features of most existing monitor plugins. I'd prefer concentrate on that...
Comment 10 Harald Judt 2012-05-22 16:50:31 CEST
> (In reply to comment #8)
> > Created attachment 4452 
> > xfce4-systemload-plugin-spacing.patch
> > 
> > After some further investigation, I came to the conclusion that this is not
> > trivial to do - if not impossible - by setting the properties of the present
> > widgets. If you set a border on the labels, it will never look the same as
> > the other plugins because the border will be on all sides.
> 
> Yeah, but if you look at netload, the border is on all sides of the label. I
> (_personally_) think it's better ui-wise that if a label is displayed it
> shouldn't be 'stuck' with the corresponding progressbar.. battery & diskperf
> plugin have this 'problem' too. (well for battery plugin it's a bit
> different, since there can be the icon between label and progressbar, and it
> adds spacing if displayed..)

If you look at netload, the label has a small border on all sides of the label. Additionally, it has empty space on the left side of the label, which is apparently not a property of the label. As said, you can't replicate this for systemload because of the widget layout. My patch tries to fake this.

Perhaps as a further modification, you can still set a label border if you like to, and also for the other plugins, if you deem this good. Anyway, the patch I proposed here certainly improves the situation to my liking, whether you set a label border or not.

> Anyway, look'n'feel unification will be far simpler with a single monitor
> plugin providing the features of most existing monitor plugins. I'd prefer
> concentrate on that...

Yes, I hope we will see the new plugin some day, it would be great. The icons and meter mockup looks really promising. However, there is still the gtk2->gtk3 conversion pending, so nothing might happen before xfce core is finished. For the time being, I'd rather concentrate on what we have and try to improve what people use now, since we're currently stuck with it. I'm already quite satisfied with the enhancements that went into the plugins recently.
Comment 11 Landry Breuil editbugs 2012-05-22 18:49:06 CEST
Thinking more about it... the issue we have here (and in other plugins, similar pattern/code) is that the optional label is created/packed regardless of the option whether the user wants to see it.

There are two spacing/padding options : the containing box spacing, and the child padding when it's packed into the box, and both accumulate. In netload, the label has a 4px padding, and in diskperf/battery/systemload the label has no padding and hence 'sticks' to the next box child since the boxes have no spacing.

If the user wants to hide the label, the only thing done is hiding the label widget, but the label spacing stays and is wasting space. Another approach which i'll experiment would be to create/pack the label only if it's really enabled (and also destroy/recreate it in the settings dialog callbacks).

I'm not rejecting your patch, i acknowledge there's an ui issue here, but since there's a code pattern between plugins that can be improved i prefer finding a 'correct' solution for all of them, instead of adding an empty widget just to 'pad' another, while gtk itself provides the functions to do so.
Comment 12 Harald Judt 2012-05-22 20:21:02 CEST
(In reply to comment #11)
> Thinking more about it... the issue we have here (and in other
> plugins, similar pattern/code) is that the optional label is
> created/packed regardless of the option whether the user wants to
> see it.

That's one point to think about. A lot of plugins spread the code for
creating/recreating widgets over various functions. I find that this
hurts clarity and makes it harder to understand and analyse the code,
especially for one who has not dealt with the panel plugins before.

> There are two spacing/padding options : the containing box spacing,
> and the child padding when it's packed into the box, and both
> accumulate. In netload, the label has a 4px padding, and in
> diskperf/battery/systemload the label has no padding and hence
> 'sticks' to the next box child since the boxes have no spacing.

How about agreeing on a default padding size (->panel plugins HIG) and
simply adding more padding to the labels in the other plugins, and
use that for systemload too?

> If the user wants to hide the label, the only thing done is hiding
> the label widget, but the label spacing stays and is wasting
> space. Another approach which i'll experiment would be to
> create/pack the label only if it's really enabled (and also
> destroy/recreate it in the settings dialog callbacks).

It is possible to set the spacing, padding and packing order of a box
after creation. I've tried to do that with systemload but found it too
complicated because of the amount of possibilities to cater for. As already mentioned, it would be possible to simplify this by only allowing to
hide/show all monitor labels at once.

> I'm not rejecting your patch, i acknowledge there's an ui issue
> here, but since there's a code pattern between plugins that can be
> improved i prefer finding a 'correct' solution for all of them,
> instead of adding an empty widget just to 'pad' another, while gtk
> itself provides the functions to do so.

For simplicity's sake, let's ignore the systemload plugin first as it
is a special case because of its three-monitors-in-one-plugin
attribute, which makes things pretty complicated.

First, I agree that the labels should not 'stick' to the meter because
it does not look very good. But if a label has a small border, say
1-2px, between it and the meter and a larger border than those 1-2px
between it and *another plugin*, then this makes it easier for the
user to correlate the two widgets with each other. To make it short, I
find it nice that labels and their meters appear 'grouped', and I'd
rather not change that.

Now let's talk about code patterns. In general I think it would be of
advantage to let the plugins handle the following standard routines
similarly in specialized functions, and as demonstrated by the sample
plugin:

plugin_construct: Initialize plugin
    (call load settings functions, setup signals)
plugin_new: Create widgets
    (all widgets we could probably use)
plugin_setup: Change visibility, spatial distance, order and
    properties of widgets
    (called after creation and after changes in config dialog)
plugin_set_orientation: Change the orientation according to the panel
    (should also be called in new, setup)
plugin_set_size: Resize plugin and widgets
    (should also be called in new, setup)

This makes it very easy for a new coder to become acquainted with all
plugins very quickly and serves clarity. Widgets will only be created
and destroyed once, setup in a specific place, and so on. You can
still check the plugin options to see if you need to update a widget
or not depending on its visibility.

Of course, creating widgets unconditionally means you waste some
memory, but I think the gained simplicity/clarity is worthwhile,
since you can rely on the fact that the widgets are always there
and you won't have to re-/create and set them up somewhere else.

> [...] instead of adding an empty widget just to 'pad' another, while
> gtk itself provides the functions to do so.

AFAIK, it does not easily permit to specify different sizes for
left, right, top and bottom borders/padding, and it seemed to be
the easiest and simplest option to achieve what I had in mind.
If the framework limits you in such a way, I consider it legit to
work around that limits.
Comment 13 Landry Breuil editbugs 2012-05-23 08:09:53 CEST
(In reply to comment #12)
> First, I agree that the labels should not 'stick' to the meter because
> it does not look very good. But if a label has a small border, say
> 1-2px, between it and the meter and a larger border than those 1-2px
> between it and *another plugin*, then this makes it easier for the
> user to correlate the two widgets with each other. To make it short, I
> find it nice that labels and their meters appear 'grouped', and I'd
> rather not change that.

Most plugins already "separate themselves" from other plugins by calling gtk_container_set_border_width() on their main box, with various values (which is another thing the HIG could take care of).
Comment 14 Harald Judt 2012-05-23 16:23:51 CEST
(In reply to comment #13)
> Most plugins already "separate themselves" from other plugins by calling
> gtk_container_set_border_width() on their main box, with various values
> (which is another thing the HIG could take care of).

True. That's also the reason we cannot string together monitors from different plugins; if you set the border or the padding of the box to 0, then the progress meters will use all available space of the panel, which looks ugly. The padding won't help here, as it is only to separate child widgets.

(In reply to comment #11)
> If the user wants to hide the label, the only thing done is hiding the label
> widget, but the label spacing stays and is wasting space. Another approach
> which i'll experiment would be to create/pack the label only if it's really
> enabled (and also destroy/recreate it in the settings dialog callbacks).

I'm sure a label spacing does not stay when hiding the label. I modified the diskperf plugin to set a spacing on the label and remove the border. When I hid the label, I could string together several diskperf monitors, which means the spacing for the label must have been removed too. In case I'm mistaking, remember that you can still set the label spacing when hiding the label.

Maybe we can find good default values for a monitor plugin? I did experiment with the diskperf plugin. I think it would be fine to keep the following values for all panel sizes: box spacing=0, label spacing=2, progress bar size=8 (the way it is now), progress bar spacing=0
I'm not sure about the box border for different panel sizes. On smaller panel sizes, a lower value does not squeeze the progress bars that much, but on larger panels a lower value looks ugly. Wasting 4 pixel on a border on a 26px panel is obviously more detrimentous than on a 28px panel.

On this page, several values are mentioned, but no consent was found.
http://wiki.xfce.org/dev/hig/panel-plugins/proposed

Here are examples for what I found to be reasonable values (panel size -> border size):
< 21 -> 1,
< 23 -> 2,
< 27 -> 3,
>= 27 -> 4
Perhaps this can be optimized, and maybe there's also a good formula for such things, however I don't know of one. So this or something similar could be made a function to be called by the various plugins to find a convenient border size.

> (which is another thing the HIG could take care of).
How do such ideas get propagated to the panel-plugins HIG? This looks a bit bureaucratical (taking a long time to be decided about)...
Comment 15 Landry Breuil editbugs 2012-05-25 21:34:31 CEST
Created attachment 4461 
plugin with/without labels with/without 2px spacing

Here's a screenshot with :
- on top, whats now in git, ie no spacing when packing the label (i've set the eventbox as visible to 'show' the widget size)
- at the bottom, labels packed with a 2px spacing, which would (at least for monitor labels, not the uptime which wastes space) "fix" the issue. I've put the no-label version with both, and you're right, the spacing is hidden with the label. If it looks ok to you, i'll make it consistent (ie pack the label with a 2px spacing) in other plugins.
Comment 16 Landry Breuil editbugs 2012-05-25 21:50:13 CEST
(In reply to comment #14)

> Maybe we can find good default values for a monitor plugin? I did experiment
> with the diskperf plugin. I think it would be fine to keep the following
> values for all panel sizes: box spacing=0, label spacing=2, progress bar
> size=8 (the way it is now), progress bar spacing=0
> I'm not sure about the box border for different panel sizes. On smaller
> panel sizes, a lower value does not squeeze the progress bars that much, but
> on larger panels a lower value looks ugly. Wasting 4 pixel on a border on a
> 26px panel is obviously more detrimentous than on a 28px panel.
> 
> On this page, several values are mentioned, but no consent was found.
> http://wiki.xfce.org/dev/hig/panel-plugins/proposed

No consent was found because so far there hasnt been a lot of discussion on
that matter on the ml since a while. The HIG page was started a while ago, then was
dormant, i tried to 'revive' the discussion about it (http://mail.xfce.org/pipermail/xfce4-dev/2012-May/029834.html), andrezj tried too (http://mail.xfce.org/pipermail/xfce4-dev/2012-May/029841.html) on the ML,but not much discussion happened. That's all.

> Here are examples for what I found to be reasonable values (panel size ->
> border size):
> < 21 -> 1,
> < 23 -> 2,
> < 27 -> 3,
> >= 27 -> 4

Adapting the border size depending on the panel size ? That's a good way to ensure
there will be no consistency across plugins, because the (several) unmaintained ones
will not do it :) The idea itself is nice, but i'm not sure the end user wants space
to be "wasted" by the border if he makes the panel bigger. After all if he makes the
panel bigger that's to have bigger plugins, no ?

> > (which is another thing the HIG could take care of).
> How do such ideas get propagated to the panel-plugins HIG? This looks a bit
> bureaucratical (taking a long time to be decided about)...

Noone works on it. Discussing things here in the bugzilla is a first step, but all
this would be better on the mailing list. Once there's some kind of consensus,
someone updates the HIG page on the wiki, and then 'someone' updates the plugins
to follow the HIG. Note that i'm not a designer nor UI expert, i prefer fixing
bugs and coding, instead of talking for hours in bugzilla comments, which kills
a bit of the fun.
Comment 17 Harald Judt 2012-05-26 10:35:39 CEST
(In reply to comment #15)
> Created attachment 4461 
> plugin with/without labels with/without 2px spacing
> 
> Here's a screenshot with :
> - on top, whats now in git, ie no spacing when packing the label (i've set
> the eventbox as visible to 'show' the widget size)
> - at the bottom, labels packed with a 2px spacing, which would (at least for
> monitor labels, not the uptime which wastes space) "fix" the issue. I've put
> the no-label version with both, and you're right, the spacing is hidden with
> the label. If it looks ok to you, i'll make it consistent (ie pack the label
> with a 2px spacing) in other plugins.

Looks fine for me, and probably as good as it can get. Even if we can't make them all look consistent, it's a good start. I've a patch for diskperf plugin in store solving exactly this, so you can save your time fixing the others ;-)

(In reply to comment #16)
> No consent was found because so far there hasnt been a lot of discussion on
> that matter on the ml since a while. The HIG page was started a while ago,
> then was
> dormant, i tried to 'revive' the discussion about it
> (http://mail.xfce.org/pipermail/xfce4-dev/2012-May/029834.html), andrezj
> tried too (http://mail.xfce.org/pipermail/xfce4-dev/2012-May/029841.html) on
> the ML,but not much discussion happened. That's all.

I'll try to summarize and write a mail about what we discussed here and other things to the discussion list. Maybe if enough people join in, something will happen.

> > Here are examples for what I found to be reasonable values (panel size ->
> > border size):
> > < 21 -> 1,
> > < 23 -> 2,
> > < 27 -> 3,
> > >= 27 -> 4
> 
> Adapting the border size depending on the panel size ? That's a good way to
> ensure
> there will be no consistency across plugins, because the (several)
> unmaintained ones
> will not do it :) The idea itself is nice, but i'm not sure the end user
> wants space
> to be "wasted" by the border if he makes the panel bigger. After all if he
> makes the
> panel bigger that's to have bigger plugins, no ?

Then those plugins need new maintainers, that is, we need to fix them ourselves. I don't see why whe should let them slow down development. Apparently, they don't have interest in improving their plugins. I've often seen them complain "we don't have time", so here are we to help them out.

I agree about what you say about bigger panels, but you have to admit that "long" meters with too small borders look really ugly, don't they? The relations do not fit in my opinion. Same with the small, squeezed ones. Besides, wasted pixels are what the user already gets currently. Also, we'd need to think about the borders (minimize them or do what?) of single-row plugins. But I guess this is another topic for the mailing list. I will send you a patch you can try where you can see the changes I proposed. It's always easier to decide about something when you can look at it.

> > > (which is another thing the HIG could take care of).
> > How do such ideas get propagated to the panel-plugins HIG? This looks a bit
> > bureaucratical (taking a long time to be decided about)...
> 
> Noone works on it. Discussing things here in the bugzilla is a first step,
> but all
> this would be better on the mailing list. Once there's some kind of
> consensus,
> someone updates the HIG page on the wiki, and then 'someone' updates the
> plugins
> to follow the HIG. Note that i'm not a designer nor UI expert, i prefer
> fixing
> bugs and coding, instead of talking for hours in bugzilla comments, which
> kills
> a bit of the fun.

I guess we are all no UI experts. But if enough people share the same opinion about the look & feel, I think we can't be that wrong? What is often missing is some sort of documentation of the reasoning behind certain decisions. That would save a lot of time and the need for users/developers discussing or asking the same things again and again.

Apart from that, I found the discussion quite constructive and thank you for the time you spent on it, though I confirm the mailing list would certainly be the better place for it.
Comment 18 Git Bot editbugs 2020-05-23 00:56:59 CEST
-- GitLab Migration Automatic Message --

This bug has been migrated to xfce.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.xfce.org/panel-plugins/xfce4-systemload-plugin/-/issues/1.

Please create an account or use an existing account on one of our supported OAuth providers. 

If you want to fork to submit patches and merge requests please continue reading here: https://docs.xfce.org/contribute/dev/git/start#gitlab_forks_and_merge_requests

Also feel free to reach out to us on the mailing list https://mail.xfce.org/mailman/listinfo/xfce4-dev

Bug #8928

Reported by:
Harald Judt
Reported on: 2012-05-21
Last modified on: 2020-05-23

People

Assignee:
Landry Breuil
CC List:
2 users

Version

Attachments

xfce4-systemload-plugin-spacing.patch (1.28 KB, patch)
2012-05-21 14:01 CEST , Harald Judt
no flags
screenshot.png (2.22 KB, image/png)
2012-05-21 22:45 CEST , Harald Judt
no flags
screenshot2.png (2.31 KB, image/png)
2012-05-21 22:49 CEST , Harald Judt
no flags
screenshot of monitors without labels (3.55 KB, image/png)
2012-05-22 09:54 CEST , Landry Breuil
no flags
xfce4-systemload-plugin-spacing.patch (2.29 KB, patch)
2012-05-22 13:41 CEST , Harald Judt
no flags
plugin with/without labels with/without 2px spacing (3.50 KB, image/png)
2012-05-25 21:34 CEST , Landry Breuil
no flags

Additional information