GNOME Bugzilla – Bug 640846
No tooltips in status picker
Last modified: 2012-04-02 09:47:15 UTC
Hello, I have a really long saved status message. It's ellipsised, but hovering over it in the dropdown does not show me the full message. Nor is it tooltip'd when I actually set it. Instead the tooltip is ‘Set your presence and current status’. Also, the Add button in Edit Custom Messages is packed with expand=True, so when you make that window wider it looks huge and dumb. Finally, having lovingly amended the spelling mistake in my very long status message in the Edit Custom Messages dialog, I couldn't activate it from there. I had to close the window, then aim for the 20×20 target of the dropdown arrow in the contact list, and then aim for the 100×20 target of the status I had just had selected in another window. A “set this” button would be nice maybe.
Sounds like easy tasks for newcomers.
(In reply to comment #0) > I have a really long saved status message. It's ellipsised, but hovering over > it in the dropdown does not show me the full message. This seems to be impossible to fix without changes to GtkComboBox. I also tried to get rid of the edit dialog entirely and just have little “delete” icons appear on hover in the dropdown menu, and that's also impossible. > Nor is it tooltip'd when I actually set it. Instead the tooltip is ‘Set your > presence and current status’. There's a FIXME in the code already about this sucking. :) > Also, the Add button in Edit Custom Messages is packed with expand=True, so > when you make that window wider it looks huge and dumb. I tried to make the Add button be the same width as the Remove and Close buttons (which are already the same size, at least in en_GB, for reasons I can't fathom), but putting Add and Remove in a size group had no effect, and when I moved the whole dialog contents over to a GtkTable, I couldn't make Remove and Add (which could trivially be made the same size) be the same size as Close.
(In reply to comment #0) > Finally, having lovingly amended the spelling mistake in my very long status > message in the Edit Custom Messages dialog, I couldn't activate it from there. > I had to close the window, then aim for the 20×20 target of the dropdown arrow > in the contact list, and then aim for the 100×20 target of the status I had > just had selected in another window. A “set this” button would be nice maybe. This is bug 599229 which some idiot filed in 2009.
So to completely sidestep the whole button-size thing, we could maybe merge the add and remove buttons into the tree view in the funky grey-tray-at-the-bottom style I've seen cropping up in a bunch of mockups of late. Here's half a mockup: http://people.collabora.com/~wjt/tmp/contact-editing-dialog-mockup.png Not shown: turning the available/busy/away icons into dropdown menus so you can pick one like that. So this would this dialog be more focused on editing existing statuses; you'd still be able to add new stuff from here if you really want, but that would be less of a focus. It's less cluttered than <http://people.collabora.com/~wjt/tmp/new-status-editor.png>, to my eye: less of a jumble of buttons and widgets. But it might be harder to use and/or understand. Just a thought.
Yeah, my main concern with this approach is how the user supposed to understand than clicking on the presence icon will allow him to change it? That would be the only place in Empathy reacting that way.
Allan actually suggested just getting rid of the dialog entirely, and making the menu only remember up to 3-4 recent statuses of each type, with “select it and remove the heart” as a way to get rid of an unwanted status.
(In reply to comment #2) > (In reply to comment #0) > > I have a really long saved status message. It's ellipsised, but hovering over > > it in the dropdown does not show me the full message. > > This seems to be impossible to fix without changes to GtkComboBox. I found a similar feature request for GtkTreeView: bug 346992. I feel like it ought to be possible to implement this using gtk_tree_view_set_tooltip_column() and binding has-tooltip to a property indicating whether or not the text was ellipsized, but I'm not smart enough. :)
Created attachment 199656 [details] [review] Setting tooltip for status I replaced the "Set your presence and current status" sentence that was appearing as tooltip and now the tooltip is the same as the status.
Review of attachment 199656 [details] [review]: ::: libempathy-gtk/empathy-presence-chooser.c @@ +453,3 @@ icon_name); gtk_entry_set_text (GTK_ENTRY (entry), status == NULL ? "" : status); + gtk_widget_set_tooltip_text (GTK_WIDGET (entry), status == NULL ? "" : status); This ternary operator should be assigned a variable rather than doing it twice. @@ +942,2 @@ /* FIXME: this string sucks */ + gtk_widget_set_tooltip_text (GTK_WIDGET (chooser), COL_STATUS_TEXT); Huh?
Created attachment 199812 [details] [review] Setting tooltip for status - update Corrected
Review of attachment 199812 [details] [review]: ::: libempathy-gtk/empathy-presence-chooser.c @@ +943,3 @@ chooser, 0); /* FIXME: this string sucks */ You've removed the string which sucks, so presumably you should also remove the comment about it sucking. :) @@ +945,3 @@ /* FIXME: this string sucks */ + status_tooltip = gtk_entry_get_text (GTK_ENTRY (entry)); + gtk_widget_set_tooltip_text (GTK_WIDGET (chooser), status_tooltip); Ideally, the tooltip would only show up if the message is too long to fit in the visible area. That said, this is definitely an improvement in my book.
(In reply to comment #11) > Ideally, the tooltip would only show up if the message is too long to fit in > the visible area. That said, this is definitely an improvement in my book. I'm not entirely sure it's worth it. Plus, it seems a bit inconsistent really. Easy enough to do via pango_layout_is_ellipsized(gtk_entry_get_layout()) but you'll need to listen for the ::size-allocate signal to know when you've been resized.
Created attachment 199871 [details] [review] Set status message as the tooltip
Created attachment 199872 [details] [review] [presence-chooser] Set status message as the tooltip \n\n Something like '[presence-chooser] Set status message as the tooltip\n\nFixes: https://bugzilla.gnome.org/show_bug.cgi?id=640846"
Created attachment 199873 [details] [review] [presence-chooser] Set status message as the tooltipFixes: https://bugzilla.gnome.org/show_bug.cgi?id=640846"
Comment on attachment 199873 [details] [review] [presence-chooser] Set status message as the tooltipFixes: https://bugzilla.gnome.org/show_bug.cgi?id=640846" Committed. As a 2nd patch, I wonder if when we're editing (presence_chooser_set_status_editing) it should have a more useful tooltip something like: "<b>Current message: %s</b>\n<small><i>Press Enter to set the new message or Esc to cancel.</i></small>"
Agreed; that's another easy task for a newcomer.
Created attachment 211109 [details] [review] Tooltip is more usefull when editing status. Fixes bug #640846
Review of attachment 211109 [details] [review]: Thanks for the patch! I inlined my comments. ::: libempathy-gtk/empathy-presence-chooser.c @@ +372,3 @@ EmpathyPresenceChooserPriv *priv = GET_PRIV (self); GtkWidget *entry; + TpConnectionPresenceType state; Those variables should be defined in the if block as they are not used outside of this block. @@ +374,3 @@ + TpConnectionPresenceType state; + gchar* tooltip_text; + char* status; gchar * @@ +384,3 @@ priv->editing_status = TRUE; + state = get_state_and_status(self, &status); This function pass the ownership of the status string to the caller. You have to free it once you're done to not leak it. @@ +385,3 @@ + state = get_state_and_status(self, &status); + priv->state = state; Why are you setting priv->state here ? @@ +386,3 @@ + state = get_state_and_status(self, &status); + priv->state = state; + tooltip_text = g_strdup_printf("<b>Current message: %s</b>\n" Our coding style are "function (arg)". Running "make check" should catch such issue.
Created attachment 211115 [details] [review] Tooltip is more usefull when editing status. Fixes bug #640846. Fixup
Review of attachment 211115 [details] [review]: FYI you don't have to include "Fixes bug..." in the commit message; git-bz will add the URL of the bug for you. ::: libempathy-gtk/empathy-presence-chooser.c @@ +385,3 @@ + get_state_and_status (self, &status); + tooltip_text = g_strdup_printf ("<b>Current message: %s</b>\n" oh sorry, I didn't notice during my first review but this string is leaked as well. As a rule: if a function return a new string, you'll have to free it once you're done with it.
Created attachment 211119 [details] [review] Tooltip is more usefull when editing status. Fixup
Merged to master; thanks a lot! Attachment 211119 [details] pushed as 812ecb8 - Tooltip is more usefull when editing status. Fixup