refactor: CreateGroupchat: Make Dialog more error resistent

- Add info label to separate errors and info messages
- Make advanced switch insensitive but active if no MUC service is offered
- Reset all fields correctly when switching accounts

3 files changed, 197 insertions, 189 deletions

diff --git a/gajim/data/gui/groupchat_creation.ui b/gajim/data/gui/groupchat_creation.ui
index 703cbe5a2..273e9f534 100644
--- a/gajim/data/gui/groupchat_creation.ui
+++ b/gajim/data/gui/groupchat_creation.ui
@@ -38,7 +38,7 @@
           </packing>
         </child>
         <child>
-          <!-- n-columns=2 n-rows=5 -->
+          <!-- n-columns=2 n-rows=9 -->
           <object class="GtkGrid" id="grid">
             <property name="visible">True</property>
             <property name="can-focus">False</property>
@@ -85,7 +85,6 @@
                 <property name="visible">True</property>
                 <property name="can-focus">True</property>
                 <property name="hexpand">True</property>
-                <signal name="changed" handler="_on_name_entry_changed" swapped="no"/>
               </object>
               <packing>
                 <property name="left-attach">1</property>
@@ -159,7 +158,7 @@
               </packing>
             </child>
             <child>
-              <object class="GtkLabel">
+              <object class="GtkLabel" id="advanced_switch_label">
                 <property name="visible">True</property>
                 <property name="can-focus">False</property>
                 <property name="halign">start</property>
@@ -172,147 +171,153 @@
               </packing>
             </child>
             <child>
-              <object class="GtkBox" id="advanced_box">
+              <object class="GtkLabel" id="error_label">
+                <property name="visible">True</property>
+                <property name="can-focus">False</property>
+                <property name="no-show-all">True</property>
+                <property name="wrap">True</property>
+                <property name="max-width-chars">38</property>
+                <style>
+                  <class name="error-color"/>
+                  <class name="padding-12"/>
+                </style>
+              </object>
+              <packing>
+                <property name="left-attach">0</property>
+                <property name="top-attach">8</property>
+                <property name="width">2</property>
+              </packing>
+            </child>
+            <child>
+              <object class="GtkLabel" id="info_label">
                 <property name="can-focus">False</property>
                 <property name="no-show-all">True</property>
-                <property name="halign">center</property>
-                <property name="orientation">vertical</property>
-                <property name="spacing">12</property>
+                <property name="wrap">True</property>
+                <property name="max-width-chars">38</property>
+                <style>
+                  <class name="padding-12"/>
+                </style>
+              </object>
+              <packing>
+                <property name="left-attach">0</property>
+                <property name="top-attach">7</property>
+                <property name="width">2</property>
+              </packing>
+            </child>
+            <child>
+              <object class="GtkLabel" id="address_entry_label">
+                <property name="can-focus">False</property>
+                <property name="no-show-all">True</property>
+                <property name="halign">end</property>
+                <property name="margin-top">12</property>
+                <property name="label" translatable="yes">_Address</property>
+                <property name="use-underline">True</property>
+                <style>
+                  <class name="dim-label"/>
+                </style>
+              </object>
+              <packing>
+                <property name="left-attach">0</property>
+                <property name="top-attach">6</property>
+              </packing>
+            </child>
+            <child>
+              <object class="GtkEntry" id="address_entry">
+                <property name="width-request">250</property>
+                <property name="can-focus">True</property>
+                <property name="no-show-all">True</property>
+                <property name="margin-top">12</property>
+                <signal name="changed" handler="_on_address_entry_changed" swapped="no"/>
+              </object>
+              <packing>
+                <property name="left-attach">1</property>
+                <property name="top-attach">6</property>
+              </packing>
+            </child>
+            <child>
+              <object class="GtkRadioButton" id="public_radio">
+                <property name="can-focus">True</property>
+                <property name="receives-default">False</property>
+                <property name="no-show-all">True</property>
+                <property name="margin-top">12</property>
+                <property name="draw-indicator">True</property>
+                <property name="group">private_radio</property>
                 <child>
-                  <object class="GtkRadioButton" id="private_radio">
+                  <object class="GtkBox">
                     <property name="visible">True</property>
-                    <property name="can-focus">True</property>
-                    <property name="receives-default">False</property>
-                    <property name="margin-top">12</property>
-                    <property name="active">True</property>
-                    <property name="draw-indicator">True</property>
-                    <property name="group">public_radio</property>
+                    <property name="can-focus">False</property>
+                    <property name="margin-start">6</property>
+                    <property name="orientation">vertical</property>
                     <child>
-                      <object class="GtkBox">
+                      <object class="GtkLabel">
                         <property name="visible">True</property>
                         <property name="can-focus">False</property>
                         <property name="halign">start</property>
-                        <property name="margin-start">6</property>
-                        <property name="orientation">vertical</property>
-                        <child>
-                          <object class="GtkLabel">
-                            <property name="visible">True</property>
-                            <property name="can-focus">False</property>
-                            <property name="halign">start</property>
-                            <property name="label" translatable="yes">Private</property>
-                            <property name="use-markup">True</property>
-                            <property name="wrap">True</property>
-                            <property name="xalign">0</property>
-                            <style>
-                              <class name="bold"/>
-                            </style>
-                          </object>
-                          <packing>
-                            <property name="expand">False</property>
-                            <property name="fill">True</property>
-                            <property name="position">0</property>
-                          </packing>
-                        </child>
-                        <child>
-                          <object class="GtkLabel">
-                            <property name="visible">True</property>
-                            <property name="can-focus">False</property>
-                            <property name="halign">start</property>
-                            <property name="label" translatable="yes">You have to invite people so they can join</property>
-                            <property name="wrap">True</property>
-                            <property name="xalign">0</property>
-                            <style>
-                              <class name="dim-label"/>
-                            </style>
-                          </object>
-                          <packing>
-                            <property name="expand">False</property>
-                            <property name="fill">True</property>
-                            <property name="position">1</property>
-                          </packing>
-                        </child>
+                        <property name="label" translatable="yes">Public</property>
+                        <property name="use-markup">True</property>
+                        <property name="xalign">0</property>
+                        <style>
+                          <class name="bold"/>
+                        </style>
                       </object>
+                      <packing>
+                        <property name="expand">False</property>
+                        <property name="fill">True</property>
+                        <property name="position">0</property>
+                      </packing>
                     </child>
-                  </object>
-                  <packing>
-                    <property name="expand">False</property>
-                    <property name="fill">True</property>
-                    <property name="position">0</property>
-                  </packing>
-                </child>
-                <child>
-                  <object class="GtkRadioButton" id="public_radio">
-                    <property name="visible">True</property>
-                    <property name="can-focus">True</property>
-                    <property name="receives-default">False</property>
-                    <property name="draw-indicator">True</property>
-                    <property name="group">private_radio</property>
                     <child>
-                      <object class="GtkBox">
+                      <object class="GtkLabel">
                         <property name="visible">True</property>
                         <property name="can-focus">False</property>
-                        <property name="margin-start">6</property>
-                        <property name="orientation">vertical</property>
-                        <child>
-                          <object class="GtkLabel">
-                            <property name="visible">True</property>
-                            <property name="can-focus">False</property>
-                            <property name="halign">start</property>
-                            <property name="label" translatable="yes">Public</property>
-                            <property name="use-markup">True</property>
-                            <property name="xalign">0</property>
-                            <style>
-                              <class name="bold"/>
-                            </style>
-                          </object>
-                          <packing>
-                            <property name="expand">False</property>
-                            <property name="fill">True</property>
-                            <property name="position">0</property>
-                          </packing>
-                        </child>
-                        <child>
-                          <object class="GtkLabel">
-                            <property name="visible">True</property>
-                            <property name="can-focus">False</property>
-                            <property name="halign">start</property>
-                            <property name="label" translatable="yes">Anyone can join</property>
-                            <property name="wrap">True</property>
-                            <property name="xalign">0</property>
-                            <style>
-                              <class name="dim-label"/>
-                            </style>
-                          </object>
-                          <packing>
-                            <property name="expand">False</property>
-                            <property name="fill">True</property>
-                            <property name="position">1</property>
-                          </packing>
-                        </child>
+                        <property name="halign">start</property>
+                        <property name="label" translatable="yes">Anyone can join</property>
+                        <property name="wrap">True</property>
+                        <property name="xalign">0</property>
+                        <style>
+                          <class name="dim-label"/>
+                        </style>
                       </object>
+                      <packing>
+                        <property name="expand">False</property>
+                        <property name="fill">True</property>
+                        <property name="position">1</property>
+                      </packing>
                     </child>
                   </object>
-                  <packing>
-                    <property name="expand">False</property>
-                    <property name="fill">True</property>
-                    <property name="position">1</property>
-                  </packing>
                 </child>
+              </object>
+              <packing>
+                <property name="left-attach">1</property>
+                <property name="top-attach">5</property>
+              </packing>
+            </child>
+            <child>
+              <object class="GtkRadioButton" id="private_radio">
+                <property name="can-focus">True</property>
+                <property name="receives-default">False</property>
+                <property name="no-show-all">True</property>
+                <property name="margin-top">12</property>
+                <property name="active">True</property>
+                <property name="draw-indicator">True</property>
                 <child>
                   <object class="GtkBox">
                     <property name="visible">True</property>
                     <property name="can-focus">False</property>
-                    <property name="halign">end</property>
-                    <property name="spacing">12</property>
+                    <property name="halign">start</property>
+                    <property name="margin-start">6</property>
+                    <property name="orientation">vertical</property>
                     <child>
                       <object class="GtkLabel">
                         <property name="visible">True</property>
                         <property name="can-focus">False</property>
-                        <property name="halign">end</property>
-                        <property name="label" translatable="yes">_Address</property>
-                        <property name="use-underline">True</property>
+                        <property name="halign">start</property>
+                        <property name="label" translatable="yes">Private</property>
+                        <property name="use-markup">True</property>
+                        <property name="wrap">True</property>
+                        <property name="xalign">0</property>
                         <style>
-                          <class name="dim-label"/>
+                          <class name="bold"/>
                         </style>
                       </object>
                       <packing>
@@ -322,11 +327,16 @@
                       </packing>
                     </child>
                     <child>
-                      <object class="GtkEntry" id="address_entry">
-                        <property name="width-request">250</property>
+                      <object class="GtkLabel">
                         <property name="visible">True</property>
-                        <property name="can-focus">True</property>
-                        <signal name="changed" handler="_on_address_entry_changed" swapped="no"/>
+                        <property name="can-focus">False</property>
+                        <property name="halign">start</property>
+                        <property name="label" translatable="yes">You have to invite people so they can join</property>
+                        <property name="wrap">True</property>
+                        <property name="xalign">0</property>
+                        <style>
+                          <class name="dim-label"/>
+                        </style>
                       </object>
                       <packing>
                         <property name="expand">False</property>
@@ -335,36 +345,19 @@
                       </packing>
                     </child>
                   </object>
-                  <packing>
-                    <property name="expand">False</property>
-                    <property name="fill">True</property>
-                    <property name="position">2</property>
-                  </packing>
-                </child>
-                <child>
-                  <object class="GtkLabel" id="error_label">
-                    <property name="visible">True</property>
-                    <property name="can-focus">False</property>
-                    <property name="no-show-all">True</property>
-                    <property name="wrap">True</property>
-                    <property name="max-width-chars">38</property>
-                    <style>
-                      <class name="warning-color"/>
-                    </style>
-                  </object>
-                  <packing>
-                    <property name="expand">False</property>
-                    <property name="fill">True</property>
-                    <property name="position">3</property>
-                  </packing>
                 </child>
               </object>
               <packing>
-                <property name="left-attach">0</property>
+                <property name="left-attach">1</property>
                 <property name="top-attach">4</property>
-                <property name="width">2</property>
               </packing>
             </child>
+            <child>
+              <placeholder/>
+            </child>
+            <child>
+              <placeholder/>
+            </child>
           </object>
           <packing>
             <property name="expand">False</property>
diff --git a/gajim/gtk/builder.pyi b/gajim/gtk/builder.pyi
index ddae1dbfd..3cac99713 100644
--- a/gajim/gtk/builder.pyi
+++ b/gajim/gtk/builder.pyi
@@ -402,11 +402,13 @@ class GroupchatCreationBuilder(Builder):
     account_combo: Gtk.ComboBox
     account_label: Gtk.Label
     advanced_switch: Gtk.Switch
-    advanced_box: Gtk.Box
-    private_radio: Gtk.RadioButton
-    public_radio: Gtk.RadioButton
-    address_entry: Gtk.Entry
+    advanced_switch_label: Gtk.Label
     error_label: Gtk.Label
+    info_label: Gtk.Label
+    address_entry_label: Gtk.Label
+    address_entry: Gtk.Entry
+    public_radio: Gtk.RadioButton
+    private_radio: Gtk.RadioButton
     spinner: Gtk.Spinner
     create_button: Gtk.Button
 
diff --git a/gajim/gtk/groupchat_creation.py b/gajim/gtk/groupchat_creation.py
index 63e5f1b23..a2db2e6ad 100644
--- a/gajim/gtk/groupchat_creation.py
+++ b/gajim/gtk/groupchat_creation.py
@@ -80,8 +80,7 @@ class CreateGroupchatWindow(Gtk.ApplicationWindow, EventHelper):
             return
 
         self._update_accounts(account)
-
-        self.set_focus(self._ui.address_entry)
+        self._ui.create_button.grab_focus()
 
     def _on_account_state(self,
                           _event: AccountConnected | AccountDisconnected
@@ -134,12 +133,16 @@ class CreateGroupchatWindow(Gtk.ApplicationWindow, EventHelper):
         self._ui.address_entry.set_placeholder_text(
             f'{placeholder[2]}@{server}')
 
+    def _has_muc_service(self, account: str) -> bool:
+        client = app.get_client(account)
+        return client.get_module('MUC').service_jid is not None
+
     def _get_muc_service_jid(self) -> str:
         assert self._account is not None
         client = app.get_client(self._account)
         service_jid = client.get_module('MUC').service_jid
         if service_jid is None:
-            return ''
+            return 'muc.example.org'
         return str(service_jid)
 
     def _on_key_press(self, _widget: Gtk.Widget, event: Gdk.EventKey) -> None:
@@ -154,23 +157,25 @@ class CreateGroupchatWindow(Gtk.ApplicationWindow, EventHelper):
             if not iter_:
                 return
             self._account = model[iter_][0]
+
         self._fill_placeholders()
+        self._unset_error()
+        self._unset_info()
+        self._ui.address_entry.get_buffer().set_text('', 0)
 
-        self._validate_jid(self._ui.address_entry.get_text())
+        has_muc_service = self._has_muc_service(self._account)
 
-        if not self._get_muc_service_jid():
-            self._set_warning(
-                _('Your server does not provide a group chat service. '
-                  'Please try with a different server.'))
-            self._ui.advanced_switch.set_active(True)
-        else:
-            self._ui.error_label.hide()
+        self._ui.advanced_switch.set_active(not has_muc_service)
+        self._ui.advanced_switch.set_sensitive(has_muc_service)
+
+        if not has_muc_service:
+            self._set_info(
+                _('Your server does not offer a group chat service. '
+                  'Please specify the address of a different server.'))
 
-    def _validate_jid(self, text: str) -> None:
+    def _is_jid_valid(self, text: str) -> bool:
         if not text:
-            self._ui.error_label.hide()
-            self._ui.create_button.set_sensitive(False)
-            return
+            return True
 
         try:
             jid = validate_jid(text)
@@ -178,10 +183,9 @@ class CreateGroupchatWindow(Gtk.ApplicationWindow, EventHelper):
                 raise ValueError
 
         except ValueError:
-            self._set_warning(_('Invalid Address'))
-        else:
-            self._ui.error_label.hide()
-            self._ui.create_button.set_sensitive(True)
+            return False
+
+        return True
 
     def _set_processing_state(self, enabled: bool) -> None:
         if enabled:
@@ -191,32 +195,36 @@ class CreateGroupchatWindow(Gtk.ApplicationWindow, EventHelper):
             self._ui.spinner.stop()
         self._ui.grid.set_sensitive(not enabled)
 
-    def _set_warning(self, text: str) -> None:
+    def _unset_info(self) -> None:
+        self._ui.info_label.hide()
+
+    def _set_info(self, text: str) -> None:
+        self._ui.info_label.set_text(text)
+        self._ui.info_label.show()
+
+    def _unset_error(self) -> None:
+        self._ui.error_label.hide()
+        self._ui.create_button.set_sensitive(True)
+
+    def _set_error(self, text: str) -> None:
         self._ui.error_label.set_text(text)
         self._ui.error_label.show()
-        self._ui.advanced_switch.set_active(True)
         self._ui.create_button.set_sensitive(False)
 
-    def _set_warning_from_error(self, error: StanzaError) -> None:
+    def _set_error_from_error(self, error: StanzaError) -> None:
         condition = error.condition or ''
         if condition == 'gone':
             condition = 'already-exists'
         text = MUC_DISCO_ERRORS.get(condition, to_user_string(error))
-        self._set_warning(text)
-
-    def _set_warning_from_error_code(self, error_code: str) -> None:
-        self._set_warning(MUC_DISCO_ERRORS[error_code])
+        self._set_error(text)
 
-    def _on_name_entry_changed(self, entry: Gtk.Entry) -> None:
-        name = entry.get_text()
-        name = name.replace(' ', '-')
-        server = self._get_muc_service_jid()
-        self._ui.address_entry.set_text(f'{name.lower()}@{server}')
+    def _set_error_from_error_code(self, error_code: str) -> None:
+        self._set_error(MUC_DISCO_ERRORS[error_code])
 
     def _on_address_entry_changed(self, entry: Gtk.Entry) -> None:
         text = entry.get_text()
         self._update_entry_completion(entry, text)
-        self._validate_jid(text)
+        self._unset_error()
 
     def _update_entry_completion(self, entry: Gtk.Entry, text: str) -> None:
         text = entry.get_text()
@@ -231,11 +239,12 @@ class CreateGroupchatWindow(Gtk.ApplicationWindow, EventHelper):
         model.append([f'{text}@{server}'])
 
     def _on_toggle_advanced(self, switch: Gtk.Switch, *args: Any) -> None:
-        if switch.get_active():
-            self._ui.advanced_box.set_no_show_all(False)
-            self._ui.advanced_box.show_all()
-        else:
-            self._ui.advanced_box.hide()
+        self._unset_error()
+        active = switch.get_active()
+        self._ui.address_entry.set_visible(active)
+        self._ui.address_entry_label.set_visible(active)
+        self._ui.public_radio.set_visible(active)
+        self._ui.private_radio.set_visible(active)
 
     def _on_create_clicked(self, _button: Gtk.Button) -> None:
         assert self._account is not None
@@ -250,6 +259,10 @@ class CreateGroupchatWindow(Gtk.ApplicationWindow, EventHelper):
             server = self._get_muc_service_jid()
             room_jid = f'{get_random_muc_localpart()}@{server}'
 
+        if not self._is_jid_valid(room_jid):
+            self._set_error(_('Invalid Address'))
+            return
+
         self._set_processing_state(True)
         client = app.get_client(self._account)
         client.get_module('Discovery').disco_info(
@@ -264,10 +277,10 @@ class CreateGroupchatWindow(Gtk.ApplicationWindow, EventHelper):
                 assert error.jid is not None
                 self._create_muc(error.jid)
                 return
-            self._set_warning_from_error(error)
+            self._set_error_from_error(error)
 
         else:
-            self._set_warning_from_error_code(
+            self._set_error_from_error_code(
                 'already-exists' if result.is_muc else 'not-muc-service')
 
         self._set_processing_state(False)