elementary: fix spinner wrap
authorBruno Dilly <bdilly@profusion.mobi>
Mon, 1 Oct 2012 20:44:19 +0000 (20:44 +0000)
committerBruno Dilly <bdilly@profusion.mobi>
Mon, 1 Oct 2012 20:44:19 +0000 (20:44 +0000)
It's weird, but looks like wrap mode of the spinner is broken at least
since the move of elm to trunk.

The current code:

  if (sd->wrap)
     {
        while (new_val < sd->val_min)
          new_val = sd->val_max + new_val + 1 - sd->val_min;
        while (new_val > sd->val_max)
          new_val = sd->val_min + new_val - sd->val_max - 1;
     }

doesn't seems correct. Since even the documented example would fails:

 * E.g.:
 * @li min value = 10
 * @li max value = 50
 * @li step value = 20
 * @li displayed value = 20
 *
 * When the user decrement value (using left or bottom arrow), it will
 * displays @c 40, because max - (min - (displayed - step)) is
 * @c 50 - (@c 10 - (@c 20 - @c 20)) = @c 40.

With the current code the value will be 41.

It also could lead to values above min, like happens on the first spinner test,
when you could go to -50.5 because new value will become:
 250 + (-50.5) + 1 - (-50) in the first while() and later since these value
 is bigger then 250, would go back to -50.5 ...

So, a reasonable algorithm would be

  if (sd->wrap)
     {
        if (new_val < sd->val_min)
          new_val = sd->val_max + new_val - sd->val_min;
        else if (new_val > sd->val_max)
          new_val = sd->val_min + new_val - sd->val_max;
     }

But it doesn't works fine for cases like the months spinners test, when you
have min = 1, max = 12, step = 1 and each option should be displayed with
wrap. This algorithm would wraps from 1 to 11, so would skip December...

So, I think just going to the max value when min is reached is the better
choice.

   if (sd->wrap)
     {
        if (new_val < sd->val_min)
          new_val = sd->val_max;
        else if (new_val > sd->val_max)
          new_val = sd->val_min;
     }

SVN revision: 77278

src/lib/elm_spinner.c
src/lib/elm_spinner.h

index 4f25696..e841c88 100644 (file)
@@ -111,10 +111,10 @@ _value_set(Evas_Object *obj,
 
    if (sd->wrap)
      {
-        while (new_val < sd->val_min)
-          new_val = sd->val_max + new_val + 1 - sd->val_min;
-        while (new_val > sd->val_max)
-          new_val = sd->val_min + new_val - sd->val_max - 1;
+        if (new_val < sd->val_min)
+          new_val = sd->val_max;
+        else if (new_val > sd->val_max)
+          new_val = sd->val_min;
      }
    else
      {
index 187f1d8..5e1cd8f 100644 (file)
@@ -215,14 +215,16 @@ EAPI double      elm_spinner_value_get(const Evas_Object *obj);
  * Disabled by default. If disabled, when the user tries to increment the
  * value,
  * but displayed value plus step value is bigger than maximum value,
- * the spinner
- * won't allow it. The same happens when the user tries to decrement it,
- * but the value less step is less than minimum value.
+ * the new value will be the maximum value.
+ * The same happens when the user tries to decrement it,
+ * but the value less step is less than minimum value. In this case,
+ * the new displayed value will be the minimum value.
  *
- * When wrap is enabled, in such situations it will allow these changes,
- * but will get the value that would be less than minimum and subtracts
- * from maximum. Or add the value that would be more than maximum to
- * the minimum.
+ * When wrap is enabled, when the user tries to increment the value,
+ * but displayed value plus step value is bigger than maximum value,
+ * the new value will be the minimum value. When the the user tries to
+ * decrement it, but the value less step is less than minimum value,
+ * the new displayed value will be the maximum value.
  *
  * E.g.:
  * @li min value = 10
@@ -231,8 +233,7 @@ EAPI double      elm_spinner_value_get(const Evas_Object *obj);
  * @li displayed value = 20
  *
  * When the user decrement value (using left or bottom arrow), it will
- * displays @c 40, because max - (min - (displayed - step)) is
- * @c 50 - (@c 10 - (@c 20 - @c 20)) = @c 40.
+ * displays @c 50.
  *
  * @see elm_spinner_wrap_get().
  *