Code review comment for lp:~samuel-buffet/entertainer/fixme-27

Revision history for this message
Matt Layman (mblayman) wrote :

Samuel, first, let me add, this looks sweet! Okay, on to the comment (I think this is 95% complete):

For the methods that are private to scroll_menu can you please add a single underscore to them to give a visual indicator that they are private? For example, actor_notify_y to _actor_notify_y.

The methods I'm referring to are:
actor_notify_y
get_opacity_for_y
scroll_items_down
scroll_items_up

Just for my knowledge can you explain the concepts around connect "new-frame" and connect "notify::y". I'm assuming that these come directly from clutter and should be applied whenever we do animation, but since your code seems to work so well, I was hoping you might explain it so I could apply the concept to any frontend code that I happen to work on.

I'm going to go ahead and approve on the condition that my comment above is addressed.

review: Approve

« Back to merge proposal