Created attachment 273642 [details] [review]
Use header bar in the avatar dialog

Created attachment 273643 [details] [review]
Use header bar in the linked accounts dialog

Created attachment 273644 [details] [review]
Use header bar in the new contact dialog

Confirming that this is something we want. I'm not able to test patches right now, so feel free to attach screenshots if you want design feedback.

This is the relevant guidelines page: https://wiki.gnome.org/Design/HIG/Dialogs

Created attachment 273697 [details]
The new contact dialog screenshot

I just wonder if we still wants the title in this dialog.
It look too havvy.

Created attachment 273698 [details]
The avatar dialog screenshot

I wonder if we wants a 'Done' and 'Cancel' buttons in the
headerbar, instead only 'X' button.

I think there is a problem with this dialog also without headerbar,
in the dialog in the stable branch - if I click on any image, I can't
cancel this.

For the linked dialog I think all it right.

Thanks. I think we do want the title in the new contact dialog. Both words should be capitalised though.

(In reply to comment #0)
> See the patches.

Try using `true` instead of `1` in the patches. Please

(In reply to comment #8)
> Thanks. I think we do want the title in the new contact dialog. Both words
> should be capitalised though.

I pushed a patch for the title:
https://git.gnome.org/browse/gnome-contacts/commit/?id=26620

(In reply to comment #9)
> (In reply to comment #0)
> > See the patches.
> 
> Try using `true` instead of `1` in the patches. Please

Hmm, I use the interface from the web, not git bz.

Created attachment 273829 [details] [review]
Use header bar in the linked accounts dialog

According to the last design I see in gnome-mockup repo.

Review of attachment 273829 [details] [review]:

::: src/contacts-linked-accounts-dialog.vala
@@ +28,2 @@
   public LinkedAccountsDialog (Contact contact) {
+    Object (use_header_bar: 1);

Please use true.

(In reply to comment #13)
> Review of attachment 273829 [details] [review]:
> 
> ::: src/contacts-linked-accounts-dialog.vala
> @@ +28,2 @@
>    public LinkedAccountsDialog (Contact contact) {
> +    Object (use_header_bar: 1);
> 
> Please use true.

Please, see the following comments on bug 727068:
https://bugzilla.gnome.org/show_bug.cgi?id=727068#c15

Review of attachment 273829 [details] [review]:

I have the same objection I had before. Use true when the data type is boolean. Code readability matters.

::: src/contacts-linked-accounts-dialog.vala
@@ +28,2 @@
   public LinkedAccountsDialog (Contact contact) {
+    Object (use_header_bar: 1);

Please use true.

(In reply to comment #15)
> Review of attachment 273829 [details] [review]:
> 
> I have the same objection I had before. Use true when the data type is boolean.
> Code readability matters.
> 
> ::: src/contacts-linked-accounts-dialog.vala
> @@ +28,2 @@
>    public LinkedAccountsDialog (Contact contact) {
> +    Object (use_header_bar: 1);
> 
> Please use true.

The use-header-bar property it int, not bool, I can't use true instead 1.
contacts-linked-accounts-dialog.vala:29.13-29.32: error: Cannot convert from `bool' to `int'
    Object (use_header_bar: true);
            ^^^^^^^^^^^^^^^^^^^^

(In reply to comment #16)
> (In reply to comment #15)
> > Review of attachment 273829 [details] [review] [details]:
> > 
> > I have the same objection I had before. Use true when the data type is boolean.
> > Code readability matters.
> > 
> > ::: src/contacts-linked-accounts-dialog.vala
> > @@ +28,2 @@
> >    public LinkedAccountsDialog (Contact contact) {
> > +    Object (use_header_bar: 1);
> > 
> > Please use true.
> 
> The use-header-bar property it int, not bool, I can't use true instead 1.
> contacts-linked-accounts-dialog.vala:29.13-29.32: error: Cannot convert from
> `bool' to `int'
>     Object (use_header_bar: true);
>             ^^^^^^^^^^^^^^^^^^^^

Yeap, just saw it, in Gtk+-3.0 vapi file is marked as int

Comment on attachment 273644 [details] [review]
Use header bar in the new contact dialog

The new contact dialog is going away before 3.14, so we don't need to patch it

Review of attachment 273829 [details] [review]:

Pushed as 93bb4 - Use header bar in the linked accounts dialog

Review of attachment 273642 [details] [review]:

Pushed as 052ee - Use header bar in the avatar dialog

(In reply to comment #18)
> (From update of attachment 273644 [details] [review])
> The new contact dialog is going away before 3.14, so we don't need to patch it

What will be instead?

(In reply to comment #21)
> (In reply to comment #18)
> > (From update of attachment 273644 [details] [review] [details])
> > The new contact dialog is going away before 3.14, so we don't need to patch it
> 
> What will be instead?

See bug 699324. Thanks for the patches, Yosef!