diff --git a/CHANGELOG.md b/CHANGELOG.md index 08bd8d0..190fc3b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,11 @@ All notable changes to this project will be documented in this file. - Set default button per wizard step in ImportConnectionsDialog - Set initial focus in MigrationDialog checkboxes for screen reader announcement - Focus remote path bar when toolbar is hidden in main app window +- Ctrl+L focuses local path bar when local pane is active (previously always focused remote) +- Restore site list selection after saving a site in Site Manager +- Validate port field on save in Site Manager; show error for non-numeric input +- Populate form with next site after removing a site in Site Manager +- Guard against `..` entry in delete and rename operations to prevent parent-directory changes ## [0.2.0] - 2026-03-10 diff --git a/src/portkeydrop/app.py b/src/portkeydrop/app.py index 18fee2f..36ac5ed 100644 --- a/src/portkeydrop/app.py +++ b/src/portkeydrop/app.py @@ -804,9 +804,15 @@ def _on_focus_address_bar(self, event: wx.CommandEvent) -> None: self.tb_host.SetFocus() self._announce("Address bar") else: - # When connected the toolbar is hidden; route to the active path bar. - self.remote_path_bar.SetFocus() # pragma: no cover - self._announce("Remote path") # pragma: no cover + # When connected the toolbar is hidden; route to whichever path bar + # matches the currently active pane so the user can edit the path + # without having to navigate to the right panel first. + if self._is_local_focused(): # pragma: no cover + self.local_path_bar.SetFocus() + self._announce("Local path") + else: # pragma: no cover + self.remote_path_bar.SetFocus() + self._announce("Remote path") def _refresh_remote_files(self) -> None: if not self._client or not self._client.connected: @@ -1363,7 +1369,7 @@ def _on_delete(self, event) -> None: def _delete_remote(self) -> None: f = self._get_selected_remote_file() - if not f or not self._client: + if not f or not self._client or f.name == "..": return result = wx.MessageBox( f"Delete {f.name}?", "Confirm Delete", wx.YES_NO | wx.ICON_WARNING, self @@ -1384,7 +1390,7 @@ def _delete_remote(self) -> None: def _delete_local(self) -> None: f = self._get_selected_local_file() - if not f: + if not f or f.name == "..": # pragma: no cover return result = wx.MessageBox( f"Delete {f.name}?", "Confirm Delete", wx.YES_NO | wx.ICON_WARNING, self @@ -1405,7 +1411,7 @@ def _on_rename(self, event) -> None: def _rename_remote(self) -> None: f = self._get_selected_remote_file() - if not f or not self._client: + if not f or not self._client or f.name == "..": return dlg = wx.TextEntryDialog(self, "New name:", "Rename", f.name) dlg.SetName("Rename File") @@ -1427,7 +1433,7 @@ def _rename_remote(self) -> None: def _rename_local(self) -> None: f = self._get_selected_local_file() - if not f: + if not f or f.name == "..": # pragma: no cover return dlg = wx.TextEntryDialog(self, "New name:", "Rename", f.name) dlg.SetName("Rename File") diff --git a/src/portkeydrop/dialogs/site_manager.py b/src/portkeydrop/dialogs/site_manager.py index 6f1feb8..a2811f9 100644 --- a/src/portkeydrop/dialogs/site_manager.py +++ b/src/portkeydrop/dialogs/site_manager.py @@ -192,6 +192,7 @@ def _on_remove(self, event: wx.CommandEvent) -> None: new_idx = min(idx, count - 1) if idx != wx.NOT_FOUND else 0 self.site_list.SetSelection(new_idx) self._selected_site = self._site_manager.sites[new_idx] + self._populate_form(self._selected_site) wx.CallAfter(self.site_list.SetFocus) def _on_toggle_password(self, event: wx.CommandEvent) -> None: @@ -253,20 +254,43 @@ def _on_toggle_password(self, event: wx.CommandEvent) -> None: def _on_save(self, event: wx.CommandEvent) -> None: if not self._selected_site: return - self._update_site_from_form(self._selected_site) + if not self._update_site_from_form(self._selected_site): # pragma: no cover + return + saved_id = self._selected_site.id # pragma: no cover self._site_manager.update(self._selected_site) self._refresh_site_list() - - def _update_site_from_form(self, site: Site) -> None: + # Re-select the saved site so the list selection is not lost when the + # name changes (which would shift its position in the rebuilt list). + for i in range(self.site_list.GetCount()): # pragma: no cover + if self.site_list.GetClientData(i) == saved_id: + self.site_list.SetSelection(i) + break + + def _update_site_from_form(self, site: Site) -> bool: + """Update site from form fields. Returns False if validation fails.""" site.name = self.name_text.GetValue().strip() site.protocol = self.protocol_choice.GetStringSelection() site.host = self.host_text.GetValue().strip() port_str = self.port_text.GetValue().strip() - site.port = int(port_str) if port_str else 0 + if port_str: + try: + site.port = int(port_str) + except ValueError: + wx.MessageBox( + f"Invalid port number: {port_str!r}. Please enter a number.", + "Invalid Port", + wx.OK | wx.ICON_ERROR, + self, + ) + self.port_text.SetFocus() + return False + else: + site.port = 0 site.username = self.username_text.GetValue().strip() site.password = self.password_text.GetValue() site.key_path = self.key_path_text.GetValue().strip() site.initial_dir = self.initial_dir_text.GetValue().strip() or "/" + return True def _on_list_key(self, event: wx.KeyEvent) -> None: """Connect on Enter, let other keys pass through.""" diff --git a/tests/test_site_manager_dialog.py b/tests/test_site_manager_dialog.py index cd0655f..653e1d1 100644 --- a/tests/test_site_manager_dialog.py +++ b/tests/test_site_manager_dialog.py @@ -231,6 +231,8 @@ def _make_fake_wx(): wx.OK = 5100 wx.ID_OK = 5100 wx.CANCEL = 5101 + wx.ICON_ERROR = 0x10000 + wx.ICON_INFORMATION = 0x20000 wx.ALIGN_CENTER_VERTICAL = 1 wx.ALIGN_RIGHT = 2 wx.LEFT = 4 @@ -424,7 +426,14 @@ def remove_site(site_id): dlg._selected_site = None dlg._connect_requested = False dlg.site_list = _ListBox() + dlg.name_text = _TextCtrl() + dlg.protocol_choice = _Choice(choices=["sftp", "ftp", "ftps"]) + dlg.host_text = _TextCtrl() + dlg.port_text = _TextCtrl() + dlg.username_text = _TextCtrl() dlg.password_text = _TextCtrl() + dlg.key_path_text = _TextCtrl() + dlg.initial_dir_text = _TextCtrl() dlg.show_password_btn = _Button() dlg.Layout = MagicMock() @@ -500,3 +509,74 @@ def test_remove_updates_selected_site_to_remaining_selection(self, dialog_module # Logical selection should track the newly selected row ("C"). assert dlg._selected_site is dlg._site_manager.sites[1] + + def test_remove_populates_form_with_next_site(self, dialog_module): + mod, fake_wx = dialog_module + dlg, sites = self._make_dialog_with_sites(mod, fake_wx, ["Alpha", "Beta", "Gamma"]) + sites[2].host = "gamma.example.com" + + # Select "Beta" (index 1) + dlg.site_list.SetSelection(1) + dlg._selected_site = sites[1] + + mod.SiteManagerDialog._on_remove(dlg, MagicMock()) + + # Form should reflect "Gamma" (now at index 1), not the removed "Beta". + assert dlg.host_text._value == "gamma.example.com" + + +class TestPortValidation: + """Port field validation in _update_site_from_form.""" + + def _make_form_dialog(self, mod, fake_wx): + from portkeydrop.sites import Site + + site = Site(name="Test") + site_manager = MagicMock() + + dlg = object.__new__(mod.SiteManagerDialog) + dlg._site_manager = site_manager + dlg._selected_site = site + dlg.name_text = _TextCtrl() + dlg.name_text.SetValue("Test") + dlg.protocol_choice = _Choice(choices=["sftp", "ftp", "ftps"]) + dlg.host_text = _TextCtrl() + dlg.port_text = _TextCtrl() + dlg.username_text = _TextCtrl() + dlg.password_text = _TextCtrl() + dlg.key_path_text = _TextCtrl() + dlg.initial_dir_text = _TextCtrl() + dlg.initial_dir_text.SetValue("/home") + return dlg, site + + def test_valid_port_accepted(self, dialog_module): + mod, fake_wx = dialog_module + dlg, site = self._make_form_dialog(mod, fake_wx) + dlg.port_text.SetValue("2222") + + result = mod.SiteManagerDialog._update_site_from_form(dlg, site) + + assert result is True + assert site.port == 2222 + + def test_empty_port_defaults_to_zero(self, dialog_module): + mod, fake_wx = dialog_module + dlg, site = self._make_form_dialog(mod, fake_wx) + dlg.port_text.SetValue("") + + result = mod.SiteManagerDialog._update_site_from_form(dlg, site) + + assert result is True + assert site.port == 0 + + def test_non_numeric_port_shows_error_and_returns_false(self, dialog_module): + mod, fake_wx = dialog_module + dlg, site = self._make_form_dialog(mod, fake_wx) + dlg.port_text.SetValue("abc") + dlg.port_text.SetFocus = MagicMock() + + result = mod.SiteManagerDialog._update_site_from_form(dlg, site) + + assert result is False + fake_wx.MessageBox.assert_called() + dlg.port_text.SetFocus.assert_called_once()