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 755771 - When joining a room, auto-choose the network from selected room (if any)
When joining a room, auto-choose the network from selected room (if any)
Status: RESOLVED FIXED
Product: polari
Classification: Applications
Component: general
3.16.x
Other Linux
: Normal minor
: ---
Assigned To: Polari maintainers
Polari maintainers
Depends on:
Blocks:
 
 
Reported: 2015-09-29 04:53 UTC by Jean-François Fortin Tam
Modified: 2015-10-07 20:44 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Use network of the currently selected room (4.22 KB, patch)
2015-10-06 14:31 UTC, Kunaal Jain
none Details | Review
Use network of the currently selected room (4.92 KB, patch)
2015-10-07 18:57 UTC, Kunaal Jain
none Details | Review
Use network of the currently selected room (4.94 KB, patch)
2015-10-07 19:16 UTC, Kunaal Jain
none Details | Review
joindialog: Default to account of the currently selected room (4.96 KB, patch)
2015-10-07 20:44 UTC, Florian Müllner
committed Details | Review

Description Jean-François Fortin Tam 2015-09-29 04:53:09 UTC
I usually have something like this:

  irc.freenode.net
    foo
    bar
  irc.gnome.org
    [baz]

"baz" is selected. When I click the + button to join a new room, it defaults to freenode because it's the first one in the list. I think it would be more logical for it to auto-select the network of the currently selected room, "baz", which means "irc.gnome.org" in this case.
Comment 1 Florian Müllner 2015-09-29 14:48:24 UTC
(In reply to Jean-François Fortin Tam from comment #0)
> When I click the + button to join a new room, it defaults to freenode 
> because it's the first one in the list.

No, the join dialog remembers the last account used and uses that as a default the next time the dialog is shown, so it's less arbitrary than simply picking the first list entry.
I wouldn't mind changing that to be based on the active channel though. Bastian, what do you think?
Comment 2 Bastian Ilsø 2015-09-29 15:10:48 UTC
(In reply to Florian Müllner from comment #1)
> No, the join dialog remembers the last account used and uses that as a
> default the next time the dialog is shown, so it's less arbitrary than
> simply picking the first list entry.
> I wouldn't mind changing that to be based on the active channel though.
> Bastian, what do you think?

I agree that this would make more sense from a logical standpoint.
Comment 3 Bastian Ilsø 2015-09-29 15:11:51 UTC
(In reply to Bastian Ilsø from comment #2)
> (In reply to Florian Müllner from comment #1)
> > No, the join dialog remembers the last account used and uses that as a
> > default the next time the dialog is shown, so it's less arbitrary than
> > simply picking the first list entry.
> > I wouldn't mind changing that to be based on the active channel though.
> > Bastian, what do you think?
> 
> I agree that this would make more sense from a logical standpoint.

I mean, this would *also* make sense and is probably preferred in most cases.
Comment 4 Florian Müllner 2015-09-29 15:39:18 UTC
Should make for a nice beginner bug then ...
Comment 5 Kunaal Jain 2015-10-06 14:31:21 UTC
Created attachment 312735 [details] [review]
Use network of the currently selected room
Comment 6 Florian Müllner 2015-10-07 16:52:43 UTC
Review of attachment 312735 [details] [review]:

Looks mostly good to me, except for those nitpicks:
 * the 'last-used-account' setting is pointless if we
   don't use it, so should be removed from to the schema
 * the commit message should contain line-breaks at
   70-76 characters
 * bonus points for not only describing what the commit
   does (which should be pretty clear from the code anyway),
   but also briefly outlining why the change is done
Comment 7 Kunaal Jain 2015-10-07 18:48:39 UTC
(In reply to Florian Müllner from comment #6)
> Review of attachment 312735 [details] [review] [review]:
> 
> Looks mostly good to me, except for those nitpicks:
>  * the 'last-used-account' setting is pointless if we
>    don't use it, so should be removed from to the schema

Oh I missed that. Thanks

>  * the commit message should contain line-breaks at
>    70-76 characters
>  * bonus points for not only describing what the commit
>    does (which should be pretty clear from the code anyway),
>    but also briefly outlining why the change is done

Thanks for the pointers. Will try to be more descriptive.
Comment 8 Kunaal Jain 2015-10-07 18:57:00 UTC
Created attachment 312848 [details] [review]
Use network of the currently selected room
Comment 9 Kunaal Jain 2015-10-07 19:11:58 UTC
Uh oh, master has changed. I will update patch again.
Comment 10 Kunaal Jain 2015-10-07 19:16:41 UTC
Created attachment 312849 [details] [review]
Use network of the currently selected room
Comment 11 Florian Müllner 2015-10-07 20:44:29 UTC
Created attachment 312851 [details] [review]
joindialog: Default to account of the currently selected room

Looks good to me, thanks - I'll push the patch with a slightly modified commit message (mostly replacing "network" by "account", as we don't use the former anywhere).
Comment 12 Florian Müllner 2015-10-07 20:44:54 UTC
Attachment 312851 [details] pushed as 677829c - joindialog: Default to account of the currently selected room