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 579135 - Touchpad practice area
Touchpad practice area
Status: RESOLVED FIXED
Product: gnome-control-center
Classification: Core
Component: Mouse
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Control-Center Maintainers
Control-Center Maintainers
: 636130 (view as bug list)
Depends on:
Blocks: 682492
 
 
Reported: 2009-04-16 10:08 UTC by Jonathan Blandford
Modified: 2012-08-30 10:37 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
new testing are according to mockup (22.56 KB, patch)
2012-08-22 12:42 UTC, Ondrej Holy
none Details | Review
new testing area according to mockup (162.01 KB, patch)
2012-08-23 13:27 UTC, Ondrej Holy
needs-work Details | Review
new testing area screenshot (48.87 KB, image/png)
2012-08-23 13:28 UTC, Ondrej Holy
  Details
new testing area according to mockup (162.81 KB, patch)
2012-08-28 10:36 UTC, Ondrej Holy
committed Details | Review
reset timeouts id and size fallback (2.39 KB, patch)
2012-08-29 07:49 UTC, Ondrej Holy
accepted-commit_now Details | Review
reset timeouts id to zero (1.92 KB, patch)
2012-08-29 10:50 UTC, Ondrej Holy
committed Details | Review
widget allocated size fallback (1013 bytes, patch)
2012-08-29 10:50 UTC, Ondrej Holy
committed Details | Review

Description Jonathan Blandford 2009-04-16 10:08:53 UTC
It would be nice to have a little practice area where you could test out the various touchpad settings.  Perhaps a grey rectangle with an indicator for clicked and scrolling.
Comment 1 Juan R. Garcia Blanco 2010-03-01 09:17:35 UTC
I think it's a good idea. Mac OS X includes videos to demonstrate what do the selected settings; having that would be ideal, but for now a grey rectangle for testing should be enough.
Comment 2 Ondrej Holy 2012-08-22 12:42:01 UTC
Created attachment 222139 [details] [review]
new testing are according to mockup

There is new mockup for testing area:

https://live.gnome.org/Design/SystemSettings/Mouse

I attached testing patch for it.
Comment 3 Ondrej Holy 2012-08-23 13:27:25 UTC
Created attachment 222224 [details] [review]
new testing area according to mockup

There are some changes according designers ideas.

Please review attached patch.
Comment 4 Ondrej Holy 2012-08-23 13:28:31 UTC
Created attachment 222225 [details]
new testing area screenshot
Comment 5 Bastien Nocera 2012-08-23 13:38:27 UTC
The timeout for the text test feedback should fade out at the same time as the display itself. Right now, the circle will have gone back to its original state, and the text will still say "double-click, primary button".

The scroll wheel on a mouse also doesn't work if you bash the mouse buttons over the test area, sometimes.

And finally, you're not cleaning up your timeouts when exiting the panel, so this can happen:
  • #0 g_type_check_instance_cast
    at gtype.c line 3997
  • #1 setup_information_label
    at gnome-mouse-test.c line 138
  • #2 information_label_timeout
    at gnome-mouse-test.c line 91
  • #3 g_timeout_dispatch
    at gmain.c line 4018
  • #4 g_main_dispatch
    at gmain.c line 2707
  • #5 g_main_context_dispatch
    at gmain.c line 3211
  • #6 g_main_context_iterate
    at gmain.c line 3282
  • #7 g_main_context_iteration
    at gmain.c line 3343
  • #8 g_application_run
    at gapplication.c line 1607
  • #9 main
    at control-center.c line 256

Comment 6 Bastien Nocera 2012-08-23 13:41:20 UTC
Review of attachment 222224 [details] [review]:

As per comment, needs-work

::: panels/mouse/gnome-mouse-test.c
@@ +38,3 @@
 
+/* Click test button sizes. */
+#define BUTTON_SIZE 180.0

That should really be dependent on the size of the test area, so that we can only see the button area, and the little girl, not more.

@@ +39,3 @@
+/* Click test button sizes. */
+#define BUTTON_SIZE 180.0
+#define SHADDOW_SIZE 10.0

shadow

@@ +232,3 @@
 	}
 
+	/* Draw shaddow. */

shadow.
Comment 7 Bastien Nocera 2012-08-23 14:51:09 UTC
*** Bug 636130 has been marked as a duplicate of this bug. ***
Comment 8 Matthias Clasen 2012-08-25 00:36:58 UTC
Review of attachment 222224 [details] [review]:

::: panels/mouse/gnome-mouse-test.c
@@ +112,3 @@
+	switch (button_state) {
+	case 1:
+		button_text = _("<b>primary button</b>");

Please keep the markup out of the translatable content - translators are prone to get this wrong.

@@ +135,3 @@
+	}
+
+	label_text = g_strconcat (click_text, button_text, NULL);

Concatenating translated fragments like this makes it often impossible to correctly translate. Better to have a larger list of complete sentences like

Single click, primary button
...
Double click, secondary button

choose the right one according to the state, and then do

label_text = g_strconcat ("<b>", message, "</b>", NULL);

or, if you want to go the extra mile:

label_text = g_markup_printf_escaped ("<b>%s</b>", message);
Comment 9 Matthias Clasen 2012-08-25 00:37:28 UTC
I really love the visuals here, btw. Great work
Comment 10 Ondrej Holy 2012-08-28 07:28:34 UTC
(In reply to comment #5)
> The timeout for the text test feedback should fade out at the same time as the
> display itself. Right now, the circle will have gone back to its original
> state, and the text will still say "double-click, primary button".
> 

The timeout for the circle is set to the double click time and the timeout for information label has fixed time. Longer time for information label was aday wish. However both can have same double click time, but user doesn't manage to read the information label in case of short time. I think setting timeout to static value is confusing for user, because the circle change its state before/after the double click time.

> The scroll wheel on a mouse also doesn't work if you bash the mouse buttons
> over the test area, sometimes.
> 

It's weird, I'm not able to reproduce it.

> And finally, you're not cleaning up your timeouts when exiting the panel, so
> this can happen:
> 

You are right, I'll fix it.

(In reply to comment #6)
> Review of attachment 222224 [details] [review]:
> 
> As per comment, needs-work
> 
> ::: panels/mouse/gnome-mouse-test.c
> @@ +38,3 @@
> 
> +/* Click test button sizes. */
> +#define BUTTON_SIZE 180.0
> 
> That should really be dependent on the size of the test area, so that we can
> only see the button area, and the little girl, not more.
> 

Ok, I'll fix it.

> @@ +39,3 @@
> +/* Click test button sizes. */
> +#define BUTTON_SIZE 180.0
> +#define SHADDOW_SIZE 10.0
> 
> shadow
> 
> @@ +232,3 @@
>      }
> 
> +    /* Draw shaddow. */
> 
> shadow.

Sorry for my English, I'll fix it.
Comment 11 Ondrej Holy 2012-08-28 07:32:22 UTC
(In reply to comment #8)
> Review of attachment 222224 [details] [review]:
> 
> ::: panels/mouse/gnome-mouse-test.c
> @@ +112,3 @@
> +    switch (button_state) {
> +    case 1:
> +        button_text = _("<b>primary button</b>");
> 
> Please keep the markup out of the translatable content - translators are prone
> to get this wrong.
> 
> @@ +135,3 @@
> +    }
> +
> +    label_text = g_strconcat (click_text, button_text, NULL);
> 
> Concatenating translated fragments like this makes it often impossible to
> correctly translate. Better to have a larger list of complete sentences like
> 

I didn't know it, I'll fix it.

(In reply to comment #9)
> I really love the visuals here, btw. Great work

Thank you.
Comment 12 Ondrej Holy 2012-08-28 10:36:35 UTC
Created attachment 222621 [details] [review]
new testing area according to mockup

There is new patch according reviews.

Timeouts are set to 2.5 seconds currently in case of double click, however the circle timeout is still set to the double click time in case of single click. Should I set all timeouts to fixed value or how can I set them?
Comment 13 Bastien Nocera 2012-08-28 17:48:14 UTC
Review of attachment 222621 [details] [review]:

Looks good overall.

::: panels/mouse/gnome-mouse-test.c
@@ +203,3 @@
+	cairo_pattern_t *pattern;
+
+	size = MIN (gtk_widget_get_allocated_width (widget), gtk_widget_get_allocated_height (widget));

Add fallback, just in case, if you don't want to divide by zero in some circumstances.

@@ +267,3 @@
+				  gtk_adjustment_get_upper (adjustment));
+
+	gdk_rgba_parse (&color, "#565854");

Do we have a bug somewhere about getting a symbolic colour for this?

@@ +291,3 @@
+
+	if (information_label_timeout_id != 0)
+		g_source_remove (information_label_timeout_id);

Dispose can be called multiple times, so reset the _id to zero.

@@ +294,3 @@
+
+	if (button_drawing_area_timeout_id != 0)
+		g_source_remove  (button_drawing_area_timeout_id);

Ditto.
Comment 14 Ondrej Holy 2012-08-29 07:49:07 UTC
Created attachment 222718 [details] [review]
reset timeouts id and size fallback
Comment 15 Ondrej Holy 2012-08-29 08:05:50 UTC
(In reply to comment #13)
> Review of attachment 222621 [details] [review]:
> 
> @@ +267,3 @@
> +                  gtk_adjustment_get_upper (adjustment));
> +
> +    gdk_rgba_parse (&color, "#565854");
> 
> Do we have a bug somewhere about getting a symbolic colour for this?
> 

We already have:

https://bugzilla.gnome.org/show_bug.cgi?id=682925

Can you write there more information please what you mean with that?
Comment 16 Bastien Nocera 2012-08-29 10:30:00 UTC
Review of attachment 222718 [details] [review]:

Please split this out in 2 patches. But looks good.
Comment 17 Ondrej Holy 2012-08-29 10:50:20 UTC
Created attachment 222731 [details] [review]
reset timeouts id to zero
Comment 18 Ondrej Holy 2012-08-29 10:50:58 UTC
Created attachment 222732 [details] [review]
widget allocated size fallback
Comment 19 Bastien Nocera 2012-08-29 10:55:48 UTC
Review of attachment 222731 [details] [review]:

Looks good
Comment 20 Bastien Nocera 2012-08-29 10:56:14 UTC
Review of attachment 222732 [details] [review]:

Looks good
Comment 21 Allan Day 2012-08-30 10:37:44 UTC
Thanks Ondrej, I'm looking forward to trying this!