Opened 10 years ago

Closed 7 years ago

#8727 closed patch (fixed)

Small close buttons - ui improvement

Reported by: karvanitis Owned by:
Milestone: 3.0.0 Component: pidgin (gtk)
Version: 2.5.5 Keywords: close button widget
Cc: elreydetodo, Paradox

Description

The following patch implements a small Gtk compatible close button widget throughout the application where previously a Gtk label was attempted to be used for similar behaviour.

This alleviates pidgin from having to handle the hit test as well as rendering such as the way the previous button was implemented. This behaviour is now handled by the native Gtk button widget.

As a result, this also allows for a consistent look and feel throughout the pidgin application as well as with other Gtk apps. Simply speaking it looks and feels better.

See attached screenshots.

Attachments (6)

gtk_small_button.patch (12.6 KB) - added by karvanitis 10 years ago.
gtk small button patch
Screenshot-Conv.png (19.0 KB) - added by karvanitis 10 years ago.
conv window screen shot
Screenshot-Buddy_List.png (17.4 KB) - added by karvanitis 10 years ago.
buddy list window screenshot
Screenshot-Conv_Windows.png (18.4 KB) - added by karvanitis 10 years ago.
conv window screenshot on windows xp
gtkscrollbook.c.patch (4.4 KB) - added by karvanitis 10 years ago.
small button patch to scrollbook ui
NewButtons.png (48.5 KB) - added by QuLogic 9 years ago.
Incorrect size in new buttons

Download all attachments as: .zip

Change History (28)

Changed 10 years ago by karvanitis

gtk small button patch

Changed 10 years ago by karvanitis

conv window screen shot

Changed 10 years ago by karvanitis

buddy list window screenshot

Changed 10 years ago by karvanitis

conv window screenshot on windows xp

comment:1 Changed 10 years ago by rekkanoryo

  • Milestone set to Patches Needing Review

This patch can't be accepted until 3.0.0, assuming anyone wants it, because it modifies public API.

comment:2 Changed 10 years ago by karvanitis

I guess it goes without saying that you may assume I "want it" - seeing as I took the time to write the patch and then submit it to you.

comment:3 Changed 10 years ago by rekkanoryo

By "assuming anyone wants it," I meant Pidgin developers.

Changed 10 years ago by karvanitis

small button patch to scrollbook ui

comment:4 follow-ups: Changed 10 years ago by elb

I don't see how this modifies public API; it could go into 2.6.0, right?

Is there a reason we didn't do this before? Are some of the gtk+ functions involved more recent than 2.0.0? Do we need a compatability fallback for older systems?

comment:5 in reply to: ↑ 4 Changed 10 years ago by rekkanoryo

Replying to elb:

I don't see how this modifies public API; it could go into 2.6.0, right?

Is there a reason we didn't do this before? Are some of the gtk+ functions involved more recent than 2.0.0? Do we need a compatability fallback for older systems?


The first patch modifies a public struct in gtkblist.h by changing a GdkPixbuf * to a GtkWidget *.

comment:6 in reply to: ↑ 4 Changed 10 years ago by karvanitis

Replying to elb:

I don't see how this modifies public API; it could go into 2.6.0, right?

Is there a reason we didn't do this before? Are some of the gtk+ functions involved more recent than 2.0.0? Do we need a compatability fallback for older systems?

As far as I know, none of the functions used are more recent 2.0.0.

It is rather unfortunate the it modifies public api since I believe the look and feel of these buttons is a rather nice improvement over the previous implementation (to which I can not speak towards).

comment:7 Changed 10 years ago by rekkanoryo

  • Milestone changed from Patches Needing Review to 3.0.0

As I noted above, this patch cannot be accepted until 3.0.0.

comment:8 Changed 9 years ago by QuLogic

So we don't really use that GdkPixbuf for much, really. Also, we don't really need the GtkWidget that's for the button after it's created. Is it possible to just leave in the image creation (and struct entry) until 3.0.0 and use the button now? Since we're going for 2.7.0, we can add that new API that's in the patch.

comment:9 Changed 9 years ago by QuLogic

  • Status changed from new to pending

What theme are you using? I'll assume the Human theme for Ubuntu and that fake-Windows one on Windows? It doesn't seem to look right with ClearLooks?. Unless I've patched it wrong, there still appears to be padding on the button and it makes the headline and conversation tabs bigger. Also, the minidialog still has the old button. I've attached a picture of old and new.

Some of the code has moved around and the patch doesn't apply cleanly. Can you see about re-creating it with the current code?

Changed 9 years ago by QuLogic

Incorrect size in new buttons

comment:10 Changed 9 years ago by QuLogic

Also, if you check out Firefox, they somehow managed to get it even smaller than what we have (though the image in our tab may be bigger.)

comment:11 Changed 9 years ago by Paradox

Looking at gedit and nautilus, they use GTK's RC Styles to remove extra padding.

style "tab-close-button-style"
{
  GtkWidget::focus-padding = 0
  GtkWidget::focus-line-width = 0
  xthickness = 0
  ythickness = 0
}

comment:12 Changed 9 years ago by karvanitis@…

(In 6d79836ef57805a40b20c46467e6a4595605d195):
Use a small GtkButton? instead of the custom "X" for close in various places in the Buddy List and conversations. The minidialog still uses the X, but it's a little different, anyway.

Refs #8727.

comment:13 Changed 9 years ago by qulogic@…

(In 2201ce7ce7e248c26f7eac80d2097135fbfdeb1b):
Maintain ABI compatibility by keeping headline_close a GdkPixbuf?. Seperate commit for easy disapproval when 3.0.0 comes around.

Refs #8727.

comment:14 Changed 9 years ago by qulogic@…

(In dfc2a97552bcd3b5317a110f55464a2241eab965):
Use an inline RC style to get rid of some other padding to make the small buttons even smaller, as suggested by Paradox on trac, and nicked from nautilus.

Refs #8727.

comment:15 Changed 9 years ago by QuLogic

Now what do we even use the buttons in the scrollbook for? I don't see them ever. Do I even need to bother changing those?

comment:16 Changed 9 years ago by karvanitis

  • Status changed from pending to new

I would change it everywhere there is a little 'x' instead of a small GtkButton?. There is no sense in maintaining your own hit test, etc.. code when that's exactly what a button is for. Further to that the GtkButton? simply more aesthetically pleasing.

Changing the padding attributes is up to you, I didnt mind the buttons with the padding but its a subjective matter really.

comment:17 Changed 9 years ago by rekkanoryo

The minidialog is intentionally small. As far as I'm aware there is no theme that provides a small enough image for use in the minidialog's close button. The tiny X there is sufficient.

QuLogic: What buttons in the scrollbook?

comment:18 Changed 9 years ago by karvanitis

I would respectfully disagree.

comment:19 Changed 9 years ago by sadrul

The rc-style doesn't help reduce the size of the buttons, and I think the new buttons look ugly. I would much prefer going back to the little 'x' labels. Any technical reasons that shouldn't be done?

comment:20 Changed 9 years ago by sadrul@…

(In 2dffa8ba1bee1171684d1f2eda5b5ba307787274):
Use cute little "×" on the close buttons in conversation tabs.

The stock icons make the tabs too large, and they look huuge, compared to the status icon and the text on the tab, even with all the style-editing to remove borders etc. We still use 'buttons', instead of event-boxes, so we don't have to capture mouse-events and do mouseover/mouseout effects ourselves. This change simply removes the use of the stock icon and uses a "×" label in the button. This looks and feels betterer. Refs #8727.

comment:21 Changed 7 years ago by QuLogic

  • Status changed from new to pending

I'm not sure there's anything more to add here, is there? I don't quite remember, but I don't think there is.

comment:22 Changed 7 years ago by salinasv

  • Resolution set to fixed
  • Status changed from pending to closed

Looks like "pending" is not working.

Note: See TracTickets for help on using tickets.
All information, including names and email addresses, entered onto this website or sent to mailing lists affiliated with this website will be public. Do not post confidential information, especially passwords!