Dynamically turn off transfer checkboxes.#653
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds functionality to dynamically turn off mutually exclusive transfer checkboxes in the TUI. When certain checkboxes (like "all" or "all_datatype") are selected, other incompatible checkboxes are automatically deselected to maintain logical consistency.
Key changes:
- Introduced
TransferDatatypeCheckboxessubclass that extends the base checkbox class with mutual exclusion logic - Refactored parameter naming from
create_or_transfertotab_namefor clarity - Added comprehensive test coverage for the new dynamic on/off behavior
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
datashuttle/tui/screens/datatypes.py |
Adds TransferDatatypeCheckboxes subclass with logic to automatically turn off mutually exclusive checkboxes; renames parameter from create_or_transfer to tab_name; adds helper function get_datatype_from_checkbox_name |
datashuttle/tui/tabs/transfer.py |
Switches from DatatypeCheckboxes to TransferDatatypeCheckboxes for transfer tab checkboxes |
datashuttle/tui/tabs/create_folders.py |
Updates DatatypeCheckboxes instantiation to use renamed tab_name parameter |
tests/tests_tui/test_tui_widgets_and_defaults.py |
Removes "all" and "all_datatype" from existing test loop; adds new test test_transfer_checkboxes_dynamic_on_off to verify mutual exclusion behavior |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@copilot can you give feedback on the inheritance structure on datatypes.py e.g. |
|
@JoeZiminski I've opened a new pull request, #678, to work on those changes. Once the pull request is ready, I'll request review from you. |
alessandrofelder
left a comment
alessandrofelder
left a comment
There was a problem hiding this comment.
As the original author of #537 I did some user-testing and confirm I am happy :)
Code changes are legible and understandable to me, so all good, I think.
|
Cheers @alessandrofelder! |
This PR properly fixes #537 as it is an issue that has come up a few times. The problem was that you could click mutually exclusive options for the custom transfer window datatype selection checkboxes. for example, you could click 'all' and 'behav' and it would then throw an errror on transfer. Now, the check boxes properly respond such that if you click one of a mutually exclusive set of options, all other options will be deselected.
Tests have been added, no additional documentation required.