Drag & Drop
Request: Drag/Drop game folder onto app window #68
Allows users to easily select game folders by dragging them directly onto the application window. I've refactored the code to consolidate folder selection logic into a single handle_folder_selection function, improving code organization and reducing redundancy. The new implementation handles both drag-and-drop and button click scenarios seamlessly.
I've also added error handling for invalid folder selections and created a separate reset_ui function for better UI state management. The changes include updating the UI to reflect the selected folder and automatically populating the game name entry with the folder name.
The implementation includes a smart feature that remembers the last folder location. When a user drags and drops a folder, the application stores this location. Subsequently, if the user clicks the 'Select folder' button, the file dialog opens at the previously selected folder location.
Also, added tkinterdnd2 to the requirements.txt file
Thanks a lot for your pull request! I'll review it, might take me a bit of time, but I'll try to do my best!
Seems pretty good! Thanks for the PR, however, I feel like the change in formatting makes it pretty hard to understand and see the code in many places, and I would like to mostly revert to the old one. I do agree that it looks better in some places, but I feel like it's just taking unnecessary space a lot of the time.
Some examples of things I consider weird are:
- Ending the last parameter of a function with a comma
,(ex:ttk.Label(a, b, c, d,))- Using parenthesis around the value of a variable, making it look like the value is a
tuple. An example of this would beGITHUB_LATESTVERSIONJSON- The line breaks around parenthesis, not really needed when a single parameter is used, and ends up taking a lot of space
- The double space for comments on single lines (ex:
RETRY_DELAY = 15 #instead ofRETRY_DELAY = 15 #) - might be a standard but I don't really get itThis new formatting made it a bit hard to review the changes, since many lines were affected.
Another important point is that some comments have been removed, while some others have been added. The added ones are a nice addition (thanks for this!), but I feel like the old ones are also nice to keep because some tkinter stuff isn't easy to remember, especially for me. Those comments help me remember and understand the syntax of some functions. This happens in
handle_folder_selection
last_dropped_folderis never saved anywhere, the app will only remember it as long as it stays open. Might be a good idea to put it in the config, I'll do it if you want to.I'll have to further review the use of
reset_uias it seems only used byhandle_folder_selectionand might be a confusing function name if it shouldn't be used in all cases. Seems correct though.Apart from that, really good changes! I still have to test the code, and review
tkinterdnd2, I'm a bit worried it might be a bit out of dateIf you have a bit of time, I would be thankful if you could make these changes. Else, I'll try to find a bit of time to do that myself. Sadly I'm a bit busy currently, but I'll try not to take too long.
I agree. When I first was reviewing the changes, I noticed there were over 500 or so more lines of code. Which I thought was really odd as I essentially only added handle_folder_selection and reset_ui and a few other things, so I tried to fix the formatting and it did help a little. But I may have to disable the formatter I'm using. I will mess around with it and get it as close as possible to the original formatting. It does make it hard to review changes. Regarding the other things you mentioned, I will take a look at them and fix them. Will make a commit sometime today. Thanks for the feedback! Regarding tkinterdnd2 being old, I also was a bit worried about that, so I did look into a newer package called tkinterdnd2-universal made in 2023 but need to do more research on it. But I will say, you should check out the changes, it works pretty well, haven't noticed any problems.
@BigBoiCJ Fixed formatting! and regarding your statements:
last_dropped_folder is never saved anywhere, the app will only remember it as long as it stays open. Might be a good idea to put it in the config, I'll do it if you want to.
Going to look into this, will have this in my next commit.
I'll have to further review the use of reset_ui as it seems only used by handle_folder_selection and might be a confusing function name if it shouldn't be used in all cases. Seems correct though.
Will also fix this soon.
@BigBoiCJ Made recent commit! renamed reset_ui to reset_folder_selection_ui and added it to the handle_folder_selection scope. Also added last_dropped_folder to the config. It's working great. When you close the program, it automatically remembers the last folder you were at! This works for both dragging the folder and selecting a folder, it will remember regardless. Take a look when you have the time it's much easier to see the changes with the fixed formatting, I believe it's ready!
After testing, seems to work great!
It might be better to save the parent directory instead of the last selected directory instead. Seems more logical.
I would also change last_dropped_folder to last_selected_folder
It also seems like windows correctly fixes when filedialog.askdirectory(initialdir=initial_dir) points to an invalid or non-existing location, such as a deleted folder, but it might be good to handle that directly in the code, through os.path.isdir for example.
Hey, are you going to merge? Or would you like me to do the minor things you pointed out earlier? Let me know. Would love to see this in the main.
Hey, are you going to merge? Or would you like me to do the minor things you pointed out earlier? Let me know. Would love to see this in the main.
Sorry for taking so long! I've been pretty busy with life once again. I'll do those changes and commit
I'm still not sure if I should push this as a stable release directly since the main tkinter library got changed
Massive thanks for your PR! I've merged it and it'll be featured in the next release that should be up in a few minutes :)
tkinterdnd2 seems to be maintained again, so that's great news!
Seems like github doesn't want to display you as a contributor (https://github.com/BigBoiCJ/SteamAutoCracker/graphs/contributors) when I rebase & merge, which sucks because it is my preferred way to merge PRs.
I wonder if I should add a Contributors section in the readme to give more visibility to some of you, because you could get lost in release notes after some time.