Looking at roomList.js and connections.js, it seems like the major difference between creation and deletion of connections is that we call req.create_account_async (where req is a Tp.AccountRequest obj) in const ConnectionDetails._createAccount, which itself calls req.create_account_finish(res).

I'm completely unfamiliar with Telepathy, though, so I have no idea (for now) how to resolve the issue.

Theoretically, though, maybe we can:

- Store the account we're editing
- Remove it temporarily
- Change the temporary room list
- Add it back to the connection list as a new connection

This would at least replicate the behavior of sorting after adding a new connection.

(In reply to Cody Welsh from comment #1)
> Looking at roomList.js and connections.js, it seems like the major
> difference between creation and deletion of connections ...

Account deletion is not involved here at all - it's really just a property change (the name shown to the user) on a particular account object.


> Theoretically, though, maybe we can:
> 
> - Store the account we're editing
> - Remove it temporarily
> - Change the temporary room list
> - Add it back to the connection list as a new connection

The list doesn't really contain connections, it's a list of rooms that is sorted by account (and room type/name), then we insert a header before each row where the account is different from the one of the previous row (see gtk_list_box_set_sort_func() and gtk_list_box_set_header_func() and how we use them in roomList.js).

The closest to your suggestion is disabling the account while editing, but that has the effect that you are disconnected from all discussions during that time - not good. Luckily, that shouldn't be necessary - we can tell the list to resort using gtk_list_box_invalidate_sort() when any of the accounts in the list changes its display name (connect to the 'notify::display-name' signal) ...

(In reply to Florian Müllner from comment #2)
> we can tell the
> list to resort using gtk_list_box_invalidate_sort() when any of the accounts
> in the list changes its display name (connect to the 'notify::display-name'
> signal) ...

Oh, I guess that was the missing part in my initial experimentation. I tried that a bunch of times in different locations, but didn't know about connecting said function to the signal. That would make a lot more sense! I'll take a look into it.

Is there any way to access the current parent of a Gtk element? Since we're already connecting to notify::display-name in RoomListHeader, it would be pretty convenient to just call gtk_list_box_invalidate_sort() on the parent list box.

(In reply to Cody Welsh from comment #4)
> Is there any way to access the current parent of a Gtk element?

Yup, gtk_widget_get_parent(). Note that the function may return null when the widget currently doesen't have a parent - we'll hit that case at least in the explicit call in _init().

Created attachment 313165 [details] [review]
ui: resort room list if header names are changed

When a user updates a connection's name, the header will also change
its name. However, the room list will not resort in case the headers
are then out of order. This patch attempts to resort the room list
every time the display name is changed.

Review of attachment 313165 [details] [review]:

Please provide a commit message[0] that briefly outlines why the change is done.

[0] https://wiki.gnome.org/Git/WorkingWithPatches#How_to_make_a_patch

::: src/roomList.js
@@ +185,3 @@
     _onDisplayNameChanged: function() {
         this._label.label = this._account.display_name;
+        if (this.get_parent() !== null) {

Nitpick: != is a better comparison here than !== - it's not like calling invalidate_sort() on undefined would work any more than calling it on null

@@ +186,3 @@
         this._label.label = this._account.display_name;
+        if (this.get_parent() !== null) {
+          this.get_parent().invalidate_sort();

This assumes that the GtkListBox is a direct parent of the header, which happens to be the case now, but there's no guarantee in the API that this won't change at some point.

You should at least do something like:

let parent = this.get_parent();
if (parent != null && parent instanceof Gtk.ListBox)
    parent.invalidate_sort();

to avoid exceptions when the assumption breaks. Or even future-proof the code a bit:

let parent;
do
    parent = this.get_parent();
while (parent && !(parent instanceof Gtk.ListBox));

if (parent)
    parent.invalidate_sort();

(In reply to Cody Welsh from comment #6)
> Created attachment 313165 [details] [review] [review]
> ui: resort room list if header names are changed
> 
> When a user updates a connection's name, the header will also change
> its name. However, the room list will not resort in case the headers
> are then out of order. This patch attempts to resort the room list
> every time the display name is changed.

Oh, so this looks like the commit message, but it got lost when attaching the patch - in that case just minor nitpicks:
 - prefix should be "roomList", not "ui" ("ui" is either used for
   changes to GtkBuilder files, or for UI changes that span various
   modules - neither is the case here)
 - use sentence capitalization in the subject: Resort ... instead of resort

Created attachment 313221 [details] [review]
roomList: Resort room list if header names are changed

When a user updates a connection's name, the header will also change
its name. However, the room list will not resort in case the headers
are then out of order. This patch attempts to resort the room list
every time the display name is changed.

I hadn't considered the part about future-proofing! Sorry, I'm still learning the ecosystem :)

Created attachment 313223 [details] [review]
roomList: Resort when header names are changed

Last patch version is fine, except that it applies to the previous version instead of master - here's the squashed patch that I'll push.

Attachment 313223 [details] pushed as f796456 - roomList: Resort when header names are changed