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

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

Created attachment 313022 [details]
Wire of a revamped connections dialog.

just removing the last lines of irrelevant copypasta sorry for the noise.

Not sure bugzilla is the best place to discuss design, but here we go:

 - I think the sidebar/details split can work quite well
 - the visual refresh of the details part looks nice
 - "undo" would be a great addition
 - I'd like to keep the ability to disable accounts
   (removing and re-adding is not the same - you lose at least
    your saved channels, and possibly chat history as well);
   I do like the sidebar in the mockups though, so that'd need
   to go elsewhere (details? or it's a fringe feature that's
   covered by the online accounts panel?)
 - I don't think instant-apply is the right choice here - failed
   connection attempts while the server field is incomplete are
   one thing ("irc.gn"), sending user information that *we* have
   guessed (default nick, possible real name) without explicit
   consent a very different one
 - before adding a password field, I'd like to try to support
   server passwords dynamically first (i.e. in response to an
   AUTH_REQUIRED error); it's uncommon for servers to actually
   require a password, except that NickServ is usually set up
   to pick that up for identification - but that only works when
   you actually connect with your "normal" nick (not nick_ etc), 
   so we should add proper NickServ support instead of relying on 
   the server password (and again, I'd like to explore other options
   first before adding upfront configuration - it's a bit weird
   to set up a connection without password, use it to register
   your nick, then go back to the configuration to set up the
   password ...)
 - I'm divided on the network list - it looks nice in the mockup
   with ~10 entries, but that's just not realistic. We'll likely 
   get in the "but foo is in the list, so why not bar?" game and 
   end up with a much larger list full of crap (empathy: 76, 
   hexchat: 93, ...). Bug 711833 has a mockup that suggests using 
   a preconfigured list to do entry completion[0] - maybe do that 
   using the full list, and then display a very reduced list (five?); 
   also not compelled by the "transition between list and entries" 
   part: the options I see there are (1) have content jump around 
   on switch (2) have loads of whitespace when showing the entries 
   (3) have a tiny scrolled list ...
 - how to enter ports belongs into the (non-existent admittedly) help ("how do
   I specify a port" or so), not into the description IMHO - it's not at
   all obvious that the answer to "what number do I need to put there"
   is "none" most of the time. Also the only networks who commonly document
   the port are the ones that use a non-default one (we never tell users
   to join "#polari on irc.gnome.org:6667")

[0] https://raw.githubusercontent.com/gnome-design-team/gnome-mockups/master/polari/connections.png

Hi, these days spending lots of time on bugzilla looking at bugs of polari, couldn't help but chime in.

(In reply to Florian Müllner from comment #4)

>  - I'm divided on the network list - it looks nice in the mockup
>    with ~10 entries, but that's just not realistic. We'll likely 
>    get in the "but foo is in the list, so why not bar?" game and 
>    end up with a much larger list full of crap (empathy: 76, 
>    hexchat: 93, ...). Bug 711833 has a mockup that suggests using 
>    a preconfigured list to do entry completion[0] - maybe do that 
>    using the full list, and then display a very reduced list (five?); 
>    also not compelled by the "transition between list and entries" 
>    part: the options I see there are (1) have content jump around 
>    on switch (2) have loads of whitespace when showing the entries 
>    (3) have a tiny scrolled list ...

As initial mockup suggested, maybe we should do entry completion with a reduced list, five or even less. 

>  - how to enter ports belongs into the (non-existent admittedly) help ("how
> do
>    I specify a port" or so), not into the description IMHO - it's not at
>    all obvious that the answer to "what number do I need to put there"
>    is "none" most of the time. Also the only networks who commonly document
>    the port are the ones that use a non-default one (we never tell users
>    to join "#polari on irc.gnome.org:6667")

Though rare, there are sometimes non default ports, and it is confusing how to enter the same presently. Two options I can think of 1.) In mockup by Bastian, description below server title, instead of one example, add two example like "irc.gnome.org , irc.foo.bar:12332" this helps user who doesn't know about port give confidence to leave out the port details, as well as points how to use port for user with non default port. 2.) Have separate port field, filled with default port 6667 and in description something in lines of if you don't know the port, than it is 6667 most of the times

> 
> [0]
> https://raw.githubusercontent.com/gnome-design-team/gnome-mockups/master/
> polari/connections.png

(In reply to Kunaal Jain from comment #5)
> Though rare, there are sometimes non default ports, and it is confusing how
> to enter the same presently.

We have moved away from separate port fields elsewhere a while ago, and I don't see a good reason to bring that back in Polari. For what it's worth, nautilus uses a simple "Enter server address ..." in its address+port entry, and I don't think non-default ports are that more wide-spread for IRC than for SSH/FTP/etc.
So no, I disagree that we have to clutter our dialogs to make a rare case more discoverable (but I should reach out to the documentation team to get user help started)

(In reply to Kunaal Jain from comment #5)
> Have separate port field, filled with default port 6667 and in description 
> something in lines of if you don't know the port, than it is 6667 most of the
> times

Also we'd like to try using SSL for new accounts and only fall back to unencrypted traffic if the server doesn't have support. This only works for "undefined" ports though, as the two default to different ports - 6667 would enforce unencrypted accounts (unless the user knows which magic number to put in there), while 6697 would require the user to fix the resulting connection error when the server doesn't have SSL support.

(In reply to Florian Müllner from comment #6)
> We have moved away from separate port fields elsewhere a while ago, and I
> don't see a good reason to bring that back in Polari. For what it's worth,
> nautilus uses a simple "Enter server address ..." in its address+port entry,
> and I don't think non-default ports are that more wide-spread for IRC than
> for SSH/FTP/etc.
> So no, I disagree that we have to clutter our dialogs to make a rare case
> more discoverable (but I should reach out to the documentation team to get
> user help started)

I didn't know about that.

(In reply to Florian Müllner from comment #7)
> (In reply to Kunaal Jain from comment #5)
> > Have separate port field, filled with default port 6667 and in description 
> > something in lines of if you don't know the port, than it is 6667 most of the
> > times
> 
> Also we'd like to try using SSL for new accounts and only fall back to
> unencrypted traffic if the server doesn't have support. This only works for
> "undefined" ports though, as the two default to different ports - 6667 would
> enforce unencrypted accounts (unless the user knows which magic number to
> put in there), while 6697 would require the user to fix the resulting
> connection error when the server doesn't have SSL support.

I agree we need undefined port for that. But I still think we should add two examples in description, one with defined port and one without..

Created attachment 313145 [details]
proposal for asking for passwords dynamically

(In reply to Florian Müllner from comment #4)
> Not sure bugzilla is the best place to discuss design, but here we go:
> 
>  - I'd like to keep the ability to disable accounts
>    (removing and re-adding is not the same - you lose at least
>     your saved channels, and possibly chat history as well);
>    I do like the sidebar in the mockups though, so that'd need
>    to go elsewhere (details? or it's a fringe feature that's
>    covered by the online accounts panel?)

I think it is a bit of a fringe feature (I've only seen myself use it when developing/testing stuff). If we really wanted it in, we could expose a switch next to the "Server Information" label.


>  - I don't think instant-apply is the right choice here - failed
>    connection attempts while the server field is incomplete are
>    one thing ("irc.gn"), sending user information that *we* have
>    guessed (default nick, possible real name) without explicit
>    consent a very different one

I changed the dialog to be an action-dialog which should adress this (then you have a way to cancel your changes).


>  - before adding a password field, I'd like to try to support
>    server passwords dynamically first (i.e. in response to an
>    AUTH_REQUIRED error);

I'm attaching something alternative to this reply, let me know if that could work.


>  - I'm divided on the network list - it looks nice in the mockup
>    with ~10 entries, but that's just not realistic. We'll likely 
>    get in the "but foo is in the list, so why not bar?" game and 
>    end up with a much larger list full of crap (empathy: 76, 
>    hexchat: 93, ...). Bug 711833 has a mockup that suggests using 
>    a preconfigured list to do entry completion[0] - maybe do that 
>    using the full list, and then display a very reduced list (five?);

Displaying a reduced list could work well I think. Alternatively we could sort the list based on the GNOME community's needs (fx let GNOME and Freenode be "verified servers" or default recommendations).


>    also not compelled by the "transition between list and entries" 
>    part: the options I see there are (1) have content jump around 
>    on switch (2) have loads of whitespace when showing the entries 
>    (3) have a tiny scrolled list ...

I think a more pleasing-to-the-eye option would be to cross-fade transition across all the content so nothing is jumping at all.


>  - how to enter ports belongs into the (non-existent admittedly) help ("how
> do
>    I specify a port" or so), not into the description IMHO

I was comparing it a bit to e-mail where you often specify the port separately, but I read your arguments and I agree it's probably fine to avoid clutter if it's that rare (ie. not even used for entering private IRC server addresses which probably would be the biggest use case here).

Created attachment 313146 [details]
Wire of a revamped connections dialog (rev2).

Updated wire.

(In reply to Bastian Ilsø from comment #9)
> (In reply to Florian Müllner from comment #4)
> >  - I'd like to keep the ability to disable accounts
>
> I think it is a bit of a fringe feature

It is, but it is useful to me :-)

But yeah, I don't think it is generally useful to users, so I'm fine with using the Online Accounts panel if I need it (though I still disagree with all the telepathy accounts being exposed there, but that's a different story really ...)


> >  - before adding a password field, I'd like to try to support
> >    server passwords dynamically first (i.e. in response to an
> >    AUTH_REQUIRED error);
> 
> I'm attaching something alternative to this reply, let me know if that could
> work.

For server passwords, though the description is wrong. It's not at all about registered nicknames, but about the server requiring a password to connect (it's really really rare - probably the most popular use case is znc, which requires a password in its default configuration ...).
I don't think the same UI works for nickname registration though. We'd have to interfere any chats with 'NickServ' and display the authentification UI instead, which means we break lesser-used NickServ commands for the user - I'd rather use an info bar which offers to save the password when we detect NickServ authentification (a "register" or "identify" command, possibly adding support for alternative bots at a later point), then authenticate nicks for which we have passwords on connection without any kind of UI.
In any case, let's keep this bug to the connections dialog and use bug 709824 and bug 709982 for authentification.


> >    also not compelled by the "transition between list and entries" 
> >    part: the options I see there are (1) have content jump around 
> >    on switch (2) have loads of whitespace when showing the entries 
> >    (3) have a tiny scrolled list ...
> 
> I think a more pleasing-to-the-eye option would be to cross-fade transition
> across all the content so nothing is jumping at all.

That would leave options (2) or (3) - if you want to avoid jumping, the stack's children need to have the same height, so either you have a small list the height of two entries (plus spacing), or extensive whitespace below the two entries.

(In reply to Florian Müllner from comment #11)
let's keep this bug to the connections dialog and use bug
> 709824 and bug 709982 for authentification.
Replying in those bugs then. :)


> That would leave options (2) or (3) - if you want to avoid jumping, the
> stack's children need to have the same height, so either you have a small
> list the height of two entries (plus spacing), or extensive whitespace below
> the two entries.

What I meant was that you could avoid jumping by transitioning everything (fx https://youtu.be/8HECZ727RIs).

(In reply to Bastian Ilsø from comment #12)
> What I meant was that you could avoid jumping by transitioning everything
> (fx https://youtu.be/8HECZ727RIs).

OK, so *technically there is no jump, because the "user section" are actually "user section 1" and "user section 2" - but
  - the result still looks jumpy
  - duplicating controls like that is ugly
    (sure, keeping the entry texts in sync isn't hard,
     but what about other state like focus, selection, ...?)

Created attachment 313631 [details]
Wire of a revamped connections dialog (rev3)

Here's a new wire, proposing a new pattern.
 * It adresses the jumpy issues of the old revision.
 * It also makes it easier to "regret" hitting the plus button by providing a clear way to go back.
 * It also adds space so we can provide extra details such as descriptions of each server (that would be nice when exploring the networks in the list).
 * I also made a proposal wire showing how our current error handling might integrate with this design.

(In reply to Bastian Ilsø from comment #14)
>  * It adresses the jumpy issues of the old revision.

Indeed. I must admit that I'm no friend of pick-from-random-predefined-crap lists, but some folks keep asking for it, so I should probably just get over myself. The mockup does look like the nicest implementation I've seen so far.

However I think that creating a custom network needs to be more obvious:
 * some people don't use search (trust me, you don't want to know
   the number of complaints we still get about removing categories
   from gnome-shell's app picker)
 * if your search still produces some results (say, 2-3 entries), it's
   completely mysterious that narrowing it down to 0 results allows you
   to finally add the network you want

How about including a "Custom network"/"Add your own" entry that matches any search?

If we do include a predefined list, it's a good time to consider that a network can consist of multiple servers (see the TpAccountWidget UI in the Online Accounts panel or the raw data[0])

Some minor observations:
 * I'm not convinced that pre-filling fields with search terms is helpful: in reality,
   strings like "myc" or "cus con" are more likely than "MyCustomConnection", so we force
   the user to edit/delete fields (even when they are optional)
 * As noted, space in the sidebar is limited, so an image button might be better suited
   than an "Undo" button whose width is locale-dependent (Spanish: "Deshacer",
   German: "Rückgängig")


>  * I also made a proposal wire showing how our current error handling might
> integrate with this design.

Yikes, unaligned icons (in case of multiple accounts with errors). I see two options to avoid this:
 * use the same pattern as in the room list, i.e. use a context popover for removal
   and end-align the error icon
 * keep using a separate dialog in this case - editing other accounts or creating
   new ones are not interesting actions when trying to fix an account error

Could do both actually.


[0] https://git.gnome.org/browse/telepathy-account-widgets/tree/tp-account-widgets/irc-networks.xml

Created attachment 313765 [details]
Wire of a revamped connections dialog (rev6)

(In reply to Florian Müllner from comment #15)
> Indeed. I must admit that I'm no friend of pick-from-random-predefined-crap
> lists, but some folks keep asking for it, so I should probably just get over
> myself. 

heh, well IMO ideally this concept of connections shouldn't really be exposed to users. If we had infinite CPU, memory and a good hash table lookup algorithm i would just get rid of this preliminary step and merge all roomlists from all popular public IRC networks together and let you explore and join chatrooms by searching that..

..But it sounds a bit impossible to my ears and IMO a predefined list with some server description is the second best thing to that.


> How about including a "Custom network"/"Add your own" entry that matches any
search?
Tested some different approaches today and this one seems the best to me too.

> If we do include a predefined list, it's a good time to consider that a
> network can consist of multiple servers (see the TpAccountWidget UI in the
> Online Accounts panel or the raw data[0])

I put in a note:
"If a network contains multiple servers, the preset chooses the one with best ping or the closest one."


> Some minor observations:
>  * I'm not convinced that pre-filling fields with search terms is helpful:
> in reality,
>    strings like "myc" or "cus con" are more likely than
> "MyCustomConnection", so we force
>    the user to edit/delete fields (even when they are optional)

I changed the behavior notes a little. I think it makes sense to expect the user's input to be a title since that's the primary label of each listed item. To me it also seems natural that a user would type out the whole title if the item in the list indicated that it would utilize the text you entered as the name the custom connection you intend to create.


>  * As noted, space in the sidebar is limited, so an image button might be
> better suited
>    than an "Undo" button whose width is locale-dependent (Spanish:
> "Deshacer",
>    German: "Rückgängig")

We could do that. I'm unsure how strongly identifiable the undo icon is on its own though. I haven't seen it used elsewhere.


> Yikes, unaligned icons (in case of multiple accounts with errors). I see two
> options to avoid this:
>  * use the same pattern as in the room list, i.e. use a context popover for
> removal
>    and end-align the error icon

Makes sense although I'm a bit worried about discoverability. I would rather have a visible flat button end-aligned[0] but then, where would the error icon go..

Perhaps aligning the icon next to the end-aligned delete button with equal margin?


>  * keep using a separate dialog in this case - editing other accounts or
> creating
>    new ones are not interesting actions when trying to fix an account error

True, although deleting your connection because you can't figure out how to solve the error seems still an interesting action to keep the sidebar perhaps.


On another note I changed "Description" to "title" since I think description is a misleading word here. Also I changed "Server" to "Address" since we now have the "Server Information" label on top.

[0]: http://i.imgur.com/egBTKFO.png

(In reply to Bastian Ilsø from comment #16)
> heh, well IMO ideally this concept of connections shouldn't really be
> exposed to users. If we had infinite CPU, memory and a good hash table
> lookup algorithm i would just get rid of this preliminary step and merge all
> roomlists from all popular public IRC networks together and let you explore
> and join chatrooms by searching that..

I disagree. Not being a centrally managed service is one of the defining characteristics of IRC (it's even relatively simple to run your own, if that's what rides your boat) - it may even be the reason why you'd want to use it in the first place, rather than say Hangouts.


> > If we do include a predefined list, it's a good time to consider that a
> > network can consist of multiple servers (see the TpAccountWidget UI in the
> > Online Accounts panel or the raw data[0])
> 
> I put in a note:
> "If a network contains multiple servers, the preset chooses the one with
> best ping or the closest one."

Well, the whole point of having multiple servers is that we can pick the best one at connection time. Pinning the one that happened to be best when the connection was created seems like a step back to me.
Thinking about it, I don't see a very strong use case for meddling with a predefined network, so maybe just use a label instead of an entry - if users absolutely need to mess with it, they can click the "Choose from list" link and pick the "custom" entry.


> We could do that. I'm unsure how strongly identifiable the undo icon is on
> its own though. I haven't seen it used elsewhere.

Yeah, neither am I. Still, I can't think of any better fix for the locale problem (other than looking for a completely different undo solution, but I kinda like the one you are proposing)


> Makes sense although I'm a bit worried about discoverability. I would rather
> have a visible flat button end-aligned[0] but then, where would the error
> icon go..

Sidenote: if discoverability is an issue here, it's even more so in the room list, which should be a much more common action than removing an account.


> Perhaps aligning the icon next to the end-aligned delete button with equal
> margin?

Not thrilled by the double icons, but yeah, maybe. FWIW, the column of close buttons looks a bit heavy to me, so I would have gone with only showing them when hovered/focused, but that obviously doesn't work with a second icon there.
Another option would be to use selection mode for deletion, but removing multiple accounts at once doesn't look like something you'd do frequently.


> True, although deleting your connection because you can't figure out how to
> solve the error seems still an interesting action to keep the sidebar
> perhaps.

Not sure - you are likely to lose all chat history for that account in that case, so I'd rather not encourage that.

Created attachment 314006 [details]
Wire of a revamped connections dialog (rev8)

(In reply to Florian Müllner from comment #17)
> Not being a centrally managed service is one of the defining
> characteristics of IRC..

I think the freedom to choose network is a key feature when a user wear the channel creator hat. It is less relevant when a user wears the channel explorer hat - it can even become an obstacle. If you intend to create a channel, I would sell the network providers in a UI and sell the idea of becoming your own provider. If you intend to explore channels, I would sell the channels I think is relevant and interesting for you. For finding a specific channel on a specific server, you can search or you were given an IRC link.


> Well, the whole point of having multiple servers is that we can pick the
> best one at connection time.

Ah, right.


> Thinking about it, I don't see a very strong use case for meddling with a
> predefined network, so maybe just use a label instead of an entry - if users
> absolutely need to mess with it, they can click the "Choose from list" link
> and pick the "custom" entry.

Mmh, then we would probably store the search string used to filter the list. Could also be useful say in case you miscliked an item in the list.

Alternatively we could let the label be "Edit Connection" if it's a preset and "Choose from List" if you've chosen to make a custom field.


> Not thrilled by the double icons, but yeah, maybe. FWIW, the column of close
> buttons looks a bit heavy to me, so I would have gone with only showing them
> when hovered/focused, but that obviously doesn't work with a second icon
> there.
> Another option would be to use selection mode for deletion, but removing
> multiple accounts at once doesn't look like something you'd do frequently.

This revision introduces a pattern inspired from Allan's Settings mockup [0]
and then combined with some Undo functionality which might be more clear.
With this pattern it's probably also less likely that you accidentically press
the button.

> > True, although deleting your connection because you can't figure out how to
> > solve the error seems still an interesting action to keep the sidebar
> > perhaps.
> 
> Not sure - you are likely to lose all chat history for that account in that
> case, so I'd rather not encourage that.

Makes sense to me.

[0]: https://raw.githubusercontent.com/gnome-design-team/gnome-mockups/master/system-settings/network/aday2/network-wires.png

(In reply to Bastian Ilsø from comment #18)
> > Thinking about it, I don't see a very strong use case for meddling with a
> > predefined network, so maybe just use a label instead of an entry - if users
> > absolutely need to mess with it, they can click the "Choose from list" link
> > and pick the "custom" entry.
> 
> Mmh, then we would probably store the search string used to filter the list.
> Could also be useful say in case you miscliked an item in the list.

Maybe for the time the dialog is open. I think this is something we have to try out before making a decision.


> This revision introduces a pattern inspired from Allan's Settings mockup [0]
> and then combined with some Undo functionality which might be more clear.
> With this pattern it's probably also less likely that you accidentically
> press
> the button.

I like it. Still, some comments:

 * the screen at the top and the new-connection-with-predefined-network one
   (left screen on the 2nd last row) look subtly different for no apparent
   reason ("Remove" on the left vs. "Remove Connection" on the right,
   "Server Information" vs. network name); personally I'd prefer the bottom
   one, but I'm fine with either as long as they are consistent

 * for the network list, I'd like to reuse the one in empathy/gnome-control-center;
   so if we want a network description, we'd need to extent that format (needs minor
   changes in telepathy-account-widgets, but shouldn't be hard) and get someone to
   write descriptions for all those networks

 * with this redesign, I'm softening on bug 730892, I'd be fine with this approach:
   use SSL if the predefined network contains any servers with SSL support, 
   no SSL if it doesn't, and expose an option for custom connections

 * what if the users creates a new connection and selects a network that's already
   been used by another connection? I can think of two options:
     (1) Always allow the user to edit the connection name, not only for custom networks
     (2) Don't create a new connection in that case, but switch to the existing one
   Technically a connection is the nickname+server combination (not at all unlike 
    user@server for email), which would be an argument for (1); I don't see a strong
    use case though (apart from polari hacking, but it's still possible to set up
    a custom connection for that), so I'm fine with (2) as well

 * "My Connection will be removed" or "My Connection has been removed"?
    Should removing a connection disconnect the account and remove its rooms from the
    room list (and undo reestablish them), or should we wait until the removal is "applied"?
    I lean towards the former

 * "Apply remove" looks sketchy to me - does that mean that "Cancel" acts like "Undo"? Why are
   those actions different then? Maybe it'd be better to have a single "Close" button in that
   case (and generally when "Cancel" and "Apply" don't make a difference, i.e. when there are
   no actual changes that could be applied)?

 * There's actually a wider issue here: what happens when switching between connections? Say I
   clear the nickname of connection Foo ("Apply" turns insensitive), then switch to connection
   Bar ("Apply turns sensitive, because all required information is filled in)
   Last time I checked, KDE's settings used that pattern, and they "solved" the issue by popping
   up a "There are unapplied changes (Discard)(Apply)" dialog on view switches (*shudder*)

I can think of two possible options for the last issue:

 (1) Separate the sidebar from the connection details - when opening the dialog, show the list of
     connections (with (X) in the header), activating an entry switches to the corresponding details
     (header as in the mockups), any of "Cancel", "Apply"/"Create" and "Remove Connection" switches
     back to the list
 (2) Use an "edit" mode - only have an (X) in the headerbar until "edit" mode is activated (could either
     be via an explicit control, or simply the user starting to make changes); while in "edit" mode,
     the sidebar does not allow to switch between connections, and the headerbar shows the controls from
     the mockup; either of "Cancel", "Apply"/"Create" and "Remove Connection" leaves "edit" mode, but
     does not close the dialog

Any other ideas?

Created attachment 314378 [details]
Wire of a revamped connections dialog (rev9)

(In reply to Florian Müllner from comment #19)
>  * the screen at the top and the new-connection-with-predefined-network one
>    (left screen on the 2nd last row) look subtly different for no apparent
>    reason [..]

woops, overlooked that one, updated now.


 
>  * for the network list, I'd like to reuse the one in
> empathy/gnome-control-center;
>    so if we want a network description, we'd need to extent that format
> (needs minor
>    changes in telepathy-account-widgets, but shouldn't be hard) and get
> someone to
>    write descriptions for all those networks

We can open a bug, but i'll gladly do it otherwise. 


>  * with this redesign, I'm softening on bug 730892, I'd be fine with this
> approach:
>    use SSL if the predefined network contains any servers with SSL support, 
>    no SSL if it doesn't, and expose an option for custom connections

Personally I'd rather only support SSL connections for the user's own safety and not promote/allow insecure IRC chatting (at least not from GUI).


>  * what if the users creates a new connection and selects a network that's
> already
>    been used by another connection?

I think your proposal 2) is the best option here. Alternatively we could remove the network from the list, but that might lead to confusion (in case you missed that you already added that network and is trying to add it).


>  * What happens when switching between
> connections? Say I clear the nickname of connection Foo, then switch to connection Bar.

The buttons in the headerbar either applies all changes made or cancels all changes made. I don't think changing sensitivity of the Apply button makes sense. If you switch to connection Bar, then I suggest we validate if Foo is correct and if not, show an error icon next to connection Foo. The user would most likely notice and returns back to Foo to fix the error. Then we can highlight the nickname field and ask the user to enter a nickname.
If the user chooses to disregard the error and press "Apply", we return back to Polari's main window. If the user has rooms on Connection Foo, the connection would show up in the sidebar with an error icon next to it. If the user doesn't have rooms added to connection Foo, the user have to open the Join dialog and attempt to add rooms to connection Foo to make use of it. Here we also have an opportunity to show an error icon next to connection Foo in the sidebar and tell the user to fix connection Foo first before joining rooms in Foo (and provide a link to jump into the connections dialog).

I'm attaching some visuals to demonstrate.


>  * "Apply remove" looks sketchy to me - does that mean that "Cancel" acts
> like "Undo"?

I suggest pressing Apply will apply all changes you make to your connections. If the only change you make is removing a connection, then that's the change you apply. Perhaps a better label than "Apply" might be "Save"?

Created attachment 314379 [details]
Wire of how a connection error might be handled (rev9)

(In reply to Bastian Ilsø from comment #20)
> >  * with this redesign, I'm softening on bug 730892, I'd be fine with this
> > approach:
> >    use SSL if the predefined network contains any servers with SSL support, 
> >    no SSL if it doesn't, and expose an option for custom connections
> 
> Personally I'd rather only support SSL connections for the user's own safety
> and not promote/allow insecure IRC chatting (at least not from GUI).

Well, we *are* allowing non-SSL connections, and the patches in bug 730892 don't really change that - for instance when setting up a connection with "irc.gnome.org" or "irc.gimp.net" instead of, say, "irc.acc.umu.se", we get a certificate error that results in a fallback to an unencrypted connection. Worse, if the user knows that polari picks SSL by default, and that GNOME servers support SSL, she will wrongly assume that the connection is secure.

Luckily when using a network list, we have a better option than guessing - predefined networks already have all the SSL information we need to pick the right servers, so we can simply use SSL if available without any fallback. But for custom connections, an option might end up being the lesser evil when compared to silently falling back to an insecure connection or throwing cryptic dialogs at the user (where most users will click the "I have no idea what this is, but this button looks like it will allow me to start chatting?!" option).

So no, I don't think an SSL option is great, but guessing has its own issues - it's a matter of balance, and the possibility of only having the unloved option in the uncommon case of setting up a fringe/private connection has shifted the weight a bit in my opinion.

(Slightly related: we should spend a thought or two on migrating existing accounts for which we can match the server to a predefined network)


> >  * What happens when switching between
> > connections? Say I clear the nickname of connection Foo, then switch to connection Bar.
> 
> The buttons in the headerbar either applies all changes made or cancels all
> changes made. I don't think changing sensitivity of the Apply button makes
> sense.

I disagree - I don't think allowing the user to make changes we *know* will fail in 100% of cases makes sense. I'm also not convinced that having a prominent control apply (pun intended!) changes to both visible and hidden settings is a great UI pattern. It's certainly a new pattern as far as I know, so I think this requires some wider discussion that this bug report (oh, if there only was some hackfest where we could talk about stuff like this ;-) )



> >  * "Apply remove" looks sketchy to me - does that mean that "Cancel" acts
> > like "Undo"?
> 
> I suggest pressing Apply will apply all changes you make to your
> connections. If the only change you make is removing a connection, then
> that's the change you apply. Perhaps a better label than "Apply" might be
> "Save"?

That sounds even worse to me - "save" suggests "remember"/"keep for later"/"make permanent", but not "really delete this".

(In reply to Florian Müllner from comment #22)
> Well, we *are* allowing non-SSL connections, and the patches in bug 730892
> don't really change that

Well, I'm making the radical suggestion to stop allowing that (from Polari UI) and assume SSL for all connections (even custom ones) without any fallback. For safety reasons.


> Luckily when using a network list, we have a better option than guessing -
> predefined networks already have all the SSL information we need to pick the
> right servers, so we can simply use SSL if available without any fallback.

I think that's a good idea.


> the possibility of only having the unloved
> option in the uncommon case of setting up a fringe/private connection has
> shifted the weight a bit in my opinion.

I suppose..but you know my opinion on this. :)


> (Slightly related: we should spend a thought or two on migrating existing
> accounts for which we can match the server to a predefined network)

agree.


> I don't think allowing the user to make changes we *know* will
> fail in 100% of cases makes sense.

I wouldn't be against leaving the Apply button insensitive as long as one of your connections contain an empty required field, like the nickname. My concern was mostly that there are cases we cannot detect (such as an invalid server address) and this can lead to some inconsistent behavior.


> I'm also not convinced that having a
> prominent control apply (pun intended!) changes to both visible and hidden
> settings is a great UI pattern. 

Isn't that also what we do fx here?
http://i.imgur.com/6SJfdGj.png


> It's certainly a new pattern as far as I
> know, so I think this requires some wider discussion that this bug report
> (oh, if there only was some hackfest where we could talk about stuff like
> this ;-) )

looking forward to it. :)

(In reply to Bastian Ilsø from comment #23)
> Well, I'm making the radical suggestion to stop allowing that (from Polari
> UI) and assume SSL for all connections (even custom ones) without any
> fallback. For safety reasons.

Uhm - we only updated our *own* servers to support SSL five months ago. So no, if we ever want to do this, this is way too early. (And even then, it'll need a lot of convincing work: I think being overly paranoid with regard to SSL is silly for completely open network where everyone is able to join and publish logs for everything that is said; or where the added privacy is close to 0 because the network itself is private/encrypted)


> > I'm also not convinced that having a
> > prominent control apply (pun intended!) changes to both visible and hidden
> > settings is a great UI pattern. 
> 
> Isn't that also what we do fx here?
> http://i.imgur.com/6SJfdGj.png

Mmmh, in a way. However everything in the different tabs still applies to the same object, not to all changes across all the existing connections. IMHO there's a huge difference between:
 - change stuff in object A -> apply
 - delete object B
 - change stuff in object C -> cancel
and
 - change stuff in object A
 - delete object B
 - change stuff in object C
 - "Apply changes to objects A and C and confirm removal of B"?

Created attachment 316114 [details]
Wire of a revamped connections dialog (rev10)

(In reply to Florian Müllner from comment #24)
> Uhm - we only updated our *own* servers to support SSL five months ago. So
> no, if we ever want to do this, this is way too early.

Here's a revision's with a switch. Meh. :-)


> > Isn't that also what we do fx here?
> > http://i.imgur.com/6SJfdGj.png
> 
> Mmmh, in a way.

The attachment here is an experiment with using just an "x". Could work reasonably fine too.

Bastian a quick question. Do you these photos  actually coded working polari screenshots or designed using some software? Just curious.

(In reply to Kunaal Jain from comment #26)
> Bastian a quick question. Do you these photos  actually coded working polari
> screenshots or designed using some software? Just curious.

It's just mockups made in Inkscape, no coding. (:

I don't think it's useful to keep this open at this point - the discussion is already unwieldily long, and the design we settled on is either on master or in bug 761859 ...