After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 766922 - Implement the redesigned shell
Implement the redesigned shell
Status: RESOLVED FIXED
Product: gnome-control-center
Classification: Core
Component: shell
unspecified
Other All
: Normal normal
: ---
Assigned To: Control-Center Maintainers
Control-Center Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-05-26 17:17 UTC by Georges Basile Stavracas Neto
Modified: 2016-06-03 15:37 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
window: add a horizontal box before the stack (1.48 KB, patch)
2016-05-26 17:17 UTC, Georges Basile Stavracas Neto
none Details | Review
window: turn into a final class (43.59 KB, patch)
2016-05-26 17:19 UTC, Georges Basile Stavracas Neto
none Details | Review
window: add a horizontal box before the stack (1.48 KB, patch)
2016-05-26 17:20 UTC, Georges Basile Stavracas Neto
none Details | Review
window: separate the panel selector (1.63 KB, patch)
2016-05-26 17:20 UTC, Georges Basile Stavracas Neto
none Details | Review
window: make the panels' stack expand (1.34 KB, patch)
2016-05-26 17:20 UTC, Georges Basile Stavracas Neto
none Details | Review
window: remove the fixed width request code (3.89 KB, patch)
2016-05-26 17:20 UTC, Georges Basile Stavracas Neto
none Details | Review
window: remove the small screen hack (5.81 KB, patch)
2016-05-26 17:20 UTC, Georges Basile Stavracas Neto
none Details | Review
window: make it a template class (22.86 KB, patch)
2016-05-26 17:20 UTC, Georges Basile Stavracas Neto
none Details | Review
window: keep searchbar visible when panel changes (1.16 KB, patch)
2016-05-26 17:20 UTC, Georges Basile Stavracas Neto
none Details | Review
window: remove the previous button (5.09 KB, patch)
2016-05-26 17:20 UTC, Georges Basile Stavracas Neto
none Details | Review
window: keep search button always visible (1.02 KB, patch)
2016-05-26 17:21 UTC, Georges Basile Stavracas Neto
none Details | Review
window: put the search button in the start of the headerbar (1.09 KB, patch)
2016-05-26 17:21 UTC, Georges Basile Stavracas Neto
none Details | Review
window: pack the headerbar inside a box (5.52 KB, patch)
2016-05-26 17:21 UTC, Georges Basile Stavracas Neto
none Details | Review
window: introduce the second headerbar (4.01 KB, patch)
2016-05-26 17:21 UTC, Georges Basile Stavracas Neto
none Details | Review
window: make the sidelist and first headerbar horizontally equal (2.28 KB, patch)
2016-05-26 17:21 UTC, Georges Basile Stavracas Neto
none Details | Review
window: set the default width and height (1.13 KB, patch)
2016-05-26 17:21 UTC, Georges Basile Stavracas Neto
none Details | Review
window: add a nice placeholder view (3.40 KB, patch)
2016-05-26 17:21 UTC, Georges Basile Stavracas Neto
none Details | Review
window: use a listbox (19.84 KB, patch)
2016-05-26 17:21 UTC, Georges Basile Stavracas Neto
none Details | Review
window: reimplement search (16.00 KB, patch)
2016-05-26 17:21 UTC, Georges Basile Stavracas Neto
none Details | Review
build: create another executable for the new layout (92.14 KB, patch)
2016-05-26 17:22 UTC, Georges Basile Stavracas Neto
none Details | Review
build: create another executable for the new layout (55.17 KB, patch)
2016-05-27 00:06 UTC, Georges Basile Stavracas Neto
none Details | Review
window: turn into a final class (43.62 KB, patch)
2016-05-27 00:06 UTC, Georges Basile Stavracas Neto
none Details | Review
window: add a horizontal box before the stack (1.50 KB, patch)
2016-05-27 00:06 UTC, Georges Basile Stavracas Neto
none Details | Review
window: separate the panel selector (1.65 KB, patch)
2016-05-27 00:06 UTC, Georges Basile Stavracas Neto
none Details | Review
window: make the panels' stack expand (1.36 KB, patch)
2016-05-27 00:06 UTC, Georges Basile Stavracas Neto
none Details | Review
window: remove the fixed width request code (3.91 KB, patch)
2016-05-27 00:06 UTC, Georges Basile Stavracas Neto
none Details | Review
window: remove the small screen hack (5.82 KB, patch)
2016-05-27 00:06 UTC, Georges Basile Stavracas Neto
needs-work Details | Review
window: make it a template class (22.87 KB, patch)
2016-05-27 00:07 UTC, Georges Basile Stavracas Neto
none Details | Review
window: keep searchbar visible when panel changes (1.18 KB, patch)
2016-05-27 00:07 UTC, Georges Basile Stavracas Neto
rejected Details | Review
window: remove the previous button (5.11 KB, patch)
2016-05-27 00:07 UTC, Georges Basile Stavracas Neto
rejected Details | Review
window: keep search button always visible (1.04 KB, patch)
2016-05-27 00:07 UTC, Georges Basile Stavracas Neto
reviewed Details | Review
window: put the search button in the start of the headerbar (1.09 KB, patch)
2016-05-27 00:07 UTC, Georges Basile Stavracas Neto
none Details | Review
window: pack the headerbar inside a box (5.54 KB, patch)
2016-05-27 00:07 UTC, Georges Basile Stavracas Neto
none Details | Review
window: introduce the second headerbar (4.03 KB, patch)
2016-05-27 00:07 UTC, Georges Basile Stavracas Neto
none Details | Review
window: make the sidelist and first headerbar horizontally equal (2.30 KB, patch)
2016-05-27 00:07 UTC, Georges Basile Stavracas Neto
none Details | Review
window: set the default width and height (1.13 KB, patch)
2016-05-27 00:08 UTC, Georges Basile Stavracas Neto
none Details | Review
window: add a nice placeholder view (3.40 KB, patch)
2016-05-27 00:08 UTC, Georges Basile Stavracas Neto
none Details | Review
window: use a listbox (19.86 KB, patch)
2016-05-27 00:08 UTC, Georges Basile Stavracas Neto
none Details | Review
window: reimplement search (16.02 KB, patch)
2016-05-27 00:08 UTC, Georges Basile Stavracas Neto
none Details | Review
window: turn into a final class (43.59 KB, patch)
2016-05-30 15:07 UTC, Georges Basile Stavracas Neto
committed Details | Review
build: copy CcWindow in alt/ directory (50.69 KB, patch)
2016-05-30 15:09 UTC, Georges Basile Stavracas Neto
committed Details | Review
build: create another executable for the new layout (3.95 KB, patch)
2016-05-30 15:09 UTC, Georges Basile Stavracas Neto
committed Details | Review
window: add a horizontal box before the stack (1.50 KB, patch)
2016-05-30 15:10 UTC, Georges Basile Stavracas Neto
committed Details | Review
window: separate the panel selector (1.65 KB, patch)
2016-05-30 15:10 UTC, Georges Basile Stavracas Neto
committed Details | Review
window: make the panels' stack expand (1.36 KB, patch)
2016-05-30 15:10 UTC, Georges Basile Stavracas Neto
committed Details | Review
window: remove the fixed width request code (3.90 KB, patch)
2016-05-30 15:11 UTC, Georges Basile Stavracas Neto
committed Details | Review
window: remove the small screen code (5.97 KB, patch)
2016-05-30 15:12 UTC, Georges Basile Stavracas Neto
committed Details | Review
window: make it a template class (22.87 KB, patch)
2016-05-30 15:13 UTC, Georges Basile Stavracas Neto
committed Details | Review
window: put the search button in the start of the headerbar (1.00 KB, patch)
2016-05-30 15:15 UTC, Georges Basile Stavracas Neto
committed Details | Review
window: pack the headerbar inside a box (7.21 KB, patch)
2016-05-30 15:16 UTC, Georges Basile Stavracas Neto
committed Details | Review
window: introduce the second headerbar (4.19 KB, patch)
2016-05-30 15:16 UTC, Georges Basile Stavracas Neto
committed Details | Review
window: make the sidelist and first headerbar horizontally equal (2.30 KB, patch)
2016-05-30 15:17 UTC, Georges Basile Stavracas Neto
committed Details | Review
window: set the default width and height (1.13 KB, patch)
2016-05-30 15:18 UTC, Georges Basile Stavracas Neto
none Details | Review
window: add a nice placeholder view (3.40 KB, patch)
2016-05-30 15:18 UTC, Georges Basile Stavracas Neto
accepted-commit_now Details | Review
window: use a listbox (19.10 KB, patch)
2016-05-30 15:19 UTC, Georges Basile Stavracas Neto
none Details | Review
window: reimplement search (16.07 KB, patch)
2016-05-30 15:19 UTC, Georges Basile Stavracas Neto
none Details | Review
window: set the default width and height (1.12 KB, patch)
2016-06-03 15:08 UTC, Georges Basile Stavracas Neto
none Details | Review
window: set the default width and height (1.12 KB, patch)
2016-06-03 15:09 UTC, Georges Basile Stavracas Neto
committed Details | Review
window: use a listbox (18.93 KB, patch)
2016-06-03 15:10 UTC, Georges Basile Stavracas Neto
none Details | Review
window: reimplement search (16.65 KB, patch)
2016-06-03 15:13 UTC, Georges Basile Stavracas Neto
committed Details | Review
window: use a listbox (18.98 KB, patch)
2016-06-03 15:27 UTC, Georges Basile Stavracas Neto
committed Details | Review

Description Georges Basile Stavracas Neto 2016-05-26 17:17:17 UTC
We have new mockups lying around for quite some time now. The patches that will be attached to this bug report are a tentative to 
move things forward.

Basically, it changes the window from a icon grid to a side list. In this patchset, I added a new non-installable binary 
(gnome-control-center-alt) to test the panels both with the current and the next layouts in parallel.
Comment 1 Georges Basile Stavracas Neto 2016-05-26 17:17:22 UTC
Created attachment 328568 [details] [review]
window: add a horizontal box before the stack

The horizontal box will be used in the future commits
to separate the sidelist from the stack.
Comment 2 Georges Basile Stavracas Neto 2016-05-26 17:19:58 UTC
Created attachment 328571 [details] [review]
window: turn into a final class

This commit updates the code to use the recently
introduced API. The new functions improve the
legibility and maintainability of the code, and
makes it easier to work on new features.
Comment 3 Georges Basile Stavracas Neto 2016-05-26 17:20:08 UTC
Created attachment 328572 [details] [review]
window: add a horizontal box before the stack

The horizontal box will be used in the future commits
to separate the sidelist from the stack.
Comment 4 Georges Basile Stavracas Neto 2016-05-26 17:20:13 UTC
Created attachment 328573 [details] [review]
window: separate the panel selector

To progressively achieve the sidelist, let's start by moving
the current iconview selector to the side and then turning
it into a GtkListBox.

This commit, then, moves the iconviews' list to the start of
the horizontal box added in the previous commit.
Comment 5 Georges Basile Stavracas Neto 2016-05-26 17:20:19 UTC
Created attachment 328574 [details] [review]
window: make the panels' stack expand

This way, panels can fill the most of the screen, which
is the desired behavior proposed by the mockups.
Comment 6 Georges Basile Stavracas Neto 2016-05-26 17:20:25 UTC
Created attachment 328575 [details] [review]
window: remove the fixed width request code

Having a fixed width is bad for a various reasons. First,
it's harder to deal with low resolution screens, which was
fixed by adding an entire new mode just to deal with it.
Second, because it may not scale well for big resolutions
as well.

Fix that by removing the code that handles the fixed width.
The next commit will completely remove the code that manages
small screens, in the hope that the new layout will handle
both cases well by design.
Comment 7 Georges Basile Stavracas Neto 2016-05-26 17:20:31 UTC
Created attachment 328576 [details] [review]
window: remove the small screen hack

Because the old layout didn't scale well on low
resolution displays, we had to introduce code
that adapts the window to be usable on low res
screens.

The problem with this code is that it still doesn't
scale down very well for really low resolution screens.
Partially because of the layout itself, partially
because the panels request a size bigger than e.g.
720x480.

Because I want to make the new sidelist layout work
on all kinds of displays, I'm removing the small screen
hack and, if needed, adapt the panels so that they
behave better.
Comment 8 Georges Basile Stavracas Neto 2016-05-26 17:20:37 UTC
Created attachment 328577 [details] [review]
window: make it a template class

In order to prepare ourselves for the future changes,
having the window as a template class is hugely advantageous
for we'll be able to modify the interface much more
quickly and cleanly.

This commit makes the window a template class, and only
that. No behavioral changes, nor new features were
introduced here.
Comment 9 Georges Basile Stavracas Neto 2016-05-26 17:20:49 UTC
Created attachment 328579 [details] [review]
window: keep searchbar visible when panel changes

The sidebar width is determined by the search entry,
and now it changes whenever we open a panel. This
would also break the desired behavior of keeping
the search visible - something impossible before.

Fix that by not hiding the search bar when the visible
panel changes.
Comment 10 Georges Basile Stavracas Neto 2016-05-26 17:20:55 UTC
Created attachment 328580 [details] [review]
window: remove the previous button

The previous button is not present on the new sidelist
layout mockups, and doesn't really fit the current
behavior anymore.

This commit removes the previous button.
Comment 11 Georges Basile Stavracas Neto 2016-05-26 17:21:02 UTC
Created attachment 328581 [details] [review]
window: keep search button always visible

Since we're not changing the entire page when selecting
a panel anymore, the search can stay visible all the
time.

This commit, then, removes the code that hides the search
button when a panel is selected.
Comment 12 Georges Basile Stavracas Neto 2016-05-26 17:21:08 UTC
Created attachment 328582 [details] [review]
window: put the search button in the start of the headerbar

Since the search bar has been moved to above the side list,
it makes sense to have the search button on the start of the
headerbar, strengtening the relation between both widgets.

To make it happen, remove the code that sets the search button
at the end of the headerbar.
Comment 13 Georges Basile Stavracas Neto 2016-05-26 17:21:14 UTC
Created attachment 328583 [details] [review]
window: pack the headerbar inside a box

In the future commits, we'll use two headerbars to visually
separate the panel area and the sidelist.
Comment 14 Georges Basile Stavracas Neto 2016-05-26 17:21:21 UTC
Created attachment 328584 [details] [review]
window: introduce the second headerbar

This commit introduces the second headerbar, where the
panel title and the panel widgets are displayed. It also
adapts the code to use the second headerbar when needed.
Comment 15 Georges Basile Stavracas Neto 2016-05-26 17:21:27 UTC
Created attachment 328585 [details] [review]
window: make the sidelist and first headerbar horizontally equal

The first headerbar and the sidelist should stay synchronized,
and this commit does so by setting the width request of the
headerbar to the allocated width of the sidelist.
Comment 16 Georges Basile Stavracas Neto 2016-05-26 17:21:34 UTC
Created attachment 328586 [details] [review]
window: set the default width and height

Since the window can shrink down too much, it makes
sense for us to have a sane default width and height.

I arbitrarily choose 720x480 because that's the default
resolution for CRT TVs.
Comment 17 Georges Basile Stavracas Neto 2016-05-26 17:21:40 UTC
Created attachment 328587 [details] [review]
window: add a nice placeholder view
Comment 18 Georges Basile Stavracas Neto 2016-05-26 17:21:47 UTC
Created attachment 328588 [details] [review]
window: use a listbox

The latest mockups use a list instead of icon grid. This
commit, after all the work porting the current UI to use
a side list, replacing the weird-looking icon grid.
Comment 19 Georges Basile Stavracas Neto 2016-05-26 17:21:53 UTC
Created attachment 328589 [details] [review]
window: reimplement search

We previously had a dedicate view for handling search,
based on model filtering and a custom panel to display
that differently.

After moving to GtkListBox, search can be trivially
done by using a filtering function, and widgets can
be fine-tuned to display extra information.

This patch, then, reimplements the search using a filtering
function over the panels' list.
Comment 20 Georges Basile Stavracas Neto 2016-05-26 17:22:00 UTC
Created attachment 328590 [details] [review]
build: create another executable for the new layout

This way, we can test both layouts in parallel and, in
case something goes wrong, we always have to old layout
as a fallback.
Comment 21 Georges Basile Stavracas Neto 2016-05-27 00:06:03 UTC
Created attachment 328605 [details] [review]
build: create another executable for the new layout

This way, we can test both layouts in parallel and, in
case something goes wrong, we always have to old layout
as a fallback.
Comment 22 Georges Basile Stavracas Neto 2016-05-27 00:06:10 UTC
Created attachment 328606 [details] [review]
window: turn into a final class

This commit updates the code to use the recently
introduced API. The new functions improve the
legibility and maintainability of the code, and
makes it easier to work on new features.
Comment 23 Georges Basile Stavracas Neto 2016-05-27 00:06:17 UTC
Created attachment 328607 [details] [review]
window: add a horizontal box before the stack

The horizontal box will be used in the future commits
to separate the sidelist from the stack.
Comment 24 Georges Basile Stavracas Neto 2016-05-27 00:06:25 UTC
Created attachment 328608 [details] [review]
window: separate the panel selector

To progressively achieve the sidelist, let's start by moving
the current iconview selector to the side and then turning
it into a GtkListBox.

This commit, then, moves the iconviews' list to the start of
the horizontal box added in the previous commit.
Comment 25 Georges Basile Stavracas Neto 2016-05-27 00:06:32 UTC
Created attachment 328609 [details] [review]
window: make the panels' stack expand

This way, panels can fill the most of the screen, which
is the desired behavior proposed by the mockups.
Comment 26 Georges Basile Stavracas Neto 2016-05-27 00:06:42 UTC
Created attachment 328610 [details] [review]
window: remove the fixed width request code

Having a fixed width is bad for a various reasons. First,
it's harder to deal with low resolution screens, which was
fixed by adding an entire new mode just to deal with it.
Second, because it may not scale well for big resolutions
as well.

Fix that by removing the code that handles the fixed width.
The next commit will completely remove the code that manages
small screens, in the hope that the new layout will handle
both cases well by design.
Comment 27 Georges Basile Stavracas Neto 2016-05-27 00:06:53 UTC
Created attachment 328611 [details] [review]
window: remove the small screen hack

Because the old layout didn't scale well on low
resolution displays, we had to introduce code
that adapts the window to be usable on low res
screens.

The problem with this code is that it still doesn't
scale down very well for really low resolution screens.
Partially because of the layout itself, partially
because the panels request a size bigger than e.g.
720x480.

Because I want to make the new sidelist layout work
on all kinds of displays, I'm removing the small screen
hack and, if needed, adapt the panels so that they
behave better.
Comment 28 Georges Basile Stavracas Neto 2016-05-27 00:07:01 UTC
Created attachment 328612 [details] [review]
window: make it a template class

In order to prepare ourselves for the future changes,
having the window as a template class is hugely advantageous
for we'll be able to modify the interface much more
quickly and cleanly.

This commit makes the window a template class, and only
that. No behavioral changes, nor new features were
introduced here.
Comment 29 Georges Basile Stavracas Neto 2016-05-27 00:07:07 UTC
Created attachment 328613 [details] [review]
window: keep searchbar visible when panel changes

The sidebar width is determined by the search entry,
and now it changes whenever we open a panel. This
would also break the desired behavior of keeping
the search visible - something impossible before.

Fix that by not hiding the search bar when the visible
panel changes.
Comment 30 Georges Basile Stavracas Neto 2016-05-27 00:07:15 UTC
Created attachment 328614 [details] [review]
window: remove the previous button

The previous button is not present on the new sidelist
layout mockups, and doesn't really fit the current
behavior anymore.

This commit removes the previous button.
Comment 31 Georges Basile Stavracas Neto 2016-05-27 00:07:23 UTC
Created attachment 328615 [details] [review]
window: keep search button always visible

Since we're not changing the entire page when selecting
a panel anymore, the search can stay visible all the
time.

This commit, then, removes the code that hides the search
button when a panel is selected.
Comment 32 Georges Basile Stavracas Neto 2016-05-27 00:07:31 UTC
Created attachment 328616 [details] [review]
window: put the search button in the start of the headerbar

Since the search bar has been moved to above the side list,
it makes sense to have the search button on the start of the
headerbar, strengtening the relation between both widgets.

To make it happen, remove the code that sets the search button
at the end of the headerbar.
Comment 33 Georges Basile Stavracas Neto 2016-05-27 00:07:38 UTC
Created attachment 328617 [details] [review]
window: pack the headerbar inside a box

In the future commits, we'll use two headerbars to visually
separate the panel area and the sidelist.
Comment 34 Georges Basile Stavracas Neto 2016-05-27 00:07:48 UTC
Created attachment 328618 [details] [review]
window: introduce the second headerbar

This commit introduces the second headerbar, where the
panel title and the panel widgets are displayed. It also
adapts the code to use the second headerbar when needed.
Comment 35 Georges Basile Stavracas Neto 2016-05-27 00:07:54 UTC
Created attachment 328619 [details] [review]
window: make the sidelist and first headerbar horizontally equal

The first headerbar and the sidelist should stay synchronized,
and this commit does so by setting the width request of the
headerbar to the allocated width of the sidelist.
Comment 36 Georges Basile Stavracas Neto 2016-05-27 00:08:03 UTC
Created attachment 328620 [details] [review]
window: set the default width and height

Since the window can shrink down too much, it makes
sense for us to have a sane default width and height.

I arbitrarily choose 720x480 because that's the default
resolution for CRT TVs.
Comment 37 Georges Basile Stavracas Neto 2016-05-27 00:08:09 UTC
Created attachment 328621 [details] [review]
window: add a nice placeholder view
Comment 38 Georges Basile Stavracas Neto 2016-05-27 00:08:18 UTC
Created attachment 328622 [details] [review]
window: use a listbox

The latest mockups use a list instead of icon grid. This
commit, after all the work porting the current UI to use
a side list, replacing the weird-looking icon grid.
Comment 39 Georges Basile Stavracas Neto 2016-05-27 00:08:26 UTC
Created attachment 328623 [details] [review]
window: reimplement search

We previously had a dedicate view for handling search,
based on model filtering and a custom panel to display
that differently.

After moving to GtkListBox, search can be trivially
done by using a filtering function, and widgets can
be fine-tuned to display extra information.

This patch, then, reimplements the search using a filtering
function over the panels' list.
Comment 40 Bastien Nocera 2016-05-27 11:34:40 UTC
Review of attachment 328605 [details] [review]:

Copy the cc-window.[ch] files as a separate commit, and mention that they are exact copies.

::: shell/Makefile.am
@@ +21,3 @@
 bin_PROGRAMS = gnome-control-center
 
+noinst_bindir = $(top_srcdir)/shell/alt

There's no bindir for a noinst.

@@ +58,3 @@
+gnome_control_center_alt_SOURCES = 		\
+	$(common_sources)			\
+	alt/cc-window.c				\

No, add a Makefile.am in alt/ and compile a library there, which you can link against the alt binary.

@@ +106,3 @@
 
+gnome_control_center_LDADD = $(common_ldadd)
+gnome_control_center_alt_LDADD = $(common_ldadd)

how about gnome_control_center_alt_LDADD = $(gnome_control_center_LDADD) and not any of the changes above?
Comment 41 Bastien Nocera 2016-05-27 11:35:53 UTC
Review of attachment 328606 [details] [review]:

That looks fine, but I would introduce this *before* the patch introducing the alt binary.
Comment 42 Bastien Nocera 2016-05-27 11:36:22 UTC
Review of attachment 328607 [details] [review]:

Ok. Pending future changes.
Comment 43 Bastien Nocera 2016-05-27 11:37:00 UTC
Review of attachment 328608 [details] [review]:

Ok.
Comment 44 Bastien Nocera 2016-05-27 11:37:52 UTC
Review of attachment 328609 [details] [review]:

Sure.
Comment 45 Bastien Nocera 2016-05-27 11:39:38 UTC
Review of attachment 328610 [details] [review]:

> for a various reasons. 

for various reasons.

> Second, because it may not scale well for big resolutions
> as well.

"it may not scale as well for big resolutions"
Comment 46 Bastien Nocera 2016-05-27 11:43:49 UTC
Review of attachment 328611 [details] [review]:

> Partially because of the layout itself, partially
> because the panels request a size bigger than e.g.
> 720x480.

It was never made to work well with 720x480 because the minimum resolution we were targeting 720x480.

You should mention that we never targeted 720x480, but that we want to support it now. Rather than imply that it was bad code :P

> Because I want to make the new sidelist layout work
> on all kinds of displays, I'm removing the small screen
> hack and, if needed, adapt the panels so that they
> behave better.

Don't use first person here. Use declarative language.

The whole commit message needs more work.
Comment 47 Bastien Nocera 2016-05-27 11:46:43 UTC
Review of attachment 328612 [details] [review]:

That would have been nicer to do as one of the first commits, rather than in the middle of the changes. Explain the reason why that is in the commit message.
Comment 48 Bastien Nocera 2016-05-27 11:49:31 UTC
Review of attachment 328613 [details] [review]:

That's not how it's been designed:
https://github.com/gnome-design-team/gnome-mockups/blob/master/system-settings/shell/settings-search.png

The sidebar's width needs to be the same width as the longest label + padding.
Comment 49 Bastien Nocera 2016-05-27 11:51:05 UTC
Review of attachment 328614 [details] [review]:

It will be used when we go into the "devices" or "details" sub-category:
https://github.com/gnome-design-team/gnome-mockups/blob/master/system-settings/shell/settings-organization.png
Comment 50 Bastien Nocera 2016-05-27 11:51:44 UTC
Review of attachment 328615 [details] [review]:

That's fine for now.
Comment 51 Bastien Nocera 2016-05-27 11:54:23 UTC
Review of attachment 328616 [details] [review]:

Yep.

> side list

Make sure you use the same word (or words) in all the commits. It's been "sidelist", "side list", "sidebar".

> strengtening

Missing letters here.

> To make it happen, remove the code that sets the search button
> at the end of the headerbar.

This is not needed, we're pretty stupid but not that much ;)
Comment 52 Bastien Nocera 2016-05-27 11:55:19 UTC
Review of attachment 328617 [details] [review]:

Ok.
Comment 53 Bastien Nocera 2016-05-27 11:56:07 UTC
Review of attachment 328618 [details] [review]:

::: shell/alt/cc-window.c
@@ +54,3 @@
   GtkWidget  *stack;
   GtkWidget  *header;
+  GtkWidget  *header2;

Needs a better name. panel_headerbar?
Comment 54 Bastien Nocera 2016-05-27 11:56:46 UTC
Review of attachment 328619 [details] [review]:

Sure.
Comment 55 Bastien Nocera 2016-05-27 11:59:21 UTC
Review of attachment 328620 [details] [review]:

That's too big on a 720x480, which will include a system toolbar of some kind (so it will get maximised), and that's probably a bit small for larger screens.

If you went with 1024x768 as a "default" size, then it will maximise the window, and look big enough on larger screens. Feel free to test.
Comment 56 Bastien Nocera 2016-05-27 12:00:31 UTC
Review of attachment 328620 [details] [review]:

And before I forget, we might want the default size to be a 16:9 ratio, so that the panel itself is 4:3. You can discuss the exact size to use with #gnome-design.
Comment 57 Bastien Nocera 2016-05-27 12:01:19 UTC
Review of attachment 328621 [details] [review]:

Do we have mockups for that? I'd like to have the language checked.
Comment 58 Bastien Nocera 2016-05-27 12:05:33 UTC
Review of attachment 328622 [details] [review]:

::: shell/alt/cc-window.c
@@ +622,3 @@
+  gtk_tree_model_get_iter_first (model, &iter);
+
+  while (gtk_tree_model_iter_next (model, &iter))

Pretty sure you're skipping the first iter. I prefer:

cont = gtk_tree_model_get_iter_first (model, &iter);
while (cont)
{
  cont = _iter_next();
}

@@ +1129,3 @@
+                                           g_str_equal,
+                                           g_free,
+                                           (GDestroyNotify) row_data_free);

I'd rather you made the RowData belong to the ListBox widget, with g_object_set_data_full()
Comment 59 Bastien Nocera 2016-05-27 12:09:31 UTC
Review of attachment 328623 [details] [review]:

> panels' list.

A list of panels that belong to multiple ones?

"panels list" is fine.

This will need work. You removed the whole filtering code that was there before without reimplementing it, that's no good, and your code assumes that the search is always visible, which isn't something we want.
Comment 60 Georges Basile Stavracas Neto 2016-05-30 15:07:56 UTC
Created attachment 328732 [details] [review]
window: turn into a final class

Done.
Comment 61 Georges Basile Stavracas Neto 2016-05-30 15:09:09 UTC
Created attachment 328733 [details] [review]
build: copy CcWindow in alt/ directory

The copied files are exact copies of shell/cc-window.c and
shell/cc-window.h, and they'll be used to implement the restyled
shell as an alternative binary.
Comment 62 Bastien Nocera 2016-05-30 15:09:16 UTC
Review of attachment 328732 [details] [review]:

Sure.
Comment 63 Georges Basile Stavracas Neto 2016-05-30 15:09:33 UTC
Created attachment 328735 [details] [review]
build: create another executable for the new layout

Done. Split the patch in 2, and this is the second one.
Comment 64 Georges Basile Stavracas Neto 2016-05-30 15:10:15 UTC
Created attachment 328736 [details] [review]
window: add a horizontal box before the stack
Comment 65 Georges Basile Stavracas Neto 2016-05-30 15:10:36 UTC
Created attachment 328737 [details] [review]
window: separate the panel selector

To progressively achieve the sidelist, let's start by moving
the current iconview selector to the side and then turning
it into a GtkListBox.

This commit, then, moves the iconviews' list to the start of
the horizontal box added in the previous commit.
Comment 66 Georges Basile Stavracas Neto 2016-05-30 15:10:51 UTC
Created attachment 328738 [details] [review]
window: make the panels' stack expand
Comment 67 Georges Basile Stavracas Neto 2016-05-30 15:11:29 UTC
Created attachment 328739 [details] [review]
window: remove the fixed width request code

Fixed the typos.
Comment 68 Georges Basile Stavracas Neto 2016-05-30 15:12:00 UTC
Created attachment 328740 [details] [review]
window: remove the small screen code

Updated the commit message.
Comment 69 Georges Basile Stavracas Neto 2016-05-30 15:13:30 UTC
Created attachment 328741 [details] [review]
window: make it a template class

I didn't made it before because I didn't even think about it. Are you OK in accepting this patch only for the sidelist CcWindow?
Comment 70 Georges Basile Stavracas Neto 2016-05-30 15:15:32 UTC
Created attachment 328742 [details] [review]
window: put the search button in the start of the headerbar

Updated the commit message.
Comment 71 Georges Basile Stavracas Neto 2016-05-30 15:16:10 UTC
Created attachment 328743 [details] [review]
window: pack the headerbar inside a box
Comment 72 Georges Basile Stavracas Neto 2016-05-30 15:16:51 UTC
Created attachment 328744 [details] [review]
window: introduce the second headerbar

(In reply to Bastien Nocera from comment #53)
> Review of attachment 328618 [details] [review] [review]:
> 
> ::: shell/alt/cc-window.c
> @@ +54,3 @@
>    GtkWidget  *stack;
>    GtkWidget  *header;
> +  GtkWidget  *header2;
> 
> Needs a better name. panel_headerbar?

Done.
Comment 73 Georges Basile Stavracas Neto 2016-05-30 15:17:16 UTC
Created attachment 328745 [details] [review]
window: make the sidelist and first headerbar horizontally equal
Comment 74 Georges Basile Stavracas Neto 2016-05-30 15:18:06 UTC
Created attachment 328746 [details] [review]
window: set the default width and height

I still have to discuss this one.
Comment 75 Bastien Nocera 2016-05-30 15:18:14 UTC
Review of attachment 328733 [details] [review]:

Sure.
Comment 76 Georges Basile Stavracas Neto 2016-05-30 15:18:27 UTC
Created attachment 328747 [details] [review]
window: add a nice placeholder view

Also have to discuss this one.
Comment 77 Georges Basile Stavracas Neto 2016-05-30 15:19:06 UTC
Created attachment 328748 [details] [review]
window: use a listbox

Removed the hash table, now it uses g_object_set_data_full().

Also fixed the iterator issue. Thanks for pointing that out.
Comment 78 Georges Basile Stavracas Neto 2016-05-30 15:19:35 UTC
Created attachment 328750 [details] [review]
window: reimplement search

Is the filtering code relevant still?
Comment 79 Bastien Nocera 2016-05-30 15:19:59 UTC
Review of attachment 328735 [details] [review]:

Looks good apart from that.

::: configure.ac
@@ +7,3 @@
 AC_CONFIG_MACRO_DIR([m4])
 
+AM_INIT_AUTOMAKE([1.11.2 no-dist-gzip dist-xz tar-ustar check-news subdir-objects])

Pretty sure you don't need subdir-objects with a Makefile.am in the alt/ directory.
Comment 80 Bastien Nocera 2016-05-30 15:20:51 UTC
Review of attachment 328736 [details] [review]:

Sure.
Comment 81 Bastien Nocera 2016-05-30 15:21:20 UTC
Review of attachment 328737 [details] [review]:

Yep.
Comment 82 Bastien Nocera 2016-05-30 15:22:00 UTC
Review of attachment 328738 [details] [review]:

Yep.
Comment 83 Bastien Nocera 2016-05-30 15:22:51 UTC
Review of attachment 328739 [details] [review]:

Sure.
Comment 84 Bastien Nocera 2016-05-30 15:23:14 UTC
Review of attachment 328740 [details] [review]:

Sure.
Comment 85 Bastien Nocera 2016-05-30 15:25:11 UTC
Review of attachment 328741 [details] [review]:

Looks good.

(as you said, you could have done that with the old code, but it's really not worth it right now, as we're unlikely to change the UI of the old shell...)
Comment 86 Bastien Nocera 2016-05-30 15:25:35 UTC
Review of attachment 328742 [details] [review]:

Yep.
Comment 87 Bastien Nocera 2016-05-30 15:27:42 UTC
Review of attachment 328743 [details] [review]:

Yep.
Comment 88 Bastien Nocera 2016-05-30 15:28:26 UTC
Review of attachment 328744 [details] [review]:

Looks good.
Comment 89 Bastien Nocera 2016-05-30 15:28:54 UTC
Review of attachment 328745 [details] [review]:

Sure.
Comment 90 Bastien Nocera 2016-05-30 15:30:05 UTC
Review of attachment 328746 [details] [review]:

> I arbitrarily choose 720x480 because that's the default
> resolution for CRT TVs.

Better without "I":
720x480 was chosen because ...

Looks good otherwise.
Comment 91 Bastien Nocera 2016-05-30 15:31:17 UTC
Review of attachment 328747 [details] [review]:

> window: add a nice placeholder view

You can remove the "nice"
Comment 92 Bastien Nocera 2016-05-30 15:50:42 UTC
Review of attachment 328748 [details] [review]:

> replacing the weird-looking icon grid.

replacing the temporary icon grid.

We can make more changes later on.

::: shell/alt/cc-window.c
@@ +169,3 @@
+
+  /* Icon */
+  image = gtk_image_new_from_gicon (icon, GTK_ICON_SIZE_BUTTON);

No icons, thanks.
Comment 93 Bastien Nocera 2016-05-30 15:57:20 UTC
Review of attachment 328750 [details] [review]:

::: shell/alt/cc-window.c
@@ +357,2 @@
 static gboolean
+filter_func (GtkListBoxRow *row,

Your filter func doesn't casefold or unaccent, doesn't split by term.

Look at the contents of search_entry_changed_cb().

The code is already there, you just need not to remove it ("network" doesn't match "Network"...)
Comment 94 Georges Basile Stavracas Neto 2016-06-03 15:08:41 UTC
Created attachment 329050 [details] [review]
window: set the default width and height

Since the window can shrink down too much, it makes
sense for us to have a sane default width and height.

720x480 was chosen because that's the default resolution
for CRT TVs.
Comment 95 Georges Basile Stavracas Neto 2016-06-03 15:09:17 UTC
Created attachment 329051 [details] [review]
window: set the default width and height

Fixed the commit message
Comment 96 Georges Basile Stavracas Neto 2016-06-03 15:10:01 UTC
Created attachment 329052 [details] [review]
window: use a listbox

Removed the icons.
Comment 97 Georges Basile Stavracas Neto 2016-06-03 15:13:23 UTC
Created attachment 329053 [details] [review]
window: reimplement search

Added unaccent, casefolding and trimming support.
Comment 98 Bastien Nocera 2016-06-03 15:22:22 UTC
Review of attachment 329052 [details] [review]:

Looks fine otherwise.

::: shell/alt/cc-window.c
@@ +611,3 @@
+  shell->store = (GtkListStore *) cc_shell_model_new ();
+  model = GTK_TREE_MODEL (shell->store);
+  valid = TRUE;

Remove this.

@@ +616,3 @@
+
+  /* Create a row for each panel */
+  gtk_tree_model_get_iter_first (model, &iter);

valid = ...
Comment 99 Georges Basile Stavracas Neto 2016-06-03 15:27:20 UTC
Created attachment 329057 [details] [review]
window: use a listbox

Fixed the valid var.
Comment 100 Georges Basile Stavracas Neto 2016-06-03 15:35:55 UTC
Done. I'll open separate bug reports for individual issues and features.

Attachment 328732 [details] pushed as 71d39a4 - window: turn into a final class
Attachment 328733 [details] pushed as fe3a237 - build: copy CcWindow in alt/ directory
Attachment 328735 [details] pushed as 2bb6d44 - build: create another executable for the new layout
Attachment 328736 [details] pushed as c006198 - window: add a horizontal box before the stack
Attachment 328737 [details] pushed as c47bc08 - window: separate the panel selector
Attachment 328738 [details] pushed as 2f844e2 - window: make the panels' stack expand
Attachment 328739 [details] pushed as 77ef6c4 - window: remove the fixed width request code
Attachment 328740 [details] pushed as 6f38ca8 - window: remove the small screen code
Attachment 328741 [details] pushed as 55cc4c3 - window: make it a template class
Attachment 328742 [details] pushed as df086d4 - window: put the search button in the start of the headerbar
Attachment 328743 [details] pushed as fa1a6b3 - window: pack the headerbar inside a box
Attachment 328744 [details] pushed as c0bdc06 - window: introduce the second headerbar
Attachment 328745 [details] pushed as a267ebd - window: make the sidelist and first headerbar horizontally equal
Attachment 329051 [details] pushed as 2590e38 - window: set the default width and height
Attachment 329053 [details] pushed as 57ba49a - window: reimplement search
Attachment 329057 [details] pushed as b6ab2a4 - window: use a listbox