After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 695902 - Mouse test area as a proper widget
Mouse test area as a proper widget
Status: RESOLVED FIXED
Product: gnome-control-center
Classification: Core
Component: Mouse
git master
Other Linux
: Normal normal
: ---
Assigned To: Control-Center Maintainers
Control-Center Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-03-15 11:27 UTC by Ondrej Holy
Modified: 2013-05-24 13:22 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
mouse test area as a proper widget (18.88 KB, patch)
2013-03-15 11:27 UTC, Ondrej Holy
needs-work Details | Review
mouse preferences as a proper widget (14.96 KB, patch)
2013-03-28 09:58 UTC, Ondrej Holy
reviewed Details | Review
mouse test area as a proper widget (18.89 KB, patch)
2013-05-24 09:10 UTC, Ondrej Holy
committed Details | Review
mouse preferences as a proper widget (15.37 KB, patch)
2013-05-24 09:11 UTC, Ondrej Holy
committed Details | Review

Description Ondrej Holy 2013-03-15 11:27:58 UTC
Created attachment 238968 [details] [review]
mouse test area as a proper widget

There is a patch making a proper widget from mouse test area to avoid using global variables.

It was discussed in Bug 693803.
Comment 1 Ondrej Holy 2013-03-28 09:58:19 UTC
Created attachment 240017 [details] [review]
mouse preferences as a proper widget

Same changes also for mouse preferences.
Comment 2 Bastien Nocera 2013-05-13 12:59:58 UTC
Review of attachment 238968 [details] [review]:

A few fixes needed.

::: panels/mouse/gnome-mouse-test.c
@@ +189,3 @@
 {
+	gint double_click_time;
+	static guint32 double_click_timestamp = 0;

A static isn't helping remove global variables ;)

@@ +239,3 @@
 static gboolean
+button_drawing_area_draw_event (GtkWidget *widget,
+				cairo_t *cr,

Please align those.

@@ +323,3 @@
+
+	if (d->mouse_settings != NULL) {
+		g_object_unref (d->mouse_settings);

Use g_clear_object().

@@ +366,3 @@
+				       "/org/gnome/control-center/mouse/gnome-mouse-test.ui",
+				       &error);
+	if (error != NULL) {

You can remove the NULL checks, they shouldn't happen.
Comment 3 Bastien Nocera 2013-05-13 13:02:05 UTC
Review of attachment 240017 [details] [review]:

Looks good to commit after those few changes.

::: panels/mouse/gnome-mouse-properties.c
@@ +285,3 @@
+
+	if (d->mouse_settings != NULL) {
+		g_object_unref (d->mouse_settings);

g_clear_object().

@@ +289,3 @@
+	}
+	if (d->touchpad_settings != NULL) {
+		g_object_unref (d->touchpad_settings);

g_clear_object().

@@ +324,3 @@
+				       "/org/gnome/control-center/mouse/gnome-mouse-properties.ui",
+				       &error);
+	if (error != NULL) {

As per other patch, feel free to remove this.
Comment 4 Ondrej Holy 2013-05-24 09:08:49 UTC
Thank you for reviews.

(In reply to comment #2)
> Review of attachment 238968 [details] [review]:
> @@ +239,3 @@
>  static gboolean
> +button_drawing_area_draw_event (GtkWidget *widget,
> +                cairo_t *cr,

There is mode line in header with tab-width 8, so it is aligned accordingly.
Comment 5 Ondrej Holy 2013-05-24 09:10:20 UTC
Created attachment 245214 [details] [review]
mouse test area as a proper widget
Comment 6 Ondrej Holy 2013-05-24 09:11:11 UTC
Created attachment 245215 [details] [review]
mouse preferences as a proper widget
Comment 7 Bastien Nocera 2013-05-24 12:26:04 UTC
Review of attachment 245215 [details] [review]:

Looks good.
Comment 8 Bastien Nocera 2013-05-24 12:26:31 UTC
Review of attachment 245214 [details] [review]:

Looks good.