Merge lp:~philip.scott/granite/Avatar into lp:~elementary-pantheon/granite/granite

[Felipe Escoto]

New widget: Avatar

Moved the Avatar class i used to draw the user's image from the new Wingpanel indicator into it's own class. It's also the same that is used in Greeter and could also be used in switchboard's user plug.

[kay van der Zander (kay20)]

Just a tip, i notice you use tabs instead of spaces

Code style quote "Vala code is indented using 4 spaces for consistency and readability"

The outlining is consistent that is what matters.

Line 49 see dif, outline could be better.
Line 115 remove white line.

Draw_connect could be made better readable.
like i learned you.
i also find it wierd that this.draw.connect always returned false. I doubt this is right.

review: Needs Fixing (extra set eyes.)

[Felipe Escoto (philip.scott)]

Corrected the code style.

draw.connect IS actually supposed to always return false to propagate the event further (see http://valadoc.org/#!api=gtk+-3.0/Gtk.Widget.draw)

And for the outlines.... if you or anyone else has any suggestions, i'll gladly change them :)

[Rico Tzschichholz (ricotz)]

No offence, but this looks awful.

* gobject-style construction with construct properties
* gtk.builder compatibility
* use "public override bool draw (Cairo.Context cr) { ... }"

[Adam Bieńkowski (donadigo)]

As others said you should use overriding for draw method. I wanted to point other issues like:
* Do not use new int margin, margin is a property reserved by Gtk and shouldn't be used in calculations, rather use standalone variable name like: avatar_margin.

* There is currently no way of getting the pixbuf directly from the avatar. There should be a method called get_pixbuf that returns it.

review: Needs Fixing

[Rico Tzschichholz (ricotz)]

review: Needs Fixing

[Rico Tzschichholz (ricotz)]

Go from here https://paste.debian.net/plain/288609

[Danielle Foré (danrabbit)]

Please don't forget to add it to Granite Demo. Probably could add it to the Headerbar with the current user's avatar.

[Felipe Escoto (philip.scott)]

Alright, it's been added to Granite Demo. Do we want to show a tooltip text on it?

[Rico Tzschichholz (ricotz)]

The demo-code should give you the hint that this class has a weird way to be used. So I hope you remember what we talked about.

E.g. add a constructor which takes a icon-name or file-path

review: Needs Fixing

[Rico Tzschichholz (ricotz)]

Get rid of this additional margin let the css-class take care of it.
Only use "this.*"-syntax if it is actually needed.

review: Needs Fixing

[Danielle Foré (danrabbit)]

Yes having a tooltip would be good practice.

[xapantu (xapantu)]

Looks fine to me. I am not super-convinced by your "/var/lib/AccountsService/icons/" thing in the demo, but as it is only in the demo, let's do that for now.

review: Approve

[Victor Martinez (victored)]

Hey it looks good. Please consider my suggestions below.

review: Needs Fixing

[Victor Martinez (victored)]

There are other places where variable declarations should use 'var' too. Mixed style doesn't look consistent.

[Rico Tzschichholz (ricotz)]

> There are other places where variable declarations should use 'var' too. Mixed
> style doesn't look consistent.

Better realize what "unowned Gtk.StyleContext style_context = get_style_context ();" does. Don't use "var style_context = ..." since it is NOT the same.

[Victor Martinez (victored)]

Hey Rico I checked the generated C code and you're totally right about 'var'. There's no type inference for unowned. Please disregard that suggestion Phillip.

[Adam Bieńkowski (donadigo)]

Looks OK from the code perspectiive.

review: Approve (code)

[Victor Martinez (victored)]

Hey thanks for making the changes!

Code builds fine against current trunk and it does what it says on the tin. Changing 'draw_theme_background' to 'false' after an avatar image has already been drawn doesn't seem to queue a redraw. This property could be made private for now, or you could connect to the notify signal like you do for pixbuf. I guess we need this because the default avatar icon has its own shadow included?

[lp:~philip.scott/granite/Avatar]

863. By Felipe Escoto on 2015-09-01

New widget: Avatar

[Felipe Escoto (philip.scott)]

I decided to go with just setting draw_theme_background as private since it's only used when setting the default icon from within the widget, and to avoid extra triggers of the draw method.

[kay van der Zander (kay20)]

review: Approve (extra set eyes.)

=== modified file 'demo/GraniteDemo.vala'
--- demo/GraniteDemo.vala	2015-08-30 10:10:29 +0000
+++ demo/GraniteDemo.vala	2015-09-01 21:14:05 +0000
@@ -104,8 +104,11 @@
             home_button.hide ();
         });
 
+        var avatar = create_avatar ();
+
         headerbar.pack_start (home_button);
         headerbar.pack_end (about_button);
+        headerbar.pack_end (avatar);
         window.set_titlebar (headerbar);
     }
 
@@ -274,6 +277,25 @@
         });
     }
 
+    private Granite.Widgets.Avatar create_avatar () {
+        var username = GLib.Environment.get_user_name ();
+        var avatar = new Granite.Widgets.Avatar ();
+        var iconfile = @"/var/lib/AccountsService/icons/$username";
+
+        avatar.valign = Gtk.Align.CENTER;
+
+        try {
+            var pixbuf = new Gdk.Pixbuf.from_file (iconfile);
+            avatar.pixbuf = pixbuf.scale_simple (24, 24, Gdk.InterpType.BILINEAR);
+            avatar.set_tooltip_text ("Avatar widget: User image found");
+        } catch (Error e) {
+            avatar.show_default (24);
+            avatar.set_tooltip_text ("Avatar widget: User image not found, using fallback");
+        }
+
+        return avatar;
+    }
+
     public static int main (string[] args) {
         var application = new Granite.Demo ();
         return application.run (args);

=== modified file 'lib/CMakeLists.txt'
--- lib/CMakeLists.txt	2015-08-27 10:10:08 +0000
+++ lib/CMakeLists.txt	2015-09-01 21:14:05 +0000
@@ -20,6 +20,7 @@
     Services/IconFactory.vala
     Services/SimpleCommand.vala
     Widgets/AlertView.vala
+    Widgets/Avatar.vala
     Widgets/Utils.vala
     Widgets/WrapLabel.vala
     Widgets/AboutDialog.vala

=== added file 'lib/Widgets/Avatar.vala'
--- lib/Widgets/Avatar.vala	1970-01-01 00:00:00 +0000
+++ lib/Widgets/Avatar.vala	2015-09-01 21:14:05 +0000
@@ -0,0 +1,140 @@
+/*
+ *  Copyright (C) 2015 Granite Developers (https://launchpad.net/granite)
+ *
+ *  This program or library is free software; you can redistribute it
+ *  and/or modify it under the terms of the GNU Lesser General Public
+ *  License as published by the Free Software Foundation; either
+ *  version 3 of the License, or (at your option) any later version.
+ *
+ *  This library is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ *  Lesser General Public License for more details.
+ *
+ *  You should have received a copy of the GNU Lesser General
+ *  Public License along with this library; if not, write to the
+ *  Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+ *  Boston, MA 02110-1301 USA.
+ *
+ *  Authored by: Felipe Escoto <[e-mail]>, Rico Tzschichholz <[e-mail]>
+ */
+
+/**
+ * The Avatar widget allowes to theme & crop images with css BORDER_RADIUS property in the .avatar class.
+ *
+ */
+public class Granite.Widgets.Avatar : Gtk.EventBox {
+    private const string DEFAULT_ICON = "avatar-default";
+    private const string DEFAULT_STYLE = "avatar";
+    private const int EXTRA_MARGIN = 4;
+    private bool draw_theme_background = true;
+
+    public Gdk.Pixbuf? pixbuf { get; set; }
+
+    /**
+     * Makes new Avatar widget
+     *
+     */
+    public Avatar () {
+    }
+
+    /**
+    * Creates a new Avatar from the speficied pixbuf
+    *
+    * @param pixbuf image to be used
+    */
+    public Avatar.from_pixbuf (Gdk.Pixbuf pixbuf) {
+        Object (pixbuf: pixbuf);
+    }
+
+    /**
+     * Creates a new Avatar from the speficied filepath and icon size
+     *
+     * @param filepath image to be used
+     * @param size to scale the image
+     */
+    public Avatar.from_file (string filepath, int icon_size) {
+        try {
+            pixbuf = new Gdk.Pixbuf.from_file_at_scale (filepath, icon_size, icon_size, true);
+        } catch (Error e) {
+            show_default (icon_size);
+        }
+    }
+
+    /**
+     * Creates a new Avatar with the default icon from theme without applying the css style
+     *
+     * @param icon_size size of the icon to be loaded
+     */
+    public Avatar.with_default_icon (int icon_size) {
+        show_default (icon_size);
+    }
+
+    construct {
+        valign = Gtk.Align.CENTER;
+        halign = Gtk.Align.CENTER;
+        visible_window = false;
+        get_style_context ().add_class (DEFAULT_STYLE);
+
+        notify["pixbuf"].connect (refresh_size_request);
+    }
+
+    ~Avatar () {
+        notify["pixbuf"].disconnect (refresh_size_request);
+    }
+
+    private void refresh_size_request () {
+        if (pixbuf != null) {
+            set_size_request (pixbuf.width + EXTRA_MARGIN * 2, pixbuf.height + EXTRA_MARGIN * 2);
+            draw_theme_background = true;
+        } else {
+            set_size_request (0, 0);
+        }
+
+        queue_draw ();
+    }
+
+    /**
+     * Load the default avatar icon from theme into the widget without applying the css style
+     *
+     * @param icon_size size of the icon to be loaded
+     */
+    public void show_default (int icon_size) {
+        Gtk.IconTheme icon_theme = Gtk.IconTheme.get_default ();
+        try {
+            pixbuf = icon_theme.load_icon (DEFAULT_ICON, icon_size, 0);
+        } catch (Error e) {
+            stderr.printf ("Error setting default avatar icon: %s ", e.message);
+        }
+
+        draw_theme_background = false;
+    }
+
+    public override bool draw (Cairo.Context cr) {
+        if (pixbuf == null) {
+            return base.draw (cr);
+        }
+
+        unowned Gtk.StyleContext style_context = get_style_context ();
+        var width = get_allocated_width () - EXTRA_MARGIN * 2;
+        var height = get_allocated_height () - EXTRA_MARGIN * 2;
+
+        if (draw_theme_background) {
+            var border_radius = style_context.get_property (Gtk.STYLE_PROPERTY_BORDER_RADIUS, Gtk.StateFlags.NORMAL).get_int ();
+            var crop_radius = int.min (width / 2, border_radius * width / 100);
+
+            Granite.Drawing.Utilities.cairo_rounded_rectangle (cr, EXTRA_MARGIN, EXTRA_MARGIN, width, height, crop_radius);
+            Gdk.cairo_set_source_pixbuf (cr, pixbuf, EXTRA_MARGIN, EXTRA_MARGIN);
+            cr.fill_preserve ();
+            style_context.render_background (cr, EXTRA_MARGIN, EXTRA_MARGIN, width, height);
+            style_context.render_frame (cr, EXTRA_MARGIN, EXTRA_MARGIN, width, height);
+
+        } else {
+            Granite.Drawing.Utilities.cairo_rounded_rectangle (cr, EXTRA_MARGIN, EXTRA_MARGIN, width, height, 0);
+            Gdk.cairo_set_source_pixbuf (cr, pixbuf, EXTRA_MARGIN, EXTRA_MARGIN);
+            cr.fill_preserve ();
+        }
+
+        return Gdk.EVENT_STOP;
+    }
+}