GNOME Bugzilla – Bug 784215
[review] lr/ui-improvements: some random UI improvements
Last modified: 2017-07-11 15:57:07 UTC
https://git.gnome.org/browse/network-manager-applet/log/?h=lr/ui-improvements
Please don’t update .po files. Damned-lies and translators take care of it in GNOME.
(In reply to Piotr Drąg from comment #1) > Please don’t update .po files. Damned-lies and translators take care of it > in GNOME. Dropped the commit, thanks. Added some more stuff into the branch, ready for review now.
`make check` fails +G_DEFINE_TYPE_WITH_CODE (NMConnectionList, nm_connection_list, GTK_TYPE_DIALOG, + G_ADD_PRIVATE (NMConnectionList)) G_ADD_PRIVATE() is glib 2.38. You'll have to bump the glib requirement, if you intend to use it. + /* Initialize the widget template */ + gtk_widget_class_set_template_from_resource (widget_class, indentation +typedef struct { + GtkDialog parent; + NMConnectionListPrivate *priv; } NMConnectionList; NM_CONNECTION_LIST_GET_PRIVATE() doesn't use the priv field. The priv field could be either dropped, or should be used (as it is a tiny bit more efficient then G_TYPE_INSTANCE_GET_PRIVATE macro). NM calls this field consistently "_priv", which is required by the _NM_GET_PRIVATE() macro. Also, if you compare with NetworkManager, the "_priv" field is usually not a pointer, but it's directly embedded in the object struct. It's only a pointer, when the class is subclassed (which NMConnectionEditor is not). For what I consider best-practice see: https://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=4d37f7a1e94f469fb1e3eacde4d2424ebf6ccf0b (but it's fine with me to just drop priv altogether). + data->validation_error = g_strdup (validation_error); + update_widget (widget); indentation (and more, in same file). In nm-c-e's connection list, when you start typing (or press CTRL+F), you can enter text -- for what I presume is search. But it doesn't work. Either implement it, or disable it. Also, while the search input field is present, clicking "X" to close nm-c-e doesn't work. Can that be fixed? A feature request: when editing a connection, accept ESC to "Cancel". could you comment on bug 783938, otherwise the patch there lgtm and I'd merge it. Nice improvements overall!!!
(In reply to Thomas Haller from comment #3) > `make check` fails Fixed. > +G_DEFINE_TYPE_WITH_CODE (NMConnectionList, nm_connection_list, > GTK_TYPE_DIALOG, > + G_ADD_PRIVATE (NMConnectionList)) > > G_ADD_PRIVATE() is glib 2.38. You'll have to bump the glib requirement, if > you intend to use it. Bumped it. We actually rely on it already, and so does Gtk+ 3.10. > + /* Initialize the widget template */ > + gtk_widget_class_set_template_from_resource (widget_class, > > indentation Fixed. > +typedef struct { > + GtkDialog parent; > + NMConnectionListPrivate *priv; > } NMConnectionList; > > NM_CONNECTION_LIST_GET_PRIVATE() doesn't use the priv field. The priv field > could be either dropped, or should be used (as it is a tiny bit more > efficient then G_TYPE_INSTANCE_GET_PRIVATE macro). > > NM calls this field consistently "_priv", which is required by the > _NM_GET_PRIVATE() macro. > > Also, if you compare with NetworkManager, the "_priv" field is usually not a > pointer, but it's directly embedded in the object struct. It's only a > pointer, when the class is subclassed (which NMConnectionEditor is not). For > what I consider best-practice see: > https://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/ > ?id=4d37f7a1e94f469fb1e3eacde4d2424ebf6ccf0b > (but it's fine with me to just drop priv altogether). Removed the extra priv field. What NetworkManager does doesn't seem to use G_ADD_PRIVATE() and thus G_PRIVATE_OFFSET() and in turn gtk_widget_class_bind_template_child_private() won't work. > + data->validation_error = g_strdup (validation_error); > + update_widget (widget); > > indentation (and more, in same file). Fixed. > In nm-c-e's connection list, when you start typing (or press CTRL+F), you > can enter text -- for what I presume is search. But it doesn't work. Either > implement it, or disable it. > Also, while the search input field is present, clicking "X" to close nm-c-e > doesn't work. Can that be fixed? > > > A feature request: when editing a connection, accept ESC to "Cancel". Added. > could you comment on bug 783938, otherwise the patch there lgtm and I'd > merge it. Commented. Thought it's merged already since I've seen GTK_WIN_POS_CENTER all around the place. Updated the branch.
Merged in f84f06a02ac3bfcb17f8550e1060d72fd3d6a2c2