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 772458 - Revamp initial setup
Revamp initial setup
Status: RESOLVED FIXED
Product: polari
Classification: Applications
Component: general
unspecified
Other Linux
: Normal enhancement
: ---
Assigned To: Polari maintainers
Polari maintainers
Depends on:
Blocks:
 
 
Reported: 2016-10-05 15:45 UTC by Bastian Ilsø
Modified: 2017-08-11 23:12 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
initial setup mockup by allan (78.70 KB, image/png)
2016-10-05 15:45 UTC, Bastian Ilsø
  Details
connections: Add :favorites-only property (3.07 KB, patch)
2017-08-10 23:59 UTC, Florian Müllner
committed Details | Review
data: Make some more networks favorites (1.19 KB, patch)
2017-08-10 23:59 UTC, Florian Müllner
committed Details | Review
initialSetup: Add a dedicated setup window (12.75 KB, patch)
2017-08-10 23:59 UTC, Florian Müllner
none Details | Review
initialSetup: Add offline handling (5.77 KB, patch)
2017-08-10 23:59 UTC, Florian Müllner
committed Details | Review
app: Run initial setup on first run (4.14 KB, patch)
2017-08-10 23:59 UTC, Florian Müllner
committed Details | Review
app: Add a way to force running initial setup (1.10 KB, patch)
2017-08-11 00:00 UTC, Florian Müllner
committed Details | Review
initialSetup: Add a dedicated setup window (13.39 KB, patch)
2017-08-11 00:28 UTC, Florian Müllner
committed Details | Review

Description Bastian Ilsø 2016-10-05 15:45:48 UTC
Created attachment 336995 [details]
initial setup mockup by allan

Allan has made some nice mockups for a faster initial setup experience. It has some nice advantages such as being faster for new users and being convenient since you no longer start from nothing. Related to this is also a nice place to ask for something like importing settings from xchat and other clients (Bug 709189).
Comment 1 Florian Müllner 2017-08-10 23:59:23 UTC
Created attachment 357365 [details] [review]
connections: Add :favorites-only property

In order to provide a better onboarding experience, we will add a guided
initial setup mode based on the existing network/room configuration UI.
However unless in the join dialog, we don't want to display the full list
of predefined networks there, or allow the creation of custom ones. To
support this use case, add a new contruct-only property to limit the list
to favorite networks rather than just prioritizing them over others.
Comment 2 Florian Müllner 2017-08-10 23:59:31 UTC
Created attachment 357366 [details] [review]
data: Make some more networks favorites

The previously added :favorites-only property now allows to display
a network list that is limited to a selected number of networks.
Currently that number is two, which is clearly on the lower side,
so promote some more popular networks to favorites.
Comment 3 Florian Müllner 2017-08-10 23:59:39 UTC
Created attachment 357367 [details] [review]
initialSetup: Add a dedicated setup window

Our current initial setup experience is essentially a label telling
the user where to click. We can do much better by combining the
existing elements in a separate, guided setup window ...
Comment 4 Florian Müllner 2017-08-10 23:59:46 UTC
Created attachment 357368 [details] [review]
initialSetup: Add offline handling

The guided setup assumes that we can connect to a newly connected
account and fetch the room list, which means it is fairly useless
without an internet connection - there's little we can do about
that, except for telling the user ...
Comment 5 Florian Müllner 2017-08-10 23:59:55 UTC
Created attachment 357369 [details] [review]
app: Run initial setup on first run

It's time to hook up the shiny new initial setup experience - when
initial setup hasn't been run before and the user hasn't configured
any rooms (with a previous version), show the initial setup window
before the main application window.
Comment 6 Florian Müllner 2017-08-11 00:00:04 UTC
Created attachment 357370 [details] [review]
app: Add a way to force running initial setup

We only run initial setup once, and only for new users that haven't set
up any channels before. As recreating those conditions for testing is
quite tedious, allow to force running setup via an environment variable.
Comment 7 Piotr Drąg 2017-08-11 00:19:51 UTC
(In reply to Florian Müllner from comment #3)
> Created attachment 357367 [details] [review] [review]
> initialSetup: Add a dedicated setup window
> 


data/resources/initial-setup-window.ui has many strings that are not marked for translation.
Comment 8 Florian Müllner 2017-08-11 00:28:41 UTC
Created attachment 357372 [details] [review]
initialSetup: Add a dedicated setup window

(In reply to Piotr Drąg from comment #7)
> data/resources/initial-setup-window.ui has many strings that are not marked
> for translation.

Indeed, and of course none of the new files were added to POTFILES ... (/me hides). Thanks for the catch!
Comment 9 Bastian Ilsø 2017-08-11 13:36:13 UTC
Review of attachment 357365 [details] [review]:

this part reads funny to me:
"However unless in the join dialog, we don't want to display the full list
of predefined networks there, or allow the creation of custom ones."
maybe
"However, we don't want to display the full list of predefined networks in the join dialog, or allow the creation of custom ones." ?

::: src/connections.js
@@ +202,3 @@
 
+        if (!this._rows.has(account.service))
+            return;

is this a related change to adding the favoritesOnly property?
Comment 10 Bastian Ilsø 2017-08-11 14:21:00 UTC
Review of attachment 357366 [details] [review]:

lgtm
Comment 11 Florian Müllner 2017-08-11 17:15:10 UTC
(In reply to Bastian Ilsø from comment #9)
> this part reads funny to me:

It does indeed :-(

However

> maybe
> "we don't want to display the full list [...] in the join dialog

is wrong. Make this "*as* in the join dialog" :-)


> ::: src/connections.js
> @@ +202,3 @@
> 
> is this a related change to adding the favoritesOnly property?

Yes. The current code assumes that there's a row for every predefined account, but with this change we only create rows for favorite networks. (Another option would be to add all networks, but keep non-favorites hidden - we'd save the extra-check, but create a bunch of pointless widgets ...)
Comment 12 Bastian Ilsø 2017-08-11 21:17:07 UTC
Review of attachment 357372 [details] [review]:

looks good to me, just one question:

::: src/initialSetup.js
@@ +110,3 @@
+        let toJoinRooms = this._serverRoomList.selectedRooms;
+
+        let accountPath = this._currentAccount.get_object_path();

do we need to check for this._currentAccount == null here?
Comment 13 Bastian Ilsø 2017-08-11 21:17:08 UTC
Review of attachment 357372 [details] [review]:

looks good to me, just one question:

::: src/initialSetup.js
@@ +110,3 @@
+        let toJoinRooms = this._serverRoomList.selectedRooms;
+
+        let accountPath = this._currentAccount.get_object_path();

do we need to check for this._currentAccount == null here?
Comment 14 Florian Müllner 2017-08-11 21:32:52 UTC
(In reply to Bastian Ilsø from comment #13)
> +        let accountPath = this._currentAccount.get_object_path();
> 
> do we need to check for this._currentAccount == null here?

That would be a bug - the only way(*) to get to the room page is by selecting an account, so _currentAccount should always be set at that point.


(*) other than using the gtk inspector to switch pages manually, but meh
Comment 15 Bastian Ilsø 2017-08-11 22:28:01 UTC
Review of attachment 357368 [details] [review]:

looks good to me
Comment 16 Bastian Ilsø 2017-08-11 22:36:06 UTC
Review of attachment 357369 [details] [review]:

lgtm
Comment 17 Bastian Ilsø 2017-08-11 22:36:46 UTC
Review of attachment 357370 [details] [review]:

lgtm
Comment 18 Bastian Ilsø 2017-08-11 22:37:55 UTC
Review of attachment 357372 [details] [review]:

lgtm
Comment 19 Florian Müllner 2017-08-11 23:11:35 UTC
Attachment 357365 [details] pushed as 9f5671e - connections: Add :favorites-only property
Attachment 357366 [details] pushed as 55db868 - data: Make some more networks favorites
Attachment 357368 [details] pushed as 446a633 - initialSetup: Add offline handling
Attachment 357369 [details] pushed as c7fdaab - app: Run initial setup on first run
Attachment 357370 [details] pushed as 8f4b4a7 - app: Add a way to force running initial setup
Attachment 357372 [details] pushed as 5000689 - initialSetup: Add a dedicated setup window