Created attachment 189229 [details] [review]
st-scroll-view-fade: Don't fade horizontal scrollbar

Take the height of the horizontal scrollbar into account when calculating
the fade area, so that only the scrollview's content is faded but not
the horizontal scrollbar.

Created attachment 189230 [details] [review]
st-scroll-view-fade: Don't fade horizontal scrollbar

Take the height of the horizontal scrollbar into account when calculating
the fade area, so that only the scrollview's content is faded but not
the horizontal scrollbar.

---

Whitespace fix.

Created attachment 189237 [details] [review]
StScrollView: Expose scrollbars visibility as property

Add two boolean readonly properties that tell whether
the scrollbars are visible or not.

Created attachment 189238 [details] [review]
StScrollViewFade: Fix scrollbar size handling

Only skip the areas of the scrollbars when they are invisible
and add take the horizontal scrollbar into account as well
when calculating the faded area.

Created attachment 189239 [details] [review]
StScrollViewFade: Fix scrollbar size handling

Only skip the areas of the scrollbars when they are invisible
and add take the horizontal scrollbar into account as well
when calculating the faded area.

Comment on attachment 189237 [details] [review]
StScrollView: Expose scrollbars visibility as property

You should notify on property changes. (ie, call g_object_notify(scrollbar, "hscrollbar-visible") when the value of that property changes)

Comment on attachment 189239 [details] [review]
StScrollViewFade: Fix scrollbar size handling

looks right, although I'm not sure how to test since I don't know where we have both horizontal scrollbars and vertical fading.

>+    cogl_program_set_uniform_1f (self->program, self->scrollbar_width_uniform, v_scroll_visible ? clutter_actor_get_width (vscroll) : 0);

fwiw, I think you could use "CLUTTER_ACTOR_IS_VISIBLE (vscroll)" rather than adding the new properties

(In reply to comment #7)
> (From update of attachment 189239 [details] [review])
> looks right, although I'm not sure how to test since I don't know where we have
> both horizontal scrollbars and vertical fading.

I tested it with tests/interactive/scroll-view-sizing.js

> >+    cogl_program_set_uniform_1f (self->program, self->scrollbar_width_uniform, v_scroll_visible ? clutter_actor_get_width (vscroll) : 0);
> 
> fwiw, I think you could use "CLUTTER_ACTOR_IS_VISIBLE (vscroll)" rather than
> adding the new properties

I initially had that but it didn't work (neither did IS_MAPPED) so I ended up doing it that way.

Comment on attachment 189239 [details] [review]
StScrollViewFade: Fix scrollbar size handling

ok, yeah, this one is fine. the other still should get some g_object_notify()s

Created attachment 190939 [details] [review]
StScrollView: Expose scrollbars visibility as property

Add two boolean readonly properties that tell whether
the scrollbars are visible or not.

---

*) Add g_object_notify()

Comment on attachment 190939 [details] [review]
StScrollView: Expose scrollbars visibility as property

>+     case PROP_VSCROLLBAR_VISIBLE:

indented one space too many

>+  if (priv->hscrollbar_visible != hscrollbar_visible)
>+    {
>+      priv->hscrollbar_visible = hscrollbar_visible;
>+      g_object_notify (G_OBJECT (actor), "hscrollbar-visible");
>+    }
>+
>+  if (priv->vscrollbar_visible != vscrollbar_visible)
>+    {
>+      priv->vscrollbar_visible = vscrollbar_visible;
>+      g_object_notify (G_OBJECT (actor), "vscrollbar-visible");
>+    }

put g_object_freeze_notify (G_OBJECT (actor)) before that, and g_object_thaw_notify (G_OBJECT (actor)) after (so that it's not possible for a signal handler to run in between the two sections).

>+  pspec = g_param_spec_boolean ("vscrollbar-visible",
>+                                "Horizontal Scrollbar Visibility",

s/Horizontal/Vertical/

ok to commit with those fixes

Attachment 189239 [details] pushed as 620330d - StScrollViewFade: Fix scrollbar size handling
Attachment 190939 [details] pushed as 3765acc - StScrollView: Expose scrollbars visibility as property