[Patch] Add xfce_image_menu_item_new to libxfce4ui

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[Patch] Add xfce_image_menu_item_new to libxfce4ui

Florian Weigelt
Please find attached a patch which adds some functionality to libxfce4ui
to use a GtkMenuItem with an icon without using the (in gtk3) deprecated
GtkImageMenuItem.
The code is based on the example given here:
https://developer.gnome.org/gtk3/stable/GtkImageMenuItem.html

This function is not intended to be used for creating menu bars. Using
GtkBuilder with GMenu (which actually has an icon property) is by far
better suited for this purpose.
However, applications like xfdesktop and xfce4-panel also use
GtkImageMenuItem's and it would be nice to offer the same functionality
in a gtk3 port of these applications.

I decided on those three parameters because those were the three
information usually available when I looked at xfdesktop's sources. It
is also possible to let the user create a GtkLabel first and pass this
widget to the function.
It would also be possible to exclude this function entirely from the
gtk2 build of libxfce4ui where GtkImageMenuItems weren't deprecated.
Please share your thoughts on this.

Kind regards

_______________________________________________
Xfce4-dev mailing list
[hidden email]
https://mail.xfce.org/mailman/listinfo/xfce4-dev

libxfce4ui-add-imagemenuitem.patch (3K) Download Attachment
signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Patch] Add xfce_image_menu_item_new to libxfce4ui

Igor Zakharov
Hi,
 
The patch looks good to me, and I'd like to have it merged.
Ristretto is also using GtkImageMenuItem, and this proposed functionality could be used in the gtk3 port.
 
Igor
 
04.02.2017, 04:59, "Florian Weigelt" <[hidden email]>:

Please find attached a patch which adds some functionality to libxfce4ui
to use a GtkMenuItem with an icon without using the (in gtk3) deprecated
GtkImageMenuItem.
The code is based on the example given here:
https://developer.gnome.org/gtk3/stable/GtkImageMenuItem.html

This function is not intended to be used for creating menu bars. Using
GtkBuilder with GMenu (which actually has an icon property) is by far
better suited for this purpose.
However, applications like xfdesktop and xfce4-panel also use
GtkImageMenuItem's and it would be nice to offer the same functionality
in a gtk3 port of these applications.

I decided on those three parameters because those were the three
information usually available when I looked at xfdesktop's sources. It
is also possible to let the user create a GtkLabel first and pass this
widget to the function.
It would also be possible to exclude this function entirely from the
gtk2 build of libxfce4ui where GtkImageMenuItems weren't deprecated.
Please share your thoughts on this.

Kind regards

,

_______________________________________________
Xfce4-dev mailing list
[hidden email]
https://mail.xfce.org/mailman/listinfo/xfce4-dev


_______________________________________________
Xfce4-dev mailing list
[hidden email]
https://mail.xfce.org/mailman/listinfo/xfce4-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Patch] Add xfce_image_menu_item_new to libxfce4ui

Simon Steinbeiss
Hi Florian,

without having reviewed the patch (although if it's based on upstream anyway, it should be fine) I agree we may want/need something like this.

So far the approach we've taken in other components was to wrap all gtk_image_menu_items in ignore deprecation statements (e.g. xfpm). I wouldn't mind using libxfce4ui instead, as it would result in cleaner code.

I don't think you have to worry much about Gtk2, it's not our goal to have a cross-compatible desktop anyway, we don't even seem to have enough manpower for one toolkit version alone...

Thanks for your efforts
Simon




Igor Zakharov <[hidden email]> schrieb am Sa., 4. Feb. 2017, 09:01:
Hi,
 
The patch looks good to me, and I'd like to have it merged.
Ristretto is also using GtkImageMenuItem, and this proposed functionality could be used in the gtk3 port.
 
Igor
 
04.02.2017, 04:59, "Florian Weigelt" <[hidden email]>:

Please find attached a patch which adds some functionality to libxfce4ui
to use a GtkMenuItem with an icon without using the (in gtk3) deprecated
GtkImageMenuItem.
The code is based on the example given here:
https://developer.gnome.org/gtk3/stable/GtkImageMenuItem.html

This function is not intended to be used for creating menu bars. Using
GtkBuilder with GMenu (which actually has an icon property) is by far
better suited for this purpose.
However, applications like xfdesktop and xfce4-panel also use
GtkImageMenuItem's and it would be nice to offer the same functionality
in a gtk3 port of these applications.

I decided on those three parameters because those were the three
information usually available when I looked at xfdesktop's sources. It
is also possible to let the user create a GtkLabel first and pass this
widget to the function.
It would also be possible to exclude this function entirely from the
gtk2 build of libxfce4ui where GtkImageMenuItems weren't deprecated.
Please share your thoughts on this.

Kind regards

,

_______________________________________________
Xfce4-dev mailing list
[hidden email]
https://mail.xfce.org/mailman/listinfo/xfce4-dev

_______________________________________________
Xfce4-dev mailing list
[hidden email]
https://mail.xfce.org/mailman/listinfo/xfce4-dev

_______________________________________________
Xfce4-dev mailing list
[hidden email]
https://mail.xfce.org/mailman/listinfo/xfce4-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Patch] Add xfce_image_menu_item_new to libxfce4ui

André Miranda
This function would be very helpful, as we have lots of menu items and IMHO dropping icons is not desired in the porting process.
Just a couple questions/requests:
* An option to create the icon from a string:
  xfce_image_menu_item_new_with_icon ("About", TRUE, "gtk-about", GTK_ICON_SIZE_MENU);
* What about accelerators? I can use gtk_widget_add_accelerator with the returned GtkMenuItem, but the label is required to use gtk_accel_label_set_accel_widget. Creating a label and adding it to menu_item defeats the purpose of this function.
* Is it possible to remove the ugly left pad of menu? (See attachment).

Cheers,
André miranda

On 02/04/2017 08:32 PM, Simon Steinbeiss wrote:
Hi Florian,

without having reviewed the patch (although if it's based on upstream anyway, it should be fine) I agree we may want/need something like this.

So far the approach we've taken in other components was to wrap all gtk_image_menu_items in ignore deprecation statements (e.g. xfpm). I wouldn't mind using libxfce4ui instead, as it would result in cleaner code.

I don't think you have to worry much about Gtk2, it's not our goal to have a cross-compatible desktop anyway, we don't even seem to have enough manpower for one toolkit version alone...

Thanks for your efforts
Simon




Igor Zakharov <[hidden email]> schrieb am Sa., 4. Feb. 2017, 09:01:
Hi,
 
The patch looks good to me, and I'd like to have it merged.
Ristretto is also using GtkImageMenuItem, and this proposed functionality could be used in the gtk3 port.
 
Igor
 
04.02.2017, 04:59, "Florian Weigelt" <[hidden email]>:

Please find attached a patch which adds some functionality to libxfce4ui
to use a GtkMenuItem with an icon without using the (in gtk3) deprecated
GtkImageMenuItem.
The code is based on the example given here:
https://developer.gnome.org/gtk3/stable/GtkImageMenuItem.html

This function is not intended to be used for creating menu bars. Using
GtkBuilder with GMenu (which actually has an icon property) is by far
better suited for this purpose.
However, applications like xfdesktop and xfce4-panel also use
GtkImageMenuItem's and it would be nice to offer the same functionality
in a gtk3 port of these applications.

I decided on those three parameters because those were the three
information usually available when I looked at xfdesktop's sources. It
is also possible to let the user create a GtkLabel first and pass this
widget to the function.
It would also be possible to exclude this function entirely from the
gtk2 build of libxfce4ui where GtkImageMenuItems weren't deprecated.
Please share your thoughts on this.

Kind regards

,

_______________________________________________
Xfce4-dev mailing list
[hidden email]
https://mail.xfce.org/mailman/listinfo/xfce4-dev

_______________________________________________
Xfce4-dev mailing list
[hidden email]
https://mail.xfce.org/mailman/listinfo/xfce4-dev


_______________________________________________
Xfce4-dev mailing list
[hidden email]
https://mail.xfce.org/mailman/listinfo/xfce4-dev


_______________________________________________
Xfce4-dev mailing list
[hidden email]
https://mail.xfce.org/mailman/listinfo/xfce4-dev

menu.png (12K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Patch] Add xfce_image_menu_item_new to libxfce4ui

Florian Weigelt
Thanks for your feedback.

> This function would be very helpful, as we have lots of menu items and
> IMHO dropping icons is not desired in the porting process.
> Just a couple questions/requests:
> * An option to create the icon from a string:
>   xfce_image_menu_item_new_with_icon ("About", TRUE, "gtk-about",
> GTK_ICON_SIZE_MENU);
Changing the function header to what you propose (icon name and icon
size instead of a pre-created GtkImage) is possible, but less flexible.
Images in menus could also be created from a pixbuf. We could also offer
a generic xfce_image_menu_item_new_from_label_and_image and a
specialized xfce_image_menu_item_new_from_label_and_icon_name (the names
here are just to reflect the function headers). I am not sure: Do we
need the generic version? Do we have cases where we create a
GtkImageMenuItem from something different than an icon name? If not, we
can change the function to what you proposed.
> * What about accelerators? I can use gtk_widget_add_accelerator with the
> returned GtkMenuItem, but the label is required to use
> gtk_accel_label_set_accel_widget. Creating a label and adding it to
> menu_item defeats the purpose of this function.
This function is intended to be used for small popup menus, like opening
the application menu in xfdesktop or a popup menu on panel plugins. I
don't see AccelLabels there, only in menu bars. For menu bars I highly
recommend using GMenu. AccelLabels are supported there.
The documentation for GtkImageMenuItem shows how accelerators can be
added to the approach of mimicking an GtkImageMenuItem with a box, an
image and a label. If it is *really* neccessary I will add it, but
currently I do not see the need for adding complexity to support a
feature that, to my knowledge, is not used.
> * Is it possible to remove the ugly left pad of menu? (See attachment).
Short answer: Yes. Why shouldn't it? But I will have to look into it
where the space is coming from.



_______________________________________________
Xfce4-dev mailing list
[hidden email]
https://mail.xfce.org/mailman/listinfo/xfce4-dev

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Patch] Add xfce_image_menu_item_new to libxfce4ui

Matthew Brush
On 2017-02-05 09:27 AM, Florian Weigelt wrote:

> Thanks for your feedback.
>
>> This function would be very helpful, as we have lots of menu items and
>> IMHO dropping icons is not desired in the porting process.
>> Just a couple questions/requests:
>> * An option to create the icon from a string:
>>   xfce_image_menu_item_new_with_icon ("About", TRUE, "gtk-about",
>> GTK_ICON_SIZE_MENU);
> Changing the function header to what you propose (icon name and icon
> size instead of a pre-created GtkImage) is possible, but less flexible.
> Images in menus could also be created from a pixbuf. We could also offer
> a generic xfce_image_menu_item_new_from_label_and_image and a
> specialized xfce_image_menu_item_new_from_label_and_icon_name (the names
> here are just to reflect the function headers). I am not sure: Do we
> need the generic version? Do we have cases where we create a
> GtkImageMenuItem from something different than an icon name? If not, we
> can change the function to what you proposed.
>> * What about accelerators? I can use gtk_widget_add_accelerator with the
>> returned GtkMenuItem, but the label is required to use
>> gtk_accel_label_set_accel_widget. Creating a label and adding it to
>> menu_item defeats the purpose of this function.
> This function is intended to be used for small popup menus, like opening
> the application menu in xfdesktop or a popup menu on panel plugins. I
> don't see AccelLabels there, only in menu bars. For menu bars I highly
> recommend using GMenu. AccelLabels are supported there.
> The documentation for GtkImageMenuItem shows how accelerators can be
> added to the approach of mimicking an GtkImageMenuItem with a box, an
> image and a label. If it is *really* neccessary I will add it, but
> currently I do not see the need for adding complexity to support a
> feature that, to my knowledge, is not used.
>> * Is it possible to remove the ugly left pad of menu? (See attachment).
> Short answer: Yes. Why shouldn't it? But I will have to look into it
> where the space is coming from.
>

Hi,

Out of curiosity, have you considered just copying the code of
gtkimagemenuitem.[ch] into libxfce4ui and changing the symbol name
prefix? Assuming it's not too intertangled with other modules, it should
be pretty easy and the licenses (and even coding conventions) are
compatible.

The main advantage here is not requiring much changes to applications
using the gtk_ versions, and avoiding the bug shown by André and which
happens in GMenu where the icon is pushed to the right instead of going
where the checkbox/radiobutton goes for other item kinds. Additionally
it could be added to the Glade catalog to allow applications to use
Glade still, which is not supported by your function or GMenu (last time
I checked).

Regards,
Matthew Brush
_______________________________________________
Xfce4-dev mailing list
[hidden email]
https://mail.xfce.org/mailman/listinfo/xfce4-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Patch] Add xfce_image_menu_item_new to libxfce4ui

Florian Weigelt
Copying the files from gtk directly is possible, but overkill in my
opinion. I shortly thought of it, but it adds a lot of maintenance work,
too. I'd rather spend my time fixing the bugs in my provided function
than maintaining a defacto fork of GtkImageMenuItem (which uses the
deprecated stock items and gtkiconfactory). It makes nearly no
difference effort-wise for the programmer to use this function (maybe
changed to Andre's proposal) compared to the rebranded
gtk_image_menu_item_new_* from the copy. We don't use glade for the
(programatically created) menus I intended this function to be used for.
For the menu bars: Yes, glade does NOT support the menu syntax. But it
is still possible to manually write a .ui file with the menu syntax. As
for GMenu displaying the icon on the right: I never saw this happen.
Your points are all valid, but I think we can go with a less complex
solution here.

On 02/06/17 00:46, Matthew Brush wrote:

> Hi,
>
> Out of curiosity, have you considered just copying the code of
> gtkimagemenuitem.[ch] into libxfce4ui and changing the symbol name
> prefix? Assuming it's not too intertangled with other modules, it should
> be pretty easy and the licenses (and even coding conventions) are
> compatible.
>
> The main advantage here is not requiring much changes to applications
> using the gtk_ versions, and avoiding the bug shown by André and which
> happens in GMenu where the icon is pushed to the right instead of going
> where the checkbox/radiobutton goes for other item kinds. Additionally
> it could be added to the Glade catalog to allow applications to use
> Glade still, which is not supported by your function or GMenu (last time
> I checked).
>
> Regards,
> Matthew Brush


_______________________________________________
Xfce4-dev mailing list
[hidden email]
https://mail.xfce.org/mailman/listinfo/xfce4-dev

signature.asc (849 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Patch] Add xfce_image_menu_item_new to libxfce4ui

André Miranda
I proposed just another function to make things even easier, but as C doesn't support function overloading, it requires another specific function, e.g. xfce_image_menu_item_new_with_icon.
Regarding accelerators, I dealt with them while porting xfce4-dict, I'm not inclined to drop them. GMenu is an option, but they have the same problem about icons.
I think you misunderstood when Matthew said the icons are pushed a little to right, i.e. it causes the left pad which is a reserved space for checkboxes/radiobuttons.

Cheers,
André Miranda

On 02/06/2017 02:19 PM, Florian Weigelt wrote:
Copying the files from gtk directly is possible, but overkill in my
opinion. I shortly thought of it, but it adds a lot of maintenance work,
too. I'd rather spend my time fixing the bugs in my provided function
than maintaining a defacto fork of GtkImageMenuItem (which uses the
deprecated stock items and gtkiconfactory). It makes nearly no
difference effort-wise for the programmer to use this function (maybe
changed to Andre's proposal) compared to the rebranded
gtk_image_menu_item_new_* from the copy. We don't use glade for the
(programatically created) menus I intended this function to be used for.
For the menu bars: Yes, glade does NOT support the menu syntax. But it
is still possible to manually write a .ui file with the menu syntax. As
for GMenu displaying the icon on the right: I never saw this happen.
Your points are all valid, but I think we can go with a less complex
solution here.

On 02/06/17 00:46, Matthew Brush wrote:
Hi,

Out of curiosity, have you considered just copying the code of
gtkimagemenuitem.[ch] into libxfce4ui and changing the symbol name
prefix? Assuming it's not too intertangled with other modules, it should
be pretty easy and the licenses (and even coding conventions) are
compatible.

The main advantage here is not requiring much changes to applications
using the gtk_ versions, and avoiding the bug shown by André and which
happens in GMenu where the icon is pushed to the right instead of going
where the checkbox/radiobutton goes for other item kinds. Additionally
it could be added to the Glade catalog to allow applications to use
Glade still, which is not supported by your function or GMenu (last time
I checked).

Regards,
Matthew Brush



_______________________________________________
Xfce4-dev mailing list
[hidden email]
https://mail.xfce.org/mailman/listinfo/xfce4-dev


_______________________________________________
Xfce4-dev mailing list
[hidden email]
https://mail.xfce.org/mailman/listinfo/xfce4-dev
Loading...