GNOME Bugzilla – Bug 762488
Open join-dialog with connections page when there are no existing connections
Last modified: 2016-02-24 21:24:43 UTC
As the title says - when we don't have any connections, the main page is useless and the user must create a connection first. Therefore it makes sense to simply start with the right page. The "back" button doesn't really make sense in that case, so we probably want to turn it into "Cancel". (It's also odd to not have a positive action like "Create" or "Connect", but that's not really limited to this case, so let's discuss that separately)
Created attachment 321913 [details] [review] joinDialog: Open connections page first in case no existing connections When we don't have any connections, the main page is useless and the user must create a connection first. Therefore it makes sense to simply start with the right page. Also in that case use cancel button instead of back button.
Review of attachment 321913 [details] [review]: Code looks good, some style suggestions below (and they really are suggestions, so feel free to ignore). I'll run this change by Allan later to get a design ack. ::: src/joinDialog.js @@ +69,3 @@ })); + + if (Object.keys(this._accounts).length > 0) Is it worth splitting this out as get _hasAccounts() { return Object.keys(this._accounts).length > 0; }? I think Object.keys(...).length is pretty clear already, but this._hasAccounts would be a bit more obvious still. @@ +70,3 @@ + + if (Object.keys(this._accounts).length > 0) + this._setPage(DialogPage.MAIN); Mmmh, getting the initial list of accounts and setting up the signals to update it are pretty tied together, so moving this code in between reads a bit odd. I think moving it towards the end (before updateCombo and updateCanJoin - the latter depends on the page!) looks better @@ +264,3 @@ this._customToggle.active = false; + this._backButton.visible = !isMain && !isAccountsEmpty; I'd prefer this as !(isMain || isAccountsEmpty) to make it obvious that back- and cancel button are mutually exclusive
Created attachment 322210 [details] [review] joinDialog: Open connections page first in case no existing connections When we don't have any connections, the main page is useless and the user must create a connection first. Therefore it makes sense to simply start with the right page. Also in that case use cancel button instead of back button.
Florian,took all your suggestions ;) Made sense to me. If you have ack from design team, I'll push the patch.
Review of attachment 322210 [details] [review]: LGTM (In reply to Kunaal Jain from comment #4) > If you have ack from design team, I'll push the patch. I don't - we ran into some xdg-app-on-continuous issues, hopefully we'll be able to sort those out later.
Created attachment 322284 [details] [review] joinDialog: Open connections page first in case no existing connections When we don't have any connections, the main page is useless and the user must create a connection first. Therefore it makes sense to simply start with the right page. Also in that case use cancel button instead of back button.
Attachment 322284 [details] pushed as 37e3493 - joinDialog: Open connections page first in case no existing connections