Created attachment 181060 [details] [review]
StScrollViewFade: Make fade-offset a property rather than a hardocded constant

Comment on attachment 181060 [details] [review]
StScrollViewFade: Make fade-offset a property rather than a hardocded constant

>-#define FADE_OFFSET 68.0f

you might as well keep that, renamed to DEFAULT_FADE_OFFSET, rather than hardcoding "68.0f" in two places.

>+    case PROP_FADE_OFFSET:
>+      st_scroll_view_fade_set_offset (self, g_value_get_int (value));

should be get_float()

>+static void
>+st_scroll_view_fade_constructed (GObject *object)
>+{
>+  StScrollViewFade *self = ST_SCROLL_VIEW_FADE (object);
>+  self->fade_offset = 68.0f;
>+}

you can do that from st_scroll_view_fade_init()

oh, and there's a typo in the summary ("hardocded")

(In reply to comment #2)
> (From update of attachment 181060 [details] [review])
> >-#define FADE_OFFSET 68.0f
> 
> you might as well keep that, renamed to DEFAULT_FADE_OFFSET, rather than
> hardcoding "68.0f" in two places.

OK makes sense.

> >+    case PROP_FADE_OFFSET:
> >+      st_scroll_view_fade_set_offset (self, g_value_get_int (value));
> 
> should be get_float()

Oops.

> >+static void
> >+st_scroll_view_fade_constructed (GObject *object)
> >+{
> >+  StScrollViewFade *self = ST_SCROLL_VIEW_FADE (object);
> >+  self->fade_offset = 68.0f;
> >+}
> 
> you can do that from st_scroll_view_fade_init()

Yes I can with the small difference that it is a nop (so I went with _constructed).

(In reply to comment #3)
> oh, and there's a typo in the summary ("hardocded")

Oops.

Created attachment 181062 [details] [review]
StScrollViewFade: Make fade-offset a property rather than a hardcoded constant

Created attachment 181070 [details] [review]
use _init rather than _constructed, and fix indentation

Attachment 181062 [details] pushed as 07bde82 - StScrollViewFade: Make fade-offset a property rather than a hardcoded constant