Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Reimplement drag drop files on sql migration #528

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

seakrueger
Copy link
Collaborator

@seakrueger seakrueger commented Sep 28, 2024

commit 4e4bf6b

  • Reimplements Drag and drop files in and out of TagStudio #153
  • Maintains commit history and message
  • Tested working dragging video/audio files into VLC
  • Tested working dragging files into TagStudio
    • Known edge-case where thumbnails aren't updated if all dragged-in files overwrite existing files

commit 2f9ae0d

  • Updates dialog to match other similar dialog
  • Cleans up and simplifies a lot of the logic in drop_import
  • Tested working dragging files and folds into TagStudio

commit 803f430

  • Extracts useful function for ProgressWidget to that class
  • Implements said function where useful across the UI
  • Tested drop_import, find_unlinked, delete_unlinked, fix_unlinked
    • merge_entries and mirror_entries are untested as I don't have dupe_guru, but should work?

Commits and commits title/messages are formatted to maintain authorship and split features, don't squash on merge

@seakrueger seakrueger marked this pull request as draft September 28, 2024 20:48
@CyanVoxel CyanVoxel added this to the SQL Parity milestone Oct 1, 2024
@CyanVoxel CyanVoxel added Type: Enhancement New feature or request Type: QoL A quality of life (QoL) enhancement or suggestion Type: UI/UX User interface and/or user experience labels Oct 1, 2024
@seakrueger seakrueger force-pushed the reimplement-drag-drop branch 3 times, most recently from 18d57c5 to 0ddd9c7 Compare October 7, 2024 21:25
@seakrueger seakrueger force-pushed the reimplement-drag-drop branch 5 times, most recently from df66b2e to 34376ec Compare November 3, 2024 02:19
@seakrueger
Copy link
Collaborator Author

Sorry for the delay, this guy kind of slipped away into several tightly related features.

Old Dialog:
Screenshot_20241109_214325
New Dialog (matches delete_unlinked entries):
Screenshot_20241109_222806

* Ability to drop local files in to TagStudio to add to library

* Added renaming option to drop import

* Improved readability and switched to pathLib

* format

* Apply suggestions from code review

Co-authored-by: yed podtrzitko <[email protected]>

* Revert Change

* Update tagstudio/src/qt/modals/drop_import.py

Co-authored-by: yed podtrzitko <[email protected]>

* Added support for folders

* formatting

* Progress bars added

* Added Ability to Drag out of window

* f

* format

* Ability to drop local files in to TagStudio to add to library

* Added renaming option to drop import

* Improved readability and switched to pathLib

* format

* Apply suggestions from code review

Co-authored-by: yed podtrzitko <[email protected]>

* Revert Change

* Update tagstudio/src/qt/modals/drop_import.py

Co-authored-by: yed podtrzitko <[email protected]>

* Added support for folders

* formatting

* Progress bars added

* Added Ability to Drag out of window

* f

* format

* format

* formatting and refactor

* format again

* formatting for mypy

* convert lambda to func for clarity

* mypy fixes

* fixed dragout only worked on selected

* Refactor typo, Add license

* Reformat QMessageBox

* Disable drops when no library is open

Co-authored-by: Sean Krueger <[email protected]>

* Rebased onto SQL migration

* Updated logic to based on selected grid_idx instead of selected ids

* Add newly dragged-in files to SQL database

* Fix buttons being inconsistant across platforms

* Fix ruff formatting

* Rename "override" button to "overwrite"

---------

Co-authored-by: yed podtrzitko <[email protected]>
Co-authored-by: Travis Abendshien <[email protected]>
Co-authored-by: Sean Krueger <[email protected]>
@seakrueger seakrueger marked this pull request as ready for review November 10, 2024 04:58
Copy link
Collaborator

@yedpodtrzitko yedpodtrzitko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, but as usual - having some tests would be nice.

tagstudio/src/qt/modals/drop_import.py Outdated Show resolved Hide resolved
tagstudio/src/qt/modals/drop_import.py Outdated Show resolved Hide resolved
tagstudio/src/qt/modals/drop_import.py Outdated Show resolved Hide resolved
tagstudio/src/qt/modals/drop_import.py Outdated Show resolved Hide resolved
item.setEditable(False)
self.model.appendRow(item)

self.show()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When an action is required, I'd expect the window will gain focus. So I'd add self.driver.main_window.raise_() here. Otherwise the result might look like this, so I might not be even aware the drag&drop action havent finished:

Screenshot 2024-11-10 at 18 54 36

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added, but window already raised on my system so unsure if that is fixed for you

tagstudio/src/qt/modals/drop_import.py Outdated Show resolved Hide resolved
tagstudio/src/qt/modals/fix_unlinked.py Outdated Show resolved Hide resolved
tagstudio/src/qt/widgets/progress.py Outdated Show resolved Hide resolved
* Handle Qt events for main window in ts_qt.py

* Replace magic values with enums

* Match import duplicate file dialog to delete missing entry dialog

* Remove excessive progess widgets

* Add docstrings and logging
Extracts the create_progress_bar function from drop_import to the
ProgressWidget class. Instead of creating a ProgressWidget,
FunctionIterator, and CustomRunnable every time a thread-safe progress
widget is needed, the from_iterable function in ProgressWidget now
handles all of that.
@seakrueger
Copy link
Collaborator Author

Updated with feedback, still no tests as getting a lot of weird errors trying to work with files and copying in pytest

@CyanVoxel CyanVoxel added the Status: Review Needed A review of this is needed label Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Review Needed A review of this is needed Type: Enhancement New feature or request Type: QoL A quality of life (QoL) enhancement or suggestion Type: UI/UX User interface and/or user experience
Projects
Status: 🚧 In progress
Development

Successfully merging this pull request may close these issues.

4 participants