(In reply to Davi from comment #0)
> - "Folders" are mixed with keys

Is that the real problem, or is the problem the current presentation of folders vs. keys? I have plans to add sorting options including folders first/last/mixed, possibly changing the default, but I’m not sure one or the other would really improve clarity, so it’s not a priority for now.

> - Columns are misaligned, depending on the data type of the entry
> - Rows of string entries have different heights, depending on the value of
> the string
> 
> Rows and columns should all have the same width and height, respectively,
> limiting the amount of visible content if necessary.

Rows of the same height are good to have, as are columns with the same width. But things depend of the content, and that’s where is the difficulty. More on that at the end.

> My ascii-art, 80-chars-per-line mockup below is a tentative to give a
> concrete proposal. It represents the view of a row. If there is interest, I
> might come up with an improved mockup, or even some patches.
> _____________________________________________________________________________
> 
>  key-name-bold-if-modified              Right aligned key value, might span a 
>  Dimmed key summary, ellipsize if ne... couple of lines, but then ellipsiz... 
> _____________________________________________________________________________

I had such ideas in mind at one point, but I think it would be suboptimal in practice. But testing is the only way to be sure, in fact, so if you want to do a patch, I’d be happy to test it. But I might have other ideas.

> What do you think?

The difficulty is the size taken by the content we want to display:

 * I think it’s important to see in all the cases the name of the key completely, even if it has been modify (and so in bold), and I see four solutions:
 — either the column change width when browsing folders with different key name length;
 — or we choose the max key name width, and loose many space in most cases;
 — or we have many space because part of the content is on another row –that’s your solution;
 — or we choose a reasonable space and extend it in corner cases –that’s the current solution.

 * I think it’s important to see most “summaries” completely, as it’s the best way to understand what’s the key for. Even in languages that take place (like french). There are some case where it should be ellipsized, because too long, but it should be an exception, not the common case.

 * I thought that the value should be displayed completely, as it was the main column on the previous design. That’s probably wrong, in fact. But for simple content like boolean value, integers, etc., it should probably anyway, and sometimes with integrated widgets (like for booleans).

So in my mind, for now, it’s more:
_____________________________________________________________________________

boolean       [ 0 | 1 ]       a boolean key that is used for true/false
_____________________________________________________________________________

empty-string  ""              a string that doesn’t take a big place as it...
_____________________________________________________________________________

string        "Lorem..." (v)  a string that has content, but a small summary
_____________________________________________________________________________

integer                  100  an integer value, maybe right aligned
_____________________________________________________________________________

key-with-long-name [ 0 | 1 ]  a key with long name that is displayed anyway
_____________________________________________________________________________

other-boolean-with-long-name [ 0 | 1 ]  that’s an abuse, but it’s unusual
_____________________________________________________________________________

with so all summaries aligned in quite all cases (not in corner-cases with keys with extremely long names, but that’s unusual), and the content only displayed if it is small, else put in a popover that shows if the button with down arrow is pressed.

If you want to work on this, I’m interested. :·)

*** Bug 769202 has been marked as a duplicate of this bug. ***

(In reply to Arnaud B. from comment #1)
> Is that the real problem, or is the problem the current presentation of
> folders vs. keys? I have plans to add sorting options including folders
> first/last/mixed, possibly changing the default, but I’m not sure one or the
> other would really improve clarity, so it’s not a priority for now.

A sorting option (I think could be a simple check like in nautilus) would be great.
 
> Rows of the same height are good to have, as are columns with the same
> width. But things depend of the content, and that’s where is the difficulty.
> More on that at the end.

> I had such ideas in mind at one point, but I think it would be suboptimal in
> practice. But testing is the only way to be sure, in fact, so if you want to
> do a patch, I’d be happy to test it. But I might have other ideas.
> 
> > What do you think?
> 
> The difficulty is the size taken by the content we want to display:
> 
>  * I think it’s important to see in all the cases the name of the key
> completely, even if it has been modify (and so in bold), and I see four
> solutions:
>  — either the column change width when browsing folders with different key
> name length;
>  — or we choose the max key name width, and loose many space in most cases;
>  — or we have many space because part of the content is on another row
> –that’s your solution;
>  — or we choose a reasonable space and extend it in corner cases –that’s the
> current solution.
> 
>  * I think it’s important to see most “summaries” completely, as it’s the
> best way to understand what’s the key for. Even in languages that take place
> (like french). There are some case where it should be ellipsized, because
> too long, but it should be an exception, not the common case.
> 
>  * I thought that the value should be displayed completely, as it was the
> main column on the previous design. That’s probably wrong, in fact. But for
> simple content like boolean value, integers, etc., it should probably
> anyway, and sometimes with integrated widgets (like for booleans).

I really think it is not useful to have the full content visible for large string or strv values. There is a focused view already in place for that, so I think the main role of the key list should be to scan over and quickly locate keys of interest.

> So in my mind, for now, it’s more:
> _____________________________________________________________________________
> 
> boolean       [ 0 | 1 ]       a boolean key that is used for true/false
> _____________________________________________________________________________
> 
> empty-string  ""              a string that doesn’t take a big place as it...
> _____________________________________________________________________________
> 
> string        "Lorem..." (v)  a string that has content, but a small summary
> _____________________________________________________________________________
> 
> integer                  100  an integer value, maybe right aligned
> _____________________________________________________________________________
> 
> key-with-long-name [ 0 | 1 ]  a key with long name that is displayed anyway
> _____________________________________________________________________________
> 
> other-boolean-with-long-name [ 0 | 1 ]  that’s an abuse, but it’s unusual
> _____________________________________________________________________________
> 
> with so all summaries aligned in quite all cases (not in corner-cases with
> keys with extremely long names, but that’s unusual), and the content only
> displayed if it is small, else put in a popover that shows if the button
> with down arrow is pressed.

My main gripe with this is that the key and summaries are too far apart from each other, while they refer to the same thing. I think the summary is more useful very close to the key (immediate right or bottom), like a "tooltip" of sorts. At the same time, the value seems "buried" and hard to recognize, specially if it is not aligned. It should be more separate from the key, and ... aligned? (I'm not O.C.D., I swear)

> If you want to work on this, I’m interested. :·)

Then take a look my patches, please :).

Created attachment 363231 [details] [review]
Fix rules half commented (I think?)

Created attachment 363232 [details] [review]
Reorganize key list rows for more uniform sizing and spacing

Created attachment 363233 [details] [review]
Do not make value label bold

Created attachment 363234 [details] [review]
Align boolean value switcher to the left

Created attachment 363235 [details] [review]
Change entry list selection mode to none

(In reply to Davi from comment #3)
> (In reply to Arnaud B. from comment #1)
> > Is that the real problem, or is the problem the current presentation of
> > folders vs. keys? I have plans to add sorting options including folders
> > first/last/mixed, possibly changing the default, but I’m not sure one or the
> > other would really improve clarity, so it’s not a priority for now.
> 
> A sorting option (I think could be a simple check like in nautilus) would be
> great.

https://bugzilla.gnome.org/show_bug.cgi?id=790116

> > I had such ideas in mind at one point, but I think it would be suboptimal in
> > practice. But testing is the only way to be sure, in fact, so if you want to
> > do a patch, I’d be happy to test it. But I might have other ideas.

In fact, seeing the result, I’m quite sure I’ve already done a simple patch in the same spirit, but didn’t go further. There are clearly some benefits to go this way, but there are some problems also. Needs more work, ideas at the end.

> I really think it is not useful to have the full content visible for large
> string or strv values. There is a focused view already in place for that, so
> I think the main role of the key list should be to scan over and quickly
> locate keys of interest.

I completely agree on that.

> My main gripe with this is that the key and summaries are too far apart from
> each other, while they refer to the same thing. I think the summary is more
> useful very close to the key (immediate right or bottom), like a "tooltip"
> of sorts.

Agree on that also.

> At the same time, the value seems "buried" and hard to recognize, specially
> if it is not aligned. It should be more separate from the key, and
> ... aligned? (I'm not O.C.D., I swear)

Ok, so please try something: right align. On small windows, there’s a problem where there’s a long summary and a boolean key (for example): the summary is on two lines, where there’s many space in the row. On big windows, there’s a problem because value could be really far away from the name, and that’s bad. Right aligning allows for big windows to fix a max width of the complete content (like is done in properties view), and for small windows to let the summary be on one line most of the time.

> Then take a look my patches, please :).

Looks like patch 1 corrects a stupid thing like I sometimes do, but it needs investigation on if my goal was to comment the whole rules or to just comment the “.non-symbolic” parts of it.

About the “Do not make value label bold” patch, I’d really prefer to have a clear visual mark for values that are edited, instead of relying on some random text being bold. Notably, with your patches, the modification of *the value* is visible on *the key name*, and that bugs me. Such a mark could even be a button that permits to reset easily, or to show initial value (needs testing of course).

Patch 5 breaks some things that rely on the selection, like pressing the “Menu” key, or searching next. But globally, there’s a need for a selection; for the keyboard shortcuts, first (“Menu”, but also things like “Toggle the boolean value” or “Reset key value”); for when there will be a real search secondly, where the search entry has the focus, but there’s a need to browse results.

It doesn’t look so cool in ascii, but that’s because it’s ascii.

_____________________________________________________________________________


boolean                                                            [ 0 | 1 ]
a boolean key that is used for true/false
_____________________________________________________________________________


empty-string                                                              ""           
a string that doesn’t take a big place as it is empty
_____________________________________________________________________________

string                                     "Lorem ipsum dolor sit amet,  
a string that has content, and a not       consectetur adipiscing elit. Sed 
so small summary                           non risus. Suspendisse lectus..."
_____________________________________________________________________________


integer                                                                  100                                   
an integer value is also right aligned
_____________________________________________________________________________


key-with-long-name                                                 [ 0 | 1 ]
a key with long name that is displayed anyway
_____________________________________________________________________________


other-boolean-with-long-name                                       [ 0 | 1 ]  
that’s an abuse, but it’s unusual
_____________________________________________________________________________

Comment on attachment 363231 [details] [review]
Fix rules half commented (I think?)

Bad CSS comments corrected.

(In reply to Arnaud B. from comment #9)
> Ok, so please try something: right align. On small windows, there’s a
> problem where there’s a long summary and a boolean key (for example): the
> summary is on two lines, where there’s many space in the row. On big
> windows, there’s a problem because value could be really far away from the
> name, and that’s bad. Right aligning allows for big windows to fix a max
> width of the complete content (like is done in properties view), and for
> small windows to let the summary be on one line most of the time.

I think next "Reorganize ..." patch is very close to your mockups on comment #10.

> Looks like patch 1 corrects a stupid thing like I sometimes do, but it needs
> investigation on if my goal was to comment the whole rules or to just
> comment the “.non-symbolic” parts of it.

I've re-based the current patches on top of your fix already.

> About the “Do not make value label bold” patch, I’d really prefer to have a
> clear visual mark for values that are edited, instead of relying on some
> random text being bold. Notably, with your patches, the modification of *the
> value* is visible on *the key name*, and that bugs me. Such a mark could
> even be a button that permits to reset easily, or to show initial value
> (needs testing of course).

The patch "Add a reset button..." goes in that direction, although it does not remove the "bold-label" code. That might be done later, if the button looks satisfactory. Note that the button solves an issue that boolean switches cannot be made bold.

*** WARNING: The reset button works, so clicking it will reset the value (or erase a schema-less key) directly and with no "undo" message. Please, test carefully. ***
 
> Patch 5 breaks some things that rely on the selection, like pressing the
> “Menu” key, or searching next. But globally, there’s a need for a selection;
> for the keyboard shortcuts, first (“Menu”, but also things like “Toggle the
> boolean value” or “Reset key value”); for when there will be a real search
> secondly, where the search entry has the focus, but there’s a need to browse
> results.

Yeah, I've rushed that, please disregard :).

Created attachment 363303 [details] [review]
Reorganize key list rows for more uniform sizing

Created attachment 363304 [details] [review]
Add a reset button to key list rows

Created attachment 363306 [details] [review]
Add a reset button to key list rows

Use a revealer instead of changing opacity. An opaque button captures still the clicks and the activate row event is not triggered.

(In reply to Davi from comment #12)
> I think next "Reorganize ..." patch is very close to your mockups on comment
> #10.

I have the feeling that this is the correct way to go for next dconf-editor UI iteration. Great work! Of course, a lot remains to do.

The main thing is for big windows, the content needs to be placed in a column centered, with a defined max width. As we don’t have the “max-width” CSS property, that’s many Gtk+ “hacks” to do, but that’s necessary so the values are not too far away from keys descriptions.

> > About the “Do not make value label bold” patch, I’d really prefer to have a
> > clear visual mark for values that are edited, instead of relying on some
> > random text being bold. Notably, with your patches, the modification of *the
> > value* is visible on *the key name*, and that bugs me. Such a mark could
> > even be a button that permits to reset easily, or to show initial value
> > (needs testing of course).
> 
> The patch "Add a reset button..." goes in that direction, although it does
> not remove the "bold-label" code. That might be done later, if the button
> looks satisfactory. Note that the button solves an issue that boolean
> switches cannot be made bold.

Testing it, I don’t feel really happy with it.
 * First, it looks dangerous for keys that contain important informations (like Polari’s “saved-channel-list” for example, a mapping of accounts with preferred channels…);
 * secondly, I don’t feel comfortable with adding an “undo” option, and a “confirm” one (done by switching in delay-mode) would really feel weird for a little boolean value change (notably);
 * in third place, it takes the focus (and it’s not a good idea to disable that), meaning that you have to <tab> multiple times to go to next row;
 * finally, it takes many place on the row (the ones where it is displayed, and the ones where it is not), and I don’t like that.
A simple visual mark, without interaction possible, might be better.

Here comes another thinking I had, without testing. You aligned the names of folders and keys; I’m not happy with all the empty space at the left, notably when browsing a folder containing only keys. Now that there’s a clear distinction between folder rows and keys rows, it might be the perfect place for having icons –exactly like the ones in a navigation list of a files browser– indicating the type of the key, and maybe conveying information about “edited or not” keys. There could be:
 * folders (like now);
 * maybe, schemas (an icon of folder with a little thing on it);
 * key with schema, default value (maybe a check mark, like “all is ok here”, ‘emblem-default-symbolic’);
 * key with schema, edited (‘document-edit-symbolic?’);
 * key without schema (maybe a ‘?’ in a circle icon);
 * key erased (the trash icon);
and maybe information about key awaiting for modification, in smaller icons, “emblems”:
 * modification delayed (for both keys without schema and keys with schema; maybe the clock icon, as used now);
 * key awaiting for suppression (for keys without schema; the trash icon used now);
 * and key awaiting for reset (for keys with schema –there’s a weird case were a key already reseted could be awaiting for reset; not sure of which icon to choose).
That would solve various “information lacking” problems.

Talking about padding, there’s a need of more padding at the right of rows, when you don’t apply the “reset button” patch. At least, there should be the same space on top/bottom and at the right of a toggle button.

There’s a problem with how long values should be justified: left like now, creating an unaesthetic alignment in the middle of the row, or right, harder to read? and, in any way, you should set the ‘xaling’ property to 1 (“right”).

> *** WARNING: The reset button works, so clicking it will reset the value (or
> erase a schema-less key) directly and with no "undo" message. Please, test
> carefully. ***

I have /ca/desrt/dconf-editor/Demo/ for that. :·)

Many thanks for working on this!

(In reply to Arnaud B. from comment #16)
> I have the feeling that this is the correct way to go for next dconf-editor
> UI iteration. Great work! Of course, a lot remains to do.

Sure thing.
 
> The main thing is for big windows, the content needs to be placed in a
> column centered, with a defined max width. As we don’t have the “max-width”
> CSS property, that’s many Gtk+ “hacks” to do, but that’s necessary so the
> values are not too far away from keys descriptions.

In this case, do you think only the row content should be centered or the entire listbox (i.e. should the white background and separators extend to the ends of the window)?

> Testing it, I don’t feel really happy with it.
>  * First, it looks dangerous for keys that contain important informations
> (like Polari’s “saved-channel-list” for example, a mapping of accounts with
> preferred channels…);
>  * secondly, I don’t feel comfortable with adding an “undo” option, and a
> “confirm” one (done by switching in delay-mode) would really feel weird for
> a little boolean value change (notably);

Such prominent button cannot be without protective measures. Either add confirmation/undo or abandon the button. (Actually, I think the whole app should have a full undo/redo stack, but I'm not sure how that would interact with delayed editing and this is talk for another bug report).

>  * in third place, it takes the focus (and it’s not a good idea to disable
> that), meaning that you have to <tab> multiple times to go to next row;

You can go to the previous/next row pressing up/down, so I don't think this is an issue.

>  * finally, it takes many place on the row (the ones where it is displayed,
> and the ones where it is not), and I don’t like that.
> A simple visual mark, without interaction possible, might be better.

Ok.

> Here comes another thinking I had, without testing. You aligned the names of
> folders and keys; I’m not happy with all the empty space at the left,
> notably when browsing a folder containing only keys. Now that there’s a
> clear distinction between folder rows and keys rows, it might be the perfect
> place for having icons –exactly like the ones in a navigation list of a
> files browser– indicating the type of the key, and maybe conveying
> information about “edited or not” keys. There could be:
>  * folders (like now);
>  * maybe, schemas (an icon of folder with a little thing on it);
>  * key with schema, default value (maybe a check mark, like “all is ok
> here”, ‘emblem-default-symbolic’);
>  * key with schema, edited (‘document-edit-symbolic?’);
>  * key without schema (maybe a ‘?’ in a circle icon);
>  * key erased (the trash icon);
> and maybe information about key awaiting for modification, in smaller icons,
> “emblems”:
>  * modification delayed (for both keys without schema and keys with schema;
> maybe the clock icon, as used now);
>  * key awaiting for suppression (for keys without schema; the trash icon
> used now);
>  * and key awaiting for reset (for keys with schema –there’s a weird case
> were a key already reseted could be awaiting for reset; not sure of which
> icon to choose).
> That would solve various “information lacking” problems.

I think for keys with schema, default-valued and no delayed action scheduled, the left margin should be left blank (i.e. "no news is good news"). Adding an icon for "nothing to report" feels like clutter to me. "information lacking" should be fixed, though, and I agree that "icon placeholder" could be used for that. I'll try something on my next patches.

> Talking about padding, there’s a need of more padding at the right of rows,
> when you don’t apply the “reset button” patch. At least, there should be the
> same space on top/bottom and at the right of a toggle button.

Ok.

> There’s a problem with how long values should be justified: left like now,
> creating an unaesthetic alignment in the middle of the row, or right, harder
> to read? and, in any way, you should set the ‘xaling’ property to 1
> (“right”).

Ok.

(In reply to Davi from comment #17)
> (In reply to Arnaud B. from comment #16)
> In this case, do you think only the row content should be centered or the
> entire listbox (i.e. should the white background and separators extend to
> the ends of the window)?

If we were displaying the content of a schema (a function that could be added at one point), I’d say I’d like a look similar to the one the GNOME Control Center panels, so white boxes on top of the grey background.

But that’s not really the case here: there are folders, there are keys not linked to a schema. The content is not fixed, it’s the result of a user requesting the objects in a given path. So, I think the background should be all-white (listbox and around), but with separator maybe not extending more than a row; like for the GNOME Shell’s search results, for example. Needs testing, of course and as usual.

> Such prominent button cannot be without protective measures. Either add
> confirmation/undo or abandon the button. (Actually, I think the whole app
> should have a full undo/redo stack, but I'm not sure how that would interact
> with delayed editing and this is talk for another bug report).

An undo/redo stack is hard to do, really. It could interact badly with more or less all the app functions. And it might even be easier to have it in GLib (to restore options to a given state of whatever “schema”/app). I’d say not to hope for it.

> >  * in third place, it takes the focus (and it’s not a good idea to disable
> > that), meaning that you have to <tab> multiple times to go to next row;
> 
> You can go to the previous/next row pressing up/down, so I don't think this
> is an issue.

I like to offer a correct focus path as that’s important for accessibility; the <tab> key is “the navigation key” for people with disabilities (that do not see the window, or that have a limited number of possible actions). That’s not on the top priorities list, just something I always keep in mind.

> I think for keys with schema, default-valued and no delayed action
> scheduled, the left margin should be left blank (i.e. "no news is good
> news"). Adding an icon for "nothing to report" feels like clutter to me.

Possible. Probable, even. But “needs testing.” :·)

Happy hacking!

Created attachment 363415 [details] [review]
Reorganize key list rows for more uniform sizing

This iteration includes some more padding to the right and sets a maximum width for list box rows, so keys and values are not kept too far in wide windows. I struggled a bit to manage to do the "max width" thing, and I'm not sure the solution I found is the best. Seems to work.

The patch is rebased on top of the master branch as of now.

Created attachment 363416 [details] [review]
Reorganize key list rows for more uniform sizing

Use real separators between rows.

Created attachment 363418 [details] [review]
Reorganize key list rows for more uniform sizing

Everything the patch does, so far:

- Move summary below key name for greater proximity (and apply
  "dim-label")
- Align key value to the right for better recognition
- Limit amount of visible text content to 3 lines (2 when
  small-keys-list-rows is enabled*)
- Uniform height for key rows
- Add separators between rows
- Limit row width when window is too wide
- Align folder names with key names
- Move "delayed modification" emblem to the left (aligned with folder
  emblem)*

* new in this iteration

(No computer)

Reading the code only, I find the wrapper a little surprising:
 * is it really needed?
 * if so, it should not inherit from ListBoxRow, should dit?

Based on what I imagine of the result, rows should have ronded corners when they don’t take all the window width.

I’m not exactly happy to add a GSettings un KeyListBoxRow, but that can be needed. If so, protected get.

And I don’t see xalign 1, so it remains to be added.

I’ll test that, thanks.

(In reply to Arnaud B. from comment #22)
> (No computer)
> 
> Reading the code only, I find the wrapper a little surprising:
>  * is it really needed?
>  * if so, it should not inherit from ListBoxRow, should dit?

One of the difficulties I had is that the ListBox only has ListBoxRow children, i.e. if the added widget is not a ListBoxRow, it is wrapped in a default ListBoxRow that always expands completely. So I had to create a custom centered ListBoxRow. Like I mentioned before, there might be a better way, some Gtk-guru's advice is welcome!
 
> Based on what I imagine of the result, rows should have ronded corners when
> they don’t take all the window width.

Ok.

> I’m not exactly happy to add a GSettings un KeyListBoxRow, but that can be
> needed. If so, protected get.

I can see a couple of alternatives to that:
1. Update the lines property manually (i.e. iterating over the list of rows)
2. Wait for next refresh of the view (i.e. set the lines property using a constructor parameter)

For now, I'm just making a protected get :).

> And I don’t see xalign 1, so it remains to be added.

I think it is there! Please, check again. :)

> I’ll test that, thanks.

Created attachment 363508 [details] [review]
Reorganize key list rows for more uniform sizing

Improve appearance for large windows (round borders and some margin for rows)

(With a computer)

> One of the difficulties I had is that the ListBox only has ListBoxRow
> children, i.e. if the added widget is not a ListBoxRow, it is wrapped in a
> default ListBoxRow that always expands completely. So I had to create a custom
> centered ListBoxRow.

Looks good in fact, that’s just GtkListBox doing some magic I didn’t knew about.

> I can see a couple of alternatives to that:
> 1. Update the lines property manually (i.e. iterating over the list of rows)
> 2. Wait for next refresh of the view (i.e. set the lines property using a
> constructor parameter)

I would probably have gone with the 1), that’s less bindings and no GSettings linked everywhere (I like to maintain a small number of entry points of application settings, to be able to check what’s happening exactly when a setting is changed). But if you don’t feel going that way, keep your code, it’s does the job.

> Improve appearance for large windows (round borders and some margin for rows)

Looks good! That’s not the design I had in mind, but it’s even probably better. ^^ Things to improve in CSS:
 * when the window changes size, add a transition for rounded corners (and probably to margins, in the same timing);
 * when the window changes size, there’s a small change on rows height, probably a 1px border thing, try to remove it;
 * when small-keys-list-rows is true, maybe disable rounded corners, the margins are taking place (I’m not using this option, but I’d like to keep it having love, as some people like that);
 * when small-keys-list-rows is true, maybe make folder rows even smaller?

Also, I see warning in the terminal, when going back and forward in a folder path with boolean value switch. (“(dconf-editor:825): Gtk-WARNING **: 08:20:27.013: Allocating size to RegistryView 0xd50410 without calling gtk_widget_get_preferred_width/height(). How does the code know the size to allocate?”)

And I have triggered a Gtk-CRITICAL one time, but I don’t know how (“(dconf-editor:31500): Gtk-CRITICAL **: 07:53:48.557: gtk_widget_queue_draw: assertion 'GTK_IS_WIDGET (widget)' failed”), needs investigation.

(In reply to Arnaud B. from comment #25)
> > I can see a couple of alternatives to that:
> > 1. Update the lines property manually (i.e. iterating over the list of rows)
> > 2. Wait for next refresh of the view (i.e. set the lines property using a
> > constructor parameter)
> 
> I would probably have gone with the 1), that’s less bindings and no
> GSettings linked everywhere (I like to maintain a small number of entry
> points of application settings, to be able to check what’s happening exactly
> when a setting is changed). But if you don’t feel going that way, keep your
> code, it’s does the job.

I moved to something along 1 and I actually like it better, now that I see it.
 
> Looks good! That’s not the design I had in mind, but it’s even probably
> better. ^^ 

If you say so... :)

> Things to improve in CSS:
>  * when the window changes size, add a transition for rounded corners (and
> probably to margins, in the same timing);

I've tried adding a transition for vertical margins/padding, but rows move in a weird way. Removing padding while adding margin (and vice-versa) is not smooth and the effect stacks for the lower rows. Maybe I was doing something wrong. For this iteration I've set transitions for min-height, border-radius, margin/padding-left/right. Note that min-height is only changed for small-keys-list-rows now, so the transition looks ok.

>  * when the window changes size, there’s a small change on rows height,
> probably a 1px border thing, try to remove it;

I've tried to avoid that changing the padding/margin accordingly. It does not happen for me now. But that probably needs improvement to deal with themes and font sizes. Not sure how to proceed.

>  * when small-keys-list-rows is true, maybe disable rounded corners, the
> margins are taking place (I’m not using this option, but I’d like to keep it
> having love, as some people like that);

I've cleaned up the CSS modifications a bit, so it might be better now. If you still feel it takes too much space (it's 0.2em, around 2px total I think), I'll remove border-radius and margins from small keys.

>  * when small-keys-list-rows is true, maybe make folder rows even smaller?

Done.
 
> Also, I see warning in the terminal, when going back and forward in a folder
> path with boolean value switch. (“(dconf-editor:825): Gtk-WARNING **:
> 08:20:27.013: Allocating size to RegistryView 0xd50410 without calling
> gtk_widget_get_preferred_width/height(). How does the code know the size to
> allocate?”)

Fixed, I think. Wrappers and row separators now call base.get_preferred_width first and then override the natural_width. Seems to satisfy Gtk expectations.
 
> And I have triggered a Gtk-CRITICAL one time, but I don’t know how
> (“(dconf-editor:31500): Gtk-CRITICAL **: 07:53:48.557:
> gtk_widget_queue_draw: assertion 'GTK_IS_WIDGET (widget)' failed”), needs
> investigation.

Never seen this one.

Created attachment 363782 [details] [review]
Reorganize key list rows for more uniform sizing

- Rework propagation of small-keys-list-rows property to change number of lines in labels;
- Fix Gtk warning about allocating without asking
- Improve spacing properties in CSS

(In reply to Davi from comment #26)
> I've tried adding a transition for vertical margins/padding, but rows move
> in a weird way. Removing padding while adding margin (and vice-versa) is not
> smooth and the effect stacks for the lower rows. Maybe I was doing something
> wrong. For this iteration I've set transitions for min-height,
> border-radius, margin/padding-left/right. Note that min-height is only
> changed for small-keys-list-rows now, so the transition looks ok.

That’s quite better already. :) There are some oddities, but that may be or not the CSS; it also might be Gtk+ doing things wrong (I’ve been suspecting such a bug for some times[1]), or the ListBox needing a refresh to correctly display things. Let’s say that would be enough for pushing, even if that might need a little more work.

> I've tried to avoid that changing the padding/margin accordingly. It does
> not happen for me now. But that probably needs improvement to deal with
> themes and font sizes. Not sure how to proceed.

I’m usually not targeting external theme designers, even if I can add some rules to help them doing their work. Now that Ubuntu decided (back) to use a GNOME experience, I might have a look to their Gtk+ theme also, but that’s all. Basically, don’t stress with that.

Sadly, the rounded corners do not work well with HighContrast, and that’s important. Dconf-editor didn’t have for now the need to handle this kind of themes specifically, so I didn’t do the job to have a .high-contrast class added (or a .adwaita one removed?) when it’s used. Correcting that might be a story for another patch, but the job has to be done before stable release.

One thing I usually test also is the “Big Text” accessibility feature, as it is in the Universal Access menu of GNOME (you should always display it to have fun regularly with accessibility functions). Looks like all is working correctly here.

> I've cleaned up the CSS modifications a bit, so it might be better now. If
> you still feel it takes too much space (it's 0.2em, around 2px total I
> think), I'll remove border-radius and margins from small keys.

I’d prefer to remove the rounded corner, yes. That’s not a story of place with this patch, more of cluttering: there is a “visual noise” in having a small white space between a grey row and a blue selected item, and people that like small rows are usually the same who complain about visual noise. :)

One thing I’d like you to look at is the transition between with and without small-keys-list-rows. You should go in the Gtk+ inspector, tab “Visual”, and slow down animations (up to 8 times); and then play with the option (using `gsettings`) in various places. Notably, there’s a margin jump on the left for keys, where the CSS handles that correctly for folders. You might want either to unify the width of this margin between the two states (might be an idea, now that there’s a margin for keys), or to make the transition smooth as it is for folders.

[1] https://bugzilla.gnome.org/show_bug.cgi?id=764524

Oh, and you should set_header_func() at construct, only one time.

(In reply to Arnaud B. from comment #28)
> That’s quite better already. :) There are some oddities, but that may be or
> not the CSS; it also might be Gtk+ doing things wrong (I’ve been suspecting
> such a bug for some times[1]), or the ListBox needing a refresh to correctly
> display things. Let’s say that would be enough for pushing, even if that
> might need a little more work.

I don't know what else to try here. Even transitioning only the selected row (where the effect is visible) gives noticeable stuttering.

> Sadly, the rounded corners do not work well with HighContrast, and that’s
> important. Dconf-editor didn’t have for now the need to handle this kind of
> themes specifically, so I didn’t do the job to have a .high-contrast class
> added (or a .adwaita one removed?) when it’s used. Correcting that might be
> a story for another patch, but the job has to be done before stable release.

I don't know what the issue with round corners + high contrast is. Can you point me to some direction?
 
> I’d prefer to remove the rounded corner, yes. That’s not a story of place
> with this patch, more of cluttering: there is a “visual noise” in having a
> small white space between a grey row and a blue selected item, and people
> that like small rows are usually the same who complain about visual noise. :)

Done.
 
> One thing I’d like you to look at is the transition between with and without
> small-keys-list-rows. You should go in the Gtk+ inspector, tab “Visual”, and
> slow down animations (up to 8 times); and then play with the option (using
> `gsettings`) in various places. Notably, there’s a margin jump on the left
> for keys, where the CSS handles that correctly for folders. You might want
> either to unify the width of this margin between the two states (might be an
> idea, now that there’s a margin for keys), or to make the transition smooth
> as it is for folders.

I went for smooth transition. Seems to work.

(In reply to Arnaud B. from comment #29)
> Oh, and you should set_header_func() at construct, only one time.

Fixed.

Created attachment 363830 [details] [review]
Reorganize key list rows for more uniform sizing

- Fixed padding transitions on key rows
- Fixed setting header function multiple times
- Removed round corder/margins for small-keys-list-rows

Hey, there’re less and less things to correct, nice. :)

(In reply to Davi from comment #30)
> I don't know what the issue with round corners + high contrast is. Can you
> point me to some direction?

In fact, I only had a small look at it, and the problem is not rounded corners, but the top/bottom padding of rows: the outline is so large that it touches text. That’s for another patch I think, it’s clearly HighContrast-specific. For rounded corners, this might make the outline better both in Adwaita and HighContrast:

.large-window                      .keys-list > row {
  -gtk-outline-radius:calc(0.5em - 1px);
}
.large-window.small-keys-list-rows .keys-list > row {
  -gtk-outline-radius:2px; /* gtk+ defaults */
}

> > I’d prefer to remove the rounded corner, yes.
> 
> Done.
> 
> > One thing I’d like you to look at is the transition between with and without
> > small-keys-list-rows.
> 
> I went for smooth transition. Seems to work.

It’s really great like that. :)

Other things:
 * you use in key-list-box-row.ui a GtkGrid inside a GtkGrid, I think you can use here the GtkGrid child properties[1] instead, can’t you?
 * for the “natural_width = 1000” (two times), please put a global const somewhere;
 * instead of a “row.show ();”, please use a “<property name="visible">True</property>” inside the UI file.

And that’s all! (until I discover something to nitpick, of course!)

[1] https://developer.gnome.org/gtk3/stable/GtkGrid.html#GtkGrid.child-properties

(In reply to Arnaud B. from comment #32)
> In fact, I only had a small look at it, and the problem is not rounded
> corners, but the top/bottom padding of rows: the outline is so large that it
> touches text. That’s for another patch I think, it’s clearly
> HighContrast-specific. 

Ok. Maybe increase padding might be a solution, since I noticed that most default widgets get more padding under HighContrast. But it might come in a later patch, as you said.

> For rounded corners, this might make the outline
> better both in Adwaita and HighContrast:
> 
> .large-window                      .keys-list > row {
>   -gtk-outline-radius:calc(0.5em - 1px);
> }
> .large-window.small-keys-list-rows .keys-list > row {
>   -gtk-outline-radius:2px; /* gtk+ defaults */
> }

Done.

> Other things:
>  * you use in key-list-box-row.ui a GtkGrid inside a GtkGrid, I think you
> can use here the GtkGrid child properties[1] instead, can’t you?

Done.

>  * for the “natural_width = 1000” (two times), please put a global const
> somewhere;

Done.

>  * instead of a “row.show ();”, please use a “<property
> name="visible">True</property>” inside the UI file.

Done.
 
> And that’s all! (until I discover something to nitpick, of course!)

I'm not complaining :).

Created attachment 363854 [details] [review]
Reorganize key list rows for more uniform sizing

- Add outline radius in large windows but not for small-keys-list-rows
- Single grid in key row
- Constant MAX_ROW_WIDTH
- Move visibility property to UI files

Created attachment 363878 [details] [review]
Reorganize key list rows for more uniform sizing

Rebase.

Created attachment 363879 [details]
Screenshot in LANG=he_IL.

Hey, something to nitpick! (:D) The alignment is not as it should be in RTL locales. And please, try to fusion CSS rules when they are related to the same thing (like the two “.large-window .keys-list > row” rules related to rounded corners).

Created attachment 363880 [details] [review]
Reorganize key list rows for more uniform sizing

- Fixed padding for RTL languages
- Small CSS code style improvement

Comment on attachment 363880 [details] [review]
Reorganize key list rows for more uniform sizing

Looks good, it’s committed. Many thanks for your hard work!

That’s a big thing done, great. :) There are some topics opened but not resolved, so I added two bugs:
 * https://bugzilla.gnome.org/show_bug.cgi?id=790476 for HighContrast things;
 * https://bugzilla.gnome.org/show_bug.cgi?id=790477 for more icons use.

Do you plan to work on some more bugs/changes in dconf-editor? I am currently working on a patchset for a new search function (well, about *all the cleaning before* a new search function, the last patches remain to be done ^^), that’s why I ask (let’s not work on the same thing/same part of the code, if you’re interested). There are various things to do, including in gtk+ to make GtkListBoxRow GtkActionable, working on the UI of the “let’s clean your dconf database” next step, etc., so help is always welcome. :)

Thanks for your reviews/guidance too!

Actually, I've been doing some experiments, so I have a few branches with half-done things. Search is one of them. I don't mind if I'm redoing what you did already, or if I picked the "wrong" direction, really. I guess you could tell what of these you think is of interest, if any, so I would gladly prioritize it:

- A ranked search results view (probably clashes with what you are doing, so
- An easily accessible list of "delayed changes" in delayed mode. Clicking on one entry of the list shows the corresponding key properties. Also, some mostly subjective improvements to the action bar layout.
- Navigation stack (back/forward, not up/down like there is already). This would be more interesting if more "jumping around" actions existed, like the above. Does not work for the current search mode, so it could come after that.
- A small iteration of the design of the properties view: wider rows (same as the browse view) and a larger text view for editing values (although this has focus issues because of the list box).

(In reply to Davi from comment #40)
> - A ranked search results view (probably clashes with what you are doing, so

In fact, I’d be surprised that whatever you’re doing it doesn’t clash somewhere, as my patchset touches many things. But if your search is more or less self-contained, it might help a little. Let’s move to https://bugzilla.gnome.org/show_bug.cgi?id=768275 to all that is search related, including my preparation patchset.

> - An easily accessible list of "delayed changes" in delayed mode. Clicking
> on one entry of the list shows the corresponding key properties. Also, some
> mostly subjective improvements to the action bar layout.

I’d be happy to improve the “action bar”, but I’m convinced it’s not the perfect UI anyway. I had ideas about making something in the top right of the window, that fusion with the “close window” button when there’re changes pending, but haven’t done anything in that direction for now. But if you “just” have some improvements, it would be great also. :)

> - Navigation stack (back/forward, not up/down like there is already). This
> would be more interesting if more "jumping around" actions existed, like the
> above. Does not work for the current search mode, so it could come after
> that.

?

> - A small iteration of the design of the properties view: wider rows (same
> as the browse view) and a larger text view for editing values (although this
> has focus issues because of the list box).

The properties view is clearly not perfect (see https://bugzilla.gnome.org/show_bug.cgi?id=777627 for example), but I don’t feel a real need to update it urgently. But if you have ideas, I’ll have a look, of course.

(In reply to Arnaud B. from comment #41)
> I’d be happy to improve the “action bar”, but I’m convinced it’s not the
> perfect UI anyway. I had ideas about making something in the top right of
> the window, that fusion with the “close window” button when there’re changes
> pending, but haven’t done anything in that direction for now. But if you
> “just” have some improvements, it would be great also. :)

https://bugzilla.gnome.org/show_bug.cgi?id=790524
 
> ?

https://bugzilla.gnome.org/show_bug.cgi?id=790527
 
> The properties view is clearly not perfect (see
> https://bugzilla.gnome.org/show_bug.cgi?id=777627 for example), but I don’t
> feel a real need to update it urgently. But if you have ideas, I’ll have a
> look, of course.

https://bugzilla.gnome.org/show_bug.cgi?id=790528