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 665140 - Draw the scale split
Draw the scale split
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Widget: Other
unspecified
Other All
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2011-11-29 16:37 UTC by Andrea Cimitan
Modified: 2011-12-14 16:22 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Fix for the issue (9.89 KB, patch)
2011-11-29 16:41 UTC, Andrea Cimitan
none Details | Review
Take 2 (10.17 KB, patch)
2011-11-29 18:23 UTC, Andrea Cimitan
reviewed Details | Review
Take 3 (10.75 KB, patch)
2011-11-29 19:02 UTC, Andrea Cimitan
accepted-commit_now Details | Review
Implemented cosimo's suggestion (10.74 KB, patch)
2011-12-13 16:01 UTC, Andrea Cimitan
accepted-commit_now Details | Review
Fixes typo (10.74 KB, patch)
2011-12-13 16:06 UTC, Andrea Cimitan
committed Details | Review

Description Andrea Cimitan 2011-11-29 16:37:15 UTC
As in Gtk+2, the scale can be split.

This patch adds a property to GtkScale, and APIs to set and get it.

Also, the GtkRange's drawing code was modified accordingly.
Comment 1 Andrea Cimitan 2011-11-29 16:41:43 UTC
Created attachment 202383 [details] [review]
Fix for the issue
Comment 2 Andrea Cimitan 2011-11-29 18:23:11 UTC
Created attachment 202388 [details] [review]
Take 2
Comment 3 Cosimo Cecchi 2011-11-29 18:30:02 UTC
Review of attachment 202388 [details] [review]:

Looks generally good, some comments below.

::: gtk/gtkrange.c
@@ +2127,3 @@
+              /* FIXME: was trough-upper and trough-lower really used,
+               * in that case, it should still be exposed somehow.
+               */

This FIXME can be removed now

::: gtk/gtkscale.c
@@ +92,2 @@
   guint         draw_value : 1;
+  guint         has_origin : 1;

You can probably use the member in GtkRange itself directly...no need to duplicate it here.

@@ +720,3 @@
+ * 
+ * Specifies whether the scale has an origin.
+ */

Missing Since: 3.4.

@@ +751,3 @@
+ * Returns whether the scale has an origin.
+ *
+ * Returns: whether the scale has an origin.

...%TRUE if the scale has an origin.
Also, it's not really explained what it means for a scale to have an origin.
Comment 4 Andrea Cimitan 2011-11-29 19:02:49 UTC
Created attachment 202390 [details] [review]
Take 3
Comment 5 Andrea Cimitan 2011-11-29 19:09:45 UTC
For the vertical one, just .trough.highlight.bottom could match the "colored" area, as .highlight is only set for the bottom element
Comment 6 Cosimo Cecchi 2011-12-01 14:07:43 UTC
Review of attachment 202390 [details] [review]:

This looks good for master to me, with the following nitpick to be fixed before.

::: gtk/gtkscale.c
@@ +517,3 @@
       break;
+    case PROP_HAS_ORIGIN:
+      g_value_set_boolean (value, _gtk_range_get_has_origin (GTK_RANGE (scale)));

Use gtk_scale_get_has_origin() instead of _gtk_range_get_has_origin() here.
Comment 7 Cosimo Cecchi 2011-12-01 14:17:51 UTC
Review of attachment 202390 [details] [review]:

This looks good for master to me, with the following nitpick to be fixed before.

::: gtk/gtkscale.c
@@ +517,3 @@
       break;
+    case PROP_HAS_ORIGIN:
+      g_value_set_boolean (value, _gtk_range_get_has_origin (GTK_RANGE (scale)));

Use gtk_scale_get_has_origin() instead of _gtk_range_get_has_origin() here.
Comment 8 Andrea Cimitan 2011-12-13 16:01:06 UTC
Created attachment 203350 [details] [review]
Implemented cosimo's suggestion
Comment 9 Matthias Clasen 2011-12-13 16:03:57 UTC
Review of attachment 203350 [details] [review]:

Fine to commit after fixing the typo

::: gtk/gtkscale.c
@@ +517,3 @@
       break;
+    case PROP_HAS_ORIGIN:
+      g_value_set_boolean (value, gtk_scake_get_has_origin (GTK_RANGE (scale)));

Typo: scale
Comment 10 Andrea Cimitan 2011-12-13 16:06:25 UTC
Created attachment 203351 [details] [review]
Fixes typo

ops
Comment 11 Cosimo Cecchi 2011-12-14 16:22:09 UTC
Pushed on Andrea's behalf; I also added the new APIs to gtk.symbols and gtk3-sections.txt.