[virt-tools-list] [PATCH] virt-viewer-file-transfer-dialog: improve error message

[Victor Toso]

Hi,

On Thu, Aug 01, 2019 at 05:31:20PM +0200, Kevin Pouget wrote:
> This patch improves the error shown to the user when a file transfer
> fails.
> 
> The previous behavior was to create a simple message dialog box, with
> the error description and the full list of the files that failed to be
> transferred. When the list of files was long, the dialog box would
> grow bigger than the screen.
> 
> Now, the file list is inserted inside a scrollable widget, whose width
> is limited to 170px.

s/width/height ?

> 
> NB: these two calls would be more adapted, but they require GTK >= 3.3:

Typo on 3.3? Gtk required is 3.12 and introduced in those
functions is 3.22. Spice-gtk requires 3.22 already so this can be
considered for virt-viewer too. Looking at
a1fd9ee1c680 in spice-gtk (the bump to 3.22), even Fedora 26 and
RHEL 7.4 had 3.22 so it shouldn't be a problem to bump if you
feel like it.

> > gtk_scrolled_window_set_max_content_height(GTK_SCROLLED_WINDOW(scrolled_window), 170);
> > gtk_scrolled_window_set_propagate_natural_height(GTK_SCROLLED_WINDOW(scrolled_window), TRUE);
> 
> This patch addresses this bug:
> 
> > https://bugzilla.redhat.com/show_bug.cgi?id=1496356

Those last three lines can be changed to
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1496356

> Signed-off-by: Kevin Pouget <kpouget at redhat.com>

Patch works fine, thanks. Let me know if you prefer sending a v2
with any changes or if I should change the commit log and push
(in case you can't push it), imho this one works fine

    Acked-by: Victor Toso <victortoso at redhat.com>

> ---
>  src/virt-viewer-file-transfer-dialog.c | 26 ++++++++++++++++++++++----
>  1 file changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/src/virt-viewer-file-transfer-dialog.c b/src/virt-viewer-file-transfer-dialog.c
> index a82d01a..b510d8e 100644
> --- a/src/virt-viewer-file-transfer-dialog.c
> +++ b/src/virt-viewer-file-transfer-dialog.c
> @@ -200,7 +200,8 @@ static gboolean hide_transfer_dialog(gpointer data)
>      if (self->priv->failed) {
>          GSList *sl;
>          GString *msg = g_string_new("");
> -        GtkWidget *dialog;
> +        GtkWidget *dialog, *files_label, *scrolled_window, *area;
> +        GtkRequisition files_label_sz;
> 
>          for (sl = self->priv->failed; sl != NULL; sl = g_slist_next(sl)) {
>              SpiceFileTransferTask *failed_task = sl->data;
> @@ -221,11 +222,28 @@ static gboolean hide_transfer_dialog(gpointer data)
> 
>          dialog = gtk_message_dialog_new(GTK_WINDOW(self), 0, GTK_MESSAGE_ERROR,
>                                          GTK_BUTTONS_OK,
> -                                        _("An error caused the following file transfers to fail:%s"),
> -                                        msg->str);
> +                                        _("An error caused the following file transfers to fail:"));
> +        gtk_window_set_title(GTK_WINDOW(dialog), "Transfer error");
> +
> +        scrolled_window = gtk_scrolled_window_new(NULL, NULL);
> +        gtk_scrolled_window_set_policy(GTK_SCROLLED_WINDOW(scrolled_window),
> +                                       GTK_POLICY_AUTOMATIC, GTK_POLICY_AUTOMATIC);
> +
> +        area = gtk_message_dialog_get_message_area(GTK_MESSAGE_DIALOG(dialog));
> +        gtk_container_add(GTK_CONTAINER(area), scrolled_window);
> +
> +        files_label = gtk_label_new(msg->str + 1); /* skip the initial '\n' */
> +        gtk_label_set_selectable(GTK_LABEL(files_label), TRUE);
> +        gtk_container_add(GTK_CONTAINER(scrolled_window), files_label);
> +
>          g_string_free(msg, TRUE);
>          g_signal_connect(dialog, "response", G_CALLBACK(error_dialog_response), NULL);
> -        gtk_widget_show(dialog);
> +        gtk_widget_show_all(dialog);
> +
> +        /* adjust panel to file_label height */
> +        gtk_widget_get_preferred_size(files_label, NULL, &files_label_sz);
> +        gtk_scrolled_window_set_min_content_height(GTK_SCROLLED_WINDOW(scrolled_window),
> +                                                   MIN(files_label_sz.height, 170));
>      }
> 
>      return G_SOURCE_REMOVE;
> --
> 2.21.0
> 
> _______________________________________________
> virt-tools-list mailing list
> virt-tools-list at redhat.com
> https://www.redhat.com/mailman/listinfo/virt-tools-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20190808/6b5223bc/attachment.sig>