hello, On Thu, Aug 8, 2019 at 2:47 PM Victor Toso <victortoso@xxxxxxxxxx> wrote: > > 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 ? yes > > > > 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. yes, it was a typo, s/3.3/3.22/, but when using these functions, the compiler was issuing warnings, that's why I removed them > virt-viewer-file-transfer-dialog.c:247:9: warning: 'gtk_scrolled_window_set_max_content_height' is deprecated: Not available before 3.22 [-Wdeprecated-declarations] > 247 | gtk_scrolled_window_set_max_content_height(GTK_SCROLLED_WINDOW(scrolled_window), 170); because of configure.ac::GTK_REQUIRED=3.12 > > > 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 I wasn't sure of the proper way to link the patch and the ticket! > > Signed-off-by: Kevin Pouget <kpouget@xxxxxxxxxx> > > 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@xxxxxxxxxx> thanks! I'm don't think I have commit rights for this repo, so if you can please do the commit Kevin > > --- > > 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@xxxxxxxxxx > > https://www.redhat.com/mailman/listinfo/virt-tools-list _______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list
