Re: [patch] making the applet stetic.

2005-06-20 Thread Jonathan Blandford
Robert Love [EMAIL PROTECTED] writes:

 As we all know, being stetic is of the utmost importance:
 
   http://primates.ximian.com/~peter/stetic.html
 
 The current NetworkManager applet is not as stetic as it deserves to be;
 it has potential, it dreams of a world in which it is the most beautiful
 applet ever, but it is not yet there.
 
 The progress bars immediately jump out as low-hanging fruit.
 
 The attached patch replaces the current gtkcellrenderprivate magic with
 Gtk Progress Bars--but not just any progress bars.  Progress bars that
 are dynamically sized based on the font size.  Beautiful, lovely, public
 progress bars.
 
 The benefits:
 
   - Follows the system theme.  The current Gtkcellrender thing
 is not correctly themed.
   - Uses a public API instead of the private in-tree gtk code.
   - Nicer looking size.
   - Dynamically-sized based on the current language and font
 ascent, to five ascents long and one high.  This elegant
 code is c/o Joey Shaw.
   - Net code reduction of -1532 lines, four files.
   - Oh, it is sooo stetic.
 
 The cons:
 
   (this list is empty)

Hi Robert,

I was on vacation when this email went by, but I'd like to see some of
this reverted.

First, a bit of history on GtkCellRendererProgress.  NetworkManager was
started against GTK+-2.4, before the cell renderer made it into GTK+
itself.  Since we were trying to get NM into FC3 at the time, we
cut-n-paste the code from gtk+-HEAD into NM.  It really should have been
conditionally compiled in a configure check based on the version of GTK+
you had, but apparently this didn't happen.  Versions of NM against
GTK+-2.6 and greater really shouldn't use that code at all.

Now for the rationale for using the cell progress bars.  The primary
purpose of the cell renderer is to provide a progress bar suitable for
rendering in menus and lists.  It is supposed to be 'flat', as opposed
to GtkProgressBar, which is supposed to be suitable for general
embedding.  Some themes draw GtkProgressBar in a flat manner, and thus
the progress looks fine in the menus.  However, themes such as Bluecurve
draw this widget beveled, which makes them look odd in a menu.
Bluecurve also draws the progress cell renderers badly.  However,
Clearlooks does a significantly better job, as it sets its x/y thickness
to 1.  I put a screenshot up here comparing the two:

http://www.gnome.org/~jrb/files/progress-comparison.png

There's no reason this progress bar should look so bad on other themes,
and we should fix the themes to make them look better.

There are a couple of other issues that you listed:

   - Follows the system theme.  The current Gtkcellrender thing
 is not correctly themed.

I'm not 100% sure what you mean by this?  Do you mean that it doesn't
honor theme changes?  Or is it getting some things wrong.

   - Dynamically-sized based on the current language and font
 ascent, to five ascents long and one high.  This elegant
 code is c/o Joey Shaw.

This code seems fine, and chould easily be propagated to the
CellRendererProgress.  If you want it to adjust in changes to the system
font or theme (which would be pretty cool), you should do it in a
style-set signal callback.

Thanks,
-Jonathan
___
NetworkManager-list mailing list
NetworkManager-list@gnome.org
http://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [patch] making the applet stetic.

2005-06-20 Thread Dan Williams
On Mon, 2005-06-20 at 13:37 -0400, Jonathan Blandford wrote:
 Robert Love [EMAIL PROTECTED] writes:
 There are a couple of other issues that you listed:
 
  - Follows the system theme.  The current Gtkcellrender thing
is not correctly themed.
 
 I'm not 100% sure what you mean by this?  Do you mean that it doesn't
 honor theme changes?  Or is it getting some things wrong.

The problem here was that the initial menu items were subclasses of the
GtkMenuItem and therefore, when the theme changed, they didn't pick up
the new GtkStyle due to inherent gtkrc (subclasses have a different
style path/name or whatever which means they don't match anything in the
gtkrc).  See:

http://bugzilla.gnome.org/show_bug.cgi?id=142417

The workarounds suggested by Owen either aren't sufficient or were just
plain ugly, since theme writers need to modify their theme rc files to
support what we are trying to do with the subclasses of GtkMenuItem.
Which sucks.

Dan

___
NetworkManager-list mailing list
NetworkManager-list@gnome.org
http://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [patch] making the applet stetic.

2005-06-20 Thread Robert Love
On Mon, 2005-06-20 at 13:37 -0400, Jonathan Blandford wrote:

Hi, jrb.

 Now for the rationale for using the cell progress bars.  The primary
 purpose of the cell renderer is to provide a progress bar suitable for
 rendering in menus and lists.  It is supposed to be 'flat', as opposed
 to GtkProgressBar, which is supposed to be suitable for general
 embedding.  Some themes draw GtkProgressBar in a flat manner, and thus
 the progress looks fine in the menus.  However, themes such as Bluecurve
 draw this widget beveled, which makes them look odd in a menu.
 Bluecurve also draws the progress cell renderers badly.  However,
 Clearlooks does a significantly better job, as it sets its x/y thickness
 to 1.  I put a screenshot up here comparing the two:
 
 http://www.gnome.org/~jrb/files/progress-comparison.png
 
 There's no reason this progress bar should look so bad on other themes,
 and we should fix the themes to make them look better.

I'm not totally following the rationale why gtkrendercellprogressfoo is
better.  Simply that it is (supposed) to look better embedded in menus?
Is that really a good enough reason for all of the code?

For what its worth, the gtkrendercell things look awful in Industrial
and whatever Ubuntu uses, compared to Gtk Progress Bars.

 There are a couple of other issues that you listed:
 
  - Follows the system theme.  The current Gtkcellrender thing
is not correctly themed.
 
 I'm not 100% sure what you mean by this?  Do you mean that it doesn't
 honor theme changes?  Or is it getting some things wrong.

Hm, maybe it was my test themes, but it didn't follow the rest of the
theme.  It was the wrong color.

It also just looks generally uglier to me, but that is objective.

  - Dynamically-sized based on the current language and font
ascent, to five ascents long and one high.  This elegant
code is c/o Joey Shaw.
 
 This code seems fine, and chould easily be propagated to the
 CellRendererProgress.  If you want it to adjust in changes to the system
 font or theme (which would be pretty cool), you should do it in a
 style-set signal callback.

Yah, I need to grab the signal.  I meant to add a TODO item since (at
the time) I did not know the name of the signal.  Thanks!  ;-)

Robert Love


___
NetworkManager-list mailing list
NetworkManager-list@gnome.org
http://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [patch] making the applet stetic.

2005-06-20 Thread Jonathan Blandford
Dan Williams [EMAIL PROTECTED] writes:

 On Mon, 2005-06-20 at 13:37 -0400, Jonathan Blandford wrote:
  Robert Love [EMAIL PROTECTED] writes:
  There are a couple of other issues that you listed:
  
 - Follows the system theme.  The current Gtkcellrender thing
   is not correctly themed.
  
  I'm not 100% sure what you mean by this?  Do you mean that it doesn't
  honor theme changes?  Or is it getting some things wrong.
 
 The problem here was that the initial menu items were subclasses of the
 GtkMenuItem and therefore, when the theme changed, they didn't pick up
 the new GtkStyle due to inherent gtkrc (subclasses have a different
 style path/name or whatever which means they don't match anything in the
 gtkrc).  See:
 
 http://bugzilla.gnome.org/show_bug.cgi?id=142417
 
 The workarounds suggested by Owen either aren't sufficient or were just
 plain ugly, since theme writers need to modify their theme rc files to
 support what we are trying to do with the subclasses of GtkMenuItem.
 Which sucks.

That problem will affect the progress bar too, though.  And it looked
like you removed the subclasses already.  Is this still an issue?

Thanks,
-Jonathan
___
NetworkManager-list mailing list
NetworkManager-list@gnome.org
http://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [patch] making the applet stetic.

2005-06-20 Thread Dan Williams
On Mon, 2005-06-20 at 14:49 -0400, Jonathan Blandford wrote:
 Dan Williams [EMAIL PROTECTED] writes:
 
  On Mon, 2005-06-20 at 13:37 -0400, Jonathan Blandford wrote:
   Robert Love [EMAIL PROTECTED] writes:
   There are a couple of other issues that you listed:
   
- Follows the system theme.  The current Gtkcellrender thing
  is not correctly themed.
   
   I'm not 100% sure what you mean by this?  Do you mean that it doesn't
   honor theme changes?  Or is it getting some things wrong.
  
  The problem here was that the initial menu items were subclasses of the
  GtkMenuItem and therefore, when the theme changed, they didn't pick up
  the new GtkStyle due to inherent gtkrc (subclasses have a different
  style path/name or whatever which means they don't match anything in the
  gtkrc).  See:
  
  http://bugzilla.gnome.org/show_bug.cgi?id=142417
  
  The workarounds suggested by Owen either aren't sufficient or were just
  plain ugly, since theme writers need to modify their theme rc files to
  support what we are trying to do with the subclasses of GtkMenuItem.
  Which sucks.
 
 That problem will affect the progress bar too, though.  And it looked
 like you removed the subclasses already.  Is this still an issue?

You're right, I did kill them (I converted the subclasses to private
structs).  So I'm unsure how much of an issue it is...

Dan

___
NetworkManager-list mailing list
NetworkManager-list@gnome.org
http://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [patch] making the applet stetic.

2005-06-20 Thread Jonathan Blandford
Robert Love [EMAIL PROTECTED] writes:

 I'm not totally following the rationale why gtkrendercellprogressfoo is
 better.  Simply that it is (supposed) to look better embedded in menus?
 Is that really a good enough reason for all of the code?

It is supposed to look better for menus and treeviews.  'All that code'
is misleading because it should only be compiled if you are building it
against gtk+-2.4 .  If you have newer versions of GTK+, you should be
able to remove it from compilation altogether.  At some point[1], NM is
going to require some newer feature from GTK+, and we can drop it then.

 For what its worth, the gtkrendercell things look awful in Industrial
 and whatever Ubuntu uses, compared to Gtk Progress Bars.

Does it look better if you change the x/y thickness in those themes for
GtkCellView?  I just filed 308428 and 308429 for these issues.  There
really should be no reason a theme engine can't make these draw
basically identically.

 It also just looks generally uglier to me, but that is objective.

'objective' or 'subjective'? (-;  

I agree that the thick black-line looks bad with the progress renderer,
but the bevel looks equally awful.  This is clearly a GTK+ issue and I'm
raising it there.  I would prefer to use the right object in the right
place though, so we can fix it for all applications.

Thanks,
-Jonathan

[1] As is inevitable with all projects...
___
NetworkManager-list mailing list
NetworkManager-list@gnome.org
http://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [patch] making the applet stetic.

2005-06-16 Thread Bryan Clark
 Compare yourself.  Current applet:
 
   http://rlove.org/images/nm-applet-1.jpg
 
 On the road to being stetic applet:
 
   http://rlove.org/images/networkmanager-progress-bar-20050615.png
 

Nice.  I like the rearranging of the secure icon.

Peace,
~ Bryan

___
NetworkManager-list mailing list
NetworkManager-list@gnome.org
http://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [patch] making the applet stetic.

2005-06-16 Thread Dan Williams
On Thu, 2005-06-16 at 13:34 -0400, Robert Love wrote:
 As we all know, being stetic is of the utmost importance:
 
   http://primates.ximian.com/~peter/stetic.html
 
 The current NetworkManager applet is not as stetic as it deserves to be;
 it has potential, it dreams of a world in which it is the most beautiful
 applet ever, but it is not yet there.
 
 The progress bars immediately jump out as low-hanging fruit.
 
 The attached patch replaces the current gtkcellrenderprivate magic with
 Gtk Progress Bars--but not just any progress bars.  Progress bars that
 are dynamically sized based on the font size.  Beautiful, lovely, public
 progress bars.
 
 The benefits:
 
   - Follows the system theme.  The current Gtkcellrender thing
 is not correctly themed.
   - Uses a public API instead of the private in-tree gtk code.
   - Nicer looking size.
   - Dynamically-sized based on the current language and font
 ascent, to five ascents long and one high.  This elegant
 code is c/o Joey Shaw.
   - Net code reduction of -1532 lines, four files.
   - Oh, it is sooo stetic.
 

Applied, thanks.  Brian also suggested making the progress bars not as
tall, so I've made them (ascent / 2).

Dan

___
NetworkManager-list mailing list
NetworkManager-list@gnome.org
http://mail.gnome.org/mailman/listinfo/networkmanager-list


Re: [patch] making the applet stetic.

2005-06-16 Thread Robert Love
On Thu, 2005-06-16 at 14:49 -0400, Dan Williams wrote:

 Applied, thanks.  Brian also suggested making the progress bars not as
 tall, so I've made them (ascent / 2).

Thanks, Dan.

Well, I would take a bullet (or at least some sort of blunt flying
projectile) for my neighbor Brian, but I think that ascent/2 is too
small.  Personally.  It is a personal thing.  Nothing against Brian, but
half is too small.

If ascent is too big, another option is to pass -1 and have the
widget scaled to the size of the other widgets.  That might be a bit
smaller, depending on your font, but not as big as ascent.

A patch doing this is attached.

If not, the comment needs to be adjusted. ;-)

Robert Love


 menu-items.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

Index: gnome/applet/menu-items.c
===
RCS file: /cvs/gnome/NetworkManager/gnome/applet/menu-items.c,v
retrieving revision 1.3
diff -u -u -r1.3 menu-items.c
--- gnome/applet/menu-items.c   16 Jun 2005 18:47:56 -  1.3
+++ gnome/applet/menu-items.c   16 Jun 2005 20:29:18 -
@@ -216,8 +216,8 @@
ascent = pango_font_metrics_get_ascent (metrics) * 1.5 / PANGO_SCALE;
pango_font_metrics_unref (metrics);
 
-   /* size our progress bar to be five ascents long, one high */
-   gtk_widget_set_size_request (item-progress, ascent * 5, ascent / 2);
+   /* size our progress bar to be five ascents long */
+   gtk_widget_set_size_request (item-progress, ascent * 5, -1);
 
gtk_box_pack_end (GTK_BOX (hbox), item-progress, FALSE, TRUE, 0);
 
___
NetworkManager-list mailing list
NetworkManager-list@gnome.org
http://mail.gnome.org/mailman/listinfo/networkmanager-list