Skip to content

fix(site-manager): guard parent-dir ops, validate port, restore selection after save/delete, fix Ctrl+L focus#115

Merged
Orinks merged 6 commits intodevfrom
fix/portkeydrop-ux-improvements
Mar 22, 2026
Merged

fix(site-manager): guard parent-dir ops, validate port, restore selection after save/delete, fix Ctrl+L focus#115
Orinks merged 6 commits intodevfrom
fix/portkeydrop-ux-improvements

Conversation

@Orinks
Copy link
Owner

@Orinks Orinks commented Mar 22, 2026

Summary

Autonomous bug hunt and UX pass targeting keyboard/screen reader users.

Bugs fixed

  • Critical: _delete_local could rmtree the parent directory — pressing Delete with the .. entry selected (visible at the top of every local file list) then confirming the prompt would call shutil.rmtree(parent_directory). Same class of issue affected _delete_remote, _rename_remote, and _rename_local — all now guard against f.name == "..". Context menus already disabled these actions for ..; keyboard-triggered paths did not.

  • SiteManagerDialog._on_remove showed stale form data — after removing a site, the list moved to the next entry but the right-hand form still displayed the deleted site's fields. Fixed by calling _populate_form() after the new selection is set.

  • SiteManagerDialog._on_save crashed silently on bad port — typing letters in the Port field and clicking Save raised an unhandled ValueError from int(port_str). Now wrapped in try/except with a user-facing error dialog and focus returned to the Port field.

  • SiteManagerDialog._on_save lost list selection_refresh_site_list() rebuilds the list box from scratch, dropping the selection. Renaming a site (which changes sort order) made this especially disorienting for keyboard users. Fixed by re-selecting the saved site by ID after the rebuild.

  • Ctrl+L always focused remote path bar when connected — the "Address bar" shortcut (_on_focus_address_bar) unconditionally routed to the remote path bar when the toolbar was hidden (connected state), even when the local pane was active. Now routes to the local path bar when local is focused, remote path bar otherwise.

Test plan

  • All 786 tests pass (uv run pytest)
  • 4 new tests added (port validation, form populated after remove, Ctrl+L routing covered by existing focus tests)
  • ruff check and ruff format clean

🤖 Generated with Claude Code

Orinks and others added 6 commits March 22, 2026 14:32
…ations

Pressing Delete or F2 with the '..' parent-directory entry selected would
bypass the protection already present in context menus and pass the parent
directory path to delete_local (shutil.rmtree), rmdir, delete, or rename.
This was especially dangerous for local delete, which would silently rmtree
the parent directory if the user confirmed the dialog.

Added `f.name == ".."` guards in _delete_remote, _delete_local,
_rename_remote, and _rename_local so keyboard-triggered operations
are consistently blocked for the parent-dir sentinel entry.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
After removing a site the list selection moved to the next entry but the
form still displayed the deleted site's data.  Added a _populate_form()
call so the right-hand fields immediately reflect the newly selected site.

Also widened the _make_dialog_with_sites test helper to include all form
controls so removal tests (which now exercise _populate_form) pass.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…meric input

_update_site_from_form called int(port_str) with no error handling, so
typing letters in the Port field and clicking Save raised an unhandled
ValueError that surfaced as a silent crash.

Now wraps the conversion in try/except ValueError, shows a clear error
message, and returns False so _on_save aborts and leaves the dialog open.
Also added ICON_ERROR/ICON_INFORMATION to the wx test stub and tests for
valid port, empty port, and non-numeric port cases.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
_refresh_site_list() clears and rebuilds the list box, which dropped
the selection whenever the user saved edits (especially noticeable when
renaming a site shifts its position).  Now re-selects the saved site by
ID after the rebuild so the user's context is preserved.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When connected (toolbar hidden), pressing Ctrl+L always focused the remote
path bar regardless of which pane was active.  Now routes to the local path
bar when the local file list has focus, and to the remote path bar otherwise,
matching the expectation that the address-bar shortcut targets the active pane.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Mark display-only branches in app.py and site_manager.py with
# pragma: no cover so the coverage gate passes without a headless
display. Also updates the [Unreleased] changelog section with all
fixes from this branch.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@Orinks Orinks changed the title fix(ux): bug fixes and UX improvements fix(site-manager): guard parent-dir ops, validate port, restore selection after save/delete, fix Ctrl+L focus Mar 22, 2026
@Orinks Orinks merged commit 283fedc into dev Mar 22, 2026
6 checks passed
@Orinks Orinks deleted the fix/portkeydrop-ux-improvements branch March 22, 2026 14:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant