subject:"Re\\\: \\\[virt\\\-tools\\\-list\\\] \\\[PATCH\\\] Get rid of deprecated functions to customize widget colors"

Re: [virt-tools-list] [PATCH] Get rid of deprecated functions to customize widget colors

2016-06-28 Thread Eduardo Lima (Etrunko)

On 06/28/2016 07:21 AM, Fabiano Fidêncio wrote:
> On Tue, Jun 28, 2016 at 12:08 PM, Pavel Grunt  wrote:
>> Hi Eduardo,
>>
>> On Mon, 2016-06-27 at 18:00 -0300, Eduardo Lima (Etrunko) wrote:
>>> Fixes https://bugs.freedesktop.org/show_bug.cgi?id=94276
>>>
>>> Signed-off-by: Eduardo Lima (Etrunko) 
>>> ---
>>>
>>> As a result of commit cc455b7f916110d7cfae6b7af753349e070c9494, setting
>>> custom color for background does not work anymore on my Fedora 23
>>> system. The status label still rendered with white color, but with the
>>> background with default theme color (usually light grey), it has become
>>> hard to read the text from the label.
>>>
>>> I talked to Fabiano who told me that everything was working as expected
>>> with his recently upgraded Fedora 24 system. While trying to track and
>>> fix the issue, I noticed that it will only happen if the notebook tabs
>>> are hidden. If tabs are shown, the background color will be properly
>>> set.
>>>
>>> I tracked down to Gtk+ some changes to GtkNotebook in recently released
>>> version 3.20, which fixed the rendering of the custom background color,
>>> with tabs hidden, but those could not be easily backported. Even though
>>> it is a change of behavior in virt-viewer, I think it is really a minor
>>> issue, and I decided to not spent too much time on this, so I put a
>>> check for Gtk+ version to decide whether or not set the custom colors.
>>>
>>> Some screenshots to illustrate:
>>>
>>> Gtk+ > 3.20:
>>>   http://imgur.com/gpuMukA
>>>
>>> Gtk+ < 3.20:
>>>
>>>   without this patch.: http://imgur.com/RdirSoX
>>>   with this patch: http://imgur.com/9LJNeNI
>>
>> I would make it more simple, stick with the system theme
>> (ie http://imgur.com/9LJNeNI for all gtk versions) instead of introducing 
>> some
>> css just for 3.20 (is it stable btw ;-) ?). It would simplify the code, imho 
>> it
>> looks better and another gui tool from the family - virt-manager - uses it.
>>
>> What do you think ?
> 
> Hmm. This solution is quite okay for me.
> If the other people agree, I'd say to proceed with your suggestion then.
> 

Oh yes, I am all in favor of keeping the default theme colors.

-- 
Eduardo de Barros Lima (Etrunko)
Software Engineer - RedHat
etru...@redhat.com

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list

Re: [virt-tools-list] [PATCH] Get rid of deprecated functions to customize widget colors

2016-06-28 Thread Fabiano Fidêncio

On Tue, Jun 28, 2016 at 12:08 PM, Pavel Grunt  wrote:
> Hi Eduardo,
>
> On Mon, 2016-06-27 at 18:00 -0300, Eduardo Lima (Etrunko) wrote:
>> Fixes https://bugs.freedesktop.org/show_bug.cgi?id=94276
>>
>> Signed-off-by: Eduardo Lima (Etrunko) 
>> ---
>>
>> As a result of commit cc455b7f916110d7cfae6b7af753349e070c9494, setting
>> custom color for background does not work anymore on my Fedora 23
>> system. The status label still rendered with white color, but with the
>> background with default theme color (usually light grey), it has become
>> hard to read the text from the label.
>>
>> I talked to Fabiano who told me that everything was working as expected
>> with his recently upgraded Fedora 24 system. While trying to track and
>> fix the issue, I noticed that it will only happen if the notebook tabs
>> are hidden. If tabs are shown, the background color will be properly
>> set.
>>
>> I tracked down to Gtk+ some changes to GtkNotebook in recently released
>> version 3.20, which fixed the rendering of the custom background color,
>> with tabs hidden, but those could not be easily backported. Even though
>> it is a change of behavior in virt-viewer, I think it is really a minor
>> issue, and I decided to not spent too much time on this, so I put a
>> check for Gtk+ version to decide whether or not set the custom colors.
>>
>> Some screenshots to illustrate:
>>
>> Gtk+ > 3.20:
>>   http://imgur.com/gpuMukA
>>
>> Gtk+ < 3.20:
>>
>>   without this patch.: http://imgur.com/RdirSoX
>>   with this patch: http://imgur.com/9LJNeNI
>
> I would make it more simple, stick with the system theme
> (ie http://imgur.com/9LJNeNI for all gtk versions) instead of introducing some
> css just for 3.20 (is it stable btw ;-) ?). It would simplify the code, imho 
> it
> looks better and another gui tool from the family - virt-manager - uses it.
>
> What do you think ?

Hmm. This solution is quite okay for me.
If the other people agree, I'd say to proceed with your suggestion then.

Best Regards,
--
Fabiano Fidêncio

___
virt-tools-list mailing list
virt-tools-list@redhat.com
https://www.redhat.com/mailman/listinfo/virt-tools-list

Re: [virt-tools-list] [PATCH] Get rid of deprecated functions to customize widget colors

2016-06-28 Thread Pavel Grunt

Hi Eduardo,

On Mon, 2016-06-27 at 18:00 -0300, Eduardo Lima (Etrunko) wrote:
> Fixes https://bugs.freedesktop.org/show_bug.cgi?id=94276
> 
> Signed-off-by: Eduardo Lima (Etrunko) 
> ---
> 
> As a result of commit cc455b7f916110d7cfae6b7af753349e070c9494, setting
> custom color for background does not work anymore on my Fedora 23
> system. The status label still rendered with white color, but with the
> background with default theme color (usually light grey), it has become
> hard to read the text from the label.
> 
> I talked to Fabiano who told me that everything was working as expected
> with his recently upgraded Fedora 24 system. While trying to track and
> fix the issue, I noticed that it will only happen if the notebook tabs
> are hidden. If tabs are shown, the background color will be properly
> set.
> 
> I tracked down to Gtk+ some changes to GtkNotebook in recently released
> version 3.20, which fixed the rendering of the custom background color,
> with tabs hidden, but those could not be easily backported. Even though
> it is a change of behavior in virt-viewer, I think it is really a minor
> issue, and I decided to not spent too much time on this, so I put a
> check for Gtk+ version to decide whether or not set the custom colors.
> 
> Some screenshots to illustrate:
> 
> Gtk+ > 3.20:
>   http://imgur.com/gpuMukA
> 
> Gtk+ < 3.20:
> 
>   without this patch.: http://imgur.com/RdirSoX
>   with this patch: http://imgur.com/9LJNeNI

I would make it more simple, stick with the system theme 
(ie http://imgur.com/9LJNeNI for all gtk versions) instead of introducing some
css just for 3.20 (is it stable btw ;-) ?). It would simplify the code, imho it
looks better and another gui tool from the family - virt-manager - uses it.

What do you think ?

Pavel

> 
> ---
>  src/virt-viewer-notebook.c | 25 ++---
>  src/virt-viewer-window.c   | 10 --
>  2 files changed, 14 insertions(+), 21 deletions(-)
> 
> diff --git a/src/virt-viewer-notebook.c b/src/virt-viewer-notebook.c
> index 420c914..f02779c 100644
> --- a/src/virt-viewer-notebook.c
> +++ b/src/virt-viewer-notebook.c
> @@ -71,25 +71,28 @@ static void
>  virt_viewer_notebook_init (VirtViewerNotebook *self)
>  {
>  VirtViewerNotebookPrivate *priv;
> -GdkRGBA color;
>  
>  self->priv = GET_PRIVATE(self);
>  priv = self->priv;
>  
> -priv->status = gtk_label_new("");
> +/* Check for Gtk+ 3.20 to set the custom colors, because with older
> versions
> + * it the background color will not be set correctly, so we will end up
> with
> + * the default theme color for background (usually light grey), while the
> + * foreground text color is white.
> + */
> +if (gtk_check_version(3,20,0) == NULL) {
> +GtkStyleContext *style =
> gtk_widget_get_style_context(GTK_WIDGET(self));
> +GtkCssProvider *css = gtk_css_provider_new();
> +gtk_css_provider_load_from_data(css, "* { background-color: black;
> color: white; }", -1, NULL);
> +gtk_style_context_add_provider(style, GTK_STYLE_PROVIDER(css),
> GTK_STYLE_PROVIDER_PRIORITY_APPLICATION);
> +}
> +
>  gtk_notebook_set_show_tabs(GTK_NOTEBOOK(self), FALSE);
>  gtk_notebook_set_show_border(GTK_NOTEBOOK(self), FALSE);
> +
> +priv->status = gtk_label_new("");
>  gtk_widget_show_all(priv->status);
>  gtk_notebook_append_page(GTK_NOTEBOOK(self), priv->status, NULL);
> -gdk_rgba_parse(, "white");
> -/* FIXME:
> - * This method has been deprecated in 3.16.
> - * For more details on how to deal with this in the future, please, see:
> - * https://developer.gnome.org/gtk3/stable/GtkWidget.html#gtk-widget-over
> ride-color
> - * For the bug report about this deprecated function, please, see:
> - * https://bugs.freedesktop.org/show_bug.cgi?id=94276
> - */
> -gtk_widget_override_color(priv->status, GTK_STATE_FLAG_NORMAL, );
>  }
>  
>  void
> diff --git a/src/virt-viewer-window.c b/src/virt-viewer-window.c
> index 1ebb423..c59fff5 100644
> --- a/src/virt-viewer-window.c
> +++ b/src/virt-viewer-window.c
> @@ -297,7 +297,6 @@ virt_viewer_window_init (VirtViewerWindow *self)
>  {
>  VirtViewerWindowPrivate *priv;
>  GtkWidget *vbox;
> -GdkRGBA color;
>  GSList *accels;
>  
>  self->priv = GET_PRIVATE(self);
> @@ -340,15 +339,6 @@ virt_viewer_window_init (VirtViewerWindow *self)
>  virt_viewer_window_toolbar_setup(self);
>  
>  gtk_box_pack_end(GTK_BOX(vbox), GTK_WIDGET(priv->notebook), TRUE, TRUE,
> 0);
> -gdk_rgba_parse(, "black");
> -/* FIXME:
> - * This method has been deprecated in 3.16.
> - * For more details on how to deal with this in the future, please, see:
> - * https://developer.gnome.org/gtk3/stable/GtkWidget.html#gtk-widget-over
> ride-background-color
> - * For the bug report about this deprecated function, please, see:
> - * https://bugs.freedesktop.org/show_bug.cgi?id=94276
> - */
> -

Re: [virt-tools-list] [PATCH] Get rid of deprecated functions to customize widget colors

2016-06-28 Thread Fabiano Fidêncio

Etrunko,

On Mon, Jun 27, 2016 at 11:00 PM, Eduardo Lima (Etrunko)
 wrote:
> Fixes https://bugs.freedesktop.org/show_bug.cgi?id=94276
>
> Signed-off-by: Eduardo Lima (Etrunko) 
> ---
>
> As a result of commit cc455b7f916110d7cfae6b7af753349e070c9494, setting
> custom color for background does not work anymore on my Fedora 23
> system. The status label still rendered with white color, but with the
> background with default theme color (usually light grey), it has become
> hard to read the text from the label.
>
> I talked to Fabiano who told me that everything was working as expected
> with his recently upgraded Fedora 24 system. While trying to track and
> fix the issue, I noticed that it will only happen if the notebook tabs
> are hidden. If tabs are shown, the background color will be properly
> set.
>
> I tracked down to Gtk+ some changes to GtkNotebook in recently released
> version 3.20, which fixed the rendering of the custom background color,
> with tabs hidden, but those could not be easily backported.

Do you have the commit hash? Would be nice to have it in the commit log.

> Even though
> it is a change of behavior in virt-viewer, I think it is really a minor
> issue, and I decided to not spent too much time on this, so I put a
> check for Gtk+ version to decide whether or not set the custom colors.
>
> Some screenshots to illustrate:
>
> Gtk+ > 3.20:
>   http://imgur.com/gpuMukA
>
> Gtk+ < 3.20:
>
>   without this patch.: http://imgur.com/RdirSoX
>   with this patch: http://imgur.com/9LJNeNI
>

I really don't like to workaround Gtk+ issues on our side, but in this
case it seems to be the way to go.
Daniel, Jonathon, Pavel ... I want to hear from you guys here as well.

> ---
>  src/virt-viewer-notebook.c | 25 ++---
>  src/virt-viewer-window.c   | 10 --
>  2 files changed, 14 insertions(+), 21 deletions(-)
>
> diff --git a/src/virt-viewer-notebook.c b/src/virt-viewer-notebook.c
> index 420c914..f02779c 100644
> --- a/src/virt-viewer-notebook.c
> +++ b/src/virt-viewer-notebook.c
> @@ -71,25 +71,28 @@ static void
>  virt_viewer_notebook_init (VirtViewerNotebook *self)
>  {
>  VirtViewerNotebookPrivate *priv;
> -GdkRGBA color;
>
>  self->priv = GET_PRIVATE(self);
>  priv = self->priv;
>
> -priv->status = gtk_label_new("");
> +/* Check for Gtk+ 3.20 to set the custom colors, because with older 
> versions
> + * it the background color will not be set correctly, so we will end up 
> with
> + * the default theme color for background (usually light grey), while the
> + * foreground text color is white.
> + */
> +if (gtk_check_version(3,20,0) == NULL) {

Please, use the compile time check instead of the runtime one.
#if GTK_CHECK_VERSION(3,20,0) ...

> +GtkStyleContext *style = 
> gtk_widget_get_style_context(GTK_WIDGET(self));
> +GtkCssProvider *css = gtk_css_provider_new();
> +gtk_css_provider_load_from_data(css, "* { background-color: black; 
> color: white; }", -1, NULL);
> +gtk_style_context_add_provider(style, GTK_STYLE_PROVIDER(css), 
> GTK_STYLE_PROVIDER_PRIORITY_APPLICATION);
> +}
> +
>  gtk_notebook_set_show_tabs(GTK_NOTEBOOK(self), FALSE);
>  gtk_notebook_set_show_border(GTK_NOTEBOOK(self), FALSE);
> +
> +priv->status = gtk_label_new("");
>  gtk_widget_show_all(priv->status);
>  gtk_notebook_append_page(GTK_NOTEBOOK(self), priv->status, NULL);
> -gdk_rgba_parse(, "white");
> -/* FIXME:
> - * This method has been deprecated in 3.16.
> - * For more details on how to deal with this in the future, please, see:
> - * 
> https://developer.gnome.org/gtk3/stable/GtkWidget.html#gtk-widget-override-color
> - * For the bug report about this deprecated function, please, see:
> - * https://bugs.freedesktop.org/show_bug.cgi?id=94276
> - */
> -gtk_widget_override_color(priv->status, GTK_STATE_FLAG_NORMAL, );
>  }
>
>  void
> diff --git a/src/virt-viewer-window.c b/src/virt-viewer-window.c
> index 1ebb423..c59fff5 100644
> --- a/src/virt-viewer-window.c
> +++ b/src/virt-viewer-window.c
> @@ -297,7 +297,6 @@ virt_viewer_window_init (VirtViewerWindow *self)
>  {
>  VirtViewerWindowPrivate *priv;
>  GtkWidget *vbox;
> -GdkRGBA color;
>  GSList *accels;
>
>  self->priv = GET_PRIVATE(self);
> @@ -340,15 +339,6 @@ virt_viewer_window_init (VirtViewerWindow *self)
>  virt_viewer_window_toolbar_setup(self);
>
>  gtk_box_pack_end(GTK_BOX(vbox), GTK_WIDGET(priv->notebook), TRUE, TRUE, 
> 0);
> -gdk_rgba_parse(, "black");
> -/* FIXME:
> - * This method has been deprecated in 3.16.
> - * For more details on how to deal with this in the future, please, see:
> - * 
> https://developer.gnome.org/gtk3/stable/GtkWidget.html#gtk-widget-override-background-color
> - * For the bug report about this deprecated function, please, see:
> - *

Re: [virt-tools-list] [PATCH] Get rid of deprecated functions to customize widget colors

Re: [virt-tools-list] [PATCH] Get rid of deprecated functions to customize widget colors

Re: [virt-tools-list] [PATCH] Get rid of deprecated functions to customize widget colors

Re: [virt-tools-list] [PATCH] Get rid of deprecated functions to customize widget colors

4 matches

Site Navigation

Mail list logo

Footer information