Created attachment 310066 [details] [review]
properties: Align the source label

Since GtkLinkButton is a button, it has some extra padding.
Using a GtkLabel with markup we have the same functionality
without the extra padding

Looks good, pushed to master.

Attachment 310066 [details] pushed as 4478068 - properties: Align the source label

Reopening.

While this fixes the horizontal alignment issue (thanks for that!), it introduces a vertical size issue. Property dialogs that don't have a GtkEntry (eg., remote items) have thinner rows now.

Created attachment 310106 [details] [review]
Revert "properties: Align the source label"

This reverts commit 44780682c56ee436746d8f6cf902d8e07fe7d9f3.

Created attachment 310107 [details] [review]
style: Align the source label

Instead of using a GtkLabel with markup, remove the extra
padding with css

(In reply to Debarshi Ray from comment #3)
> Reopening.
> 
> While this fixes the horizontal alignment issue (thanks for that!), it
> introduces a vertical size issue. Property dialogs that don't have a
> GtkEntry (eg., remote items) have thinner rows now.

Could that be fixed with a vertical size group instead of reverting to buttons?

(In reply to Cosimo Cecchi from comment #6)
> (In reply to Debarshi Ray from comment #3)
> > Reopening.
> > 
> > While this fixes the horizontal alignment issue (thanks for that!), it
> > introduces a vertical size issue. Property dialogs that don't have a
> > GtkEntry (eg., remote items) have thinner rows now.
> 
> Could that be fixed with a vertical size group instead of reverting to
> buttons?

A vertical size group might work, but it would need a hack. Since we want to retain the height of the rows across all kinds of dialogs (with and without GtkEntries), we have to always add a row with a hidden entry for that.

Got it; feel free to revert to buttons then :-)
I think you should remove padding from both sides though, or RTL might break.

Created attachment 310133 [details] [review]
properties: Port to templates

Created attachment 310136 [details] [review]
properties: Make all rows with the same height

Created attachment 310196 [details] [review]
properties: Port to templates

Review of attachment 310196 [details] [review]:

Thanks for the port, Alessandro.

There is one unsolved problem in dialogs that have an entry, but not a creation date. eg., a local collection. There is too much space between the rows. I think something is causing the GtkGrid to expand vertically.

::: data/ui/properties-dialog.ui
@@ +45,3 @@
+            <property name="orientation">vertical</property>
+            <property name="row_spacing">6</property>
+            <property name="column_spacing">24</property>

I would suggest setting row-homogeneous to TRUE here. Otherwise, even dialogs with a GtkEntry become shorter. Actually, the way you have arranged the widgets, I think, making the rows homogeneous is enough to ensure that rows have the same height with or without the GtkEntry.

Removing the column-homogenous is OK, because I do not know how it worked the way it did till now and it looks better without it. Even then, we should mention it in the commit message.

@@ +184,3 @@
+                <property name="visible">True</property>
+                <property name="can_focus">False</property>
+                <property name="orientation">vertical</property>

The GtkGrid needs to have valign=center. Otherwise, titleEntry/titleLabel won't be aligned in dialogs without a creation date.

@@ +192,3 @@
+                    <property name="valign">center</property>
+                    <property name="hexpand">True</property>
+                    <property name="vexpand">True</property>

Don't set the valign and vexpand here.

@@ +206,3 @@
+                    <property name="no_show_all">True</property>
+                    <property name="valign">center</property>
+                    <property name="vexpand">True</property>

Ditto.

::: po/POTFILES.in
@@ +10,3 @@
 [type: gettext/glade]data/ui/preview-context-menu.ui
 [type: gettext/glade]data/ui/preview-menu.ui
+[type: gettext/glade]data/ui/properties-dialog.ui

And we can remove src/properties.js.

::: src/properties.js
@@ +54,2 @@
         let toplevel = Application.application.get_windows()[0];
+        this.parent({ transient_for: toplevel, use_header_bar: 1 });

'true', not '1'

@@ +89,3 @@
+            this._author.show()
+            this._authorData.show();
+            this._authorData.set_text(doc.author);

We usually call show after setting the text.

@@ +140,3 @@
+            this._dateCreatedData.set_text(dateCreated.format('%c'));
+            this._dateCreatedData.show();
+            this._dateCreated.show();

This is missing from InternalChildren.

Created attachment 310347 [details] [review]
properties: Port to templates

I addressed the above issues, but we still need to figure out why things are expanding vertically for local collections.

Created attachment 312448 [details] [review]
properties: Port to templates

Columns are no longer set to homogeneous because we don't want the left
column to be as wide as the right. Earlier, we had a gutter of empty
space on the left edge, which becomes even wider when used with
templates.
Rows aren't set to homogeneous as well because in some dialog there is
too much space between them. The right visual effect will be obtained
using a GtkSizeGroup

Created attachment 312449 [details] [review]
properties: Make all rows with the same height

Created attachment 317013 [details] [review]
properties: Make all rows with the same height

(In reply to Bastien Nocera from comment #16)
> Created attachment 317013 [details] [review] [review]
> properties: Make all rows with the same height

This stops the rows being overly big if there's no text entry in the panel.

GNOME is going to shut down bugzilla.gnome.org in favor of gitlab.gnome.org.
As part of that, we are mass-closing older open tickets in bugzilla.gnome.org
which have not seen updates for a longer time (resources are unfortunately
quite limited so not every ticket can get handled).

If you can still reproduce the situation described in this ticket in a recent
and supported software version, then please follow
  https://wiki.gnome.org/GettingInTouch/BugReportingGuidelines
and create a new ticket at
  https://gitlab.gnome.org/GNOME/gnome-documents/-/issues/

Thank you for your understanding and your help.