TagStudioDev
Only show 'Close Library' if your in a library
This is my first commit like ever on anything I know I made a lot of changes but I don't think this should have any effect outside of just only showing the close button when inside a library!
I would suggest also hiding the "Refresh Directories", "Save Library" and "Save Library Backup" Buttons
I honestly did not even think of that but this hides all the buttons in the "files" menu that do not have any effect until you open a library I will look to see if there is anything else that might benefit from being hidden til you open a library.
A better way to approach this in my opinion would be to simply disable the menu options instead of removing them entirely, for example how the Edit menu handles this:
This way the options don't change locations making them more difficult to find based on the state of the library, plus the user is made aware of all options whether they're presently available or not.
Before Opening a Library
After
I looked at how the items were disabled in the version you showed (Alpha 9.4.0) and decided to add the "update_clipboard_actions" function over. Not the code from it since none of that is added yet just the function its self. This means that all the buttons will be enabled at first but since its the same function the ones that need to be disabled will be checked then disabled in the same code so its not a problem.
I woke up to a lot of merges and this no longer working so ill have a look at what broke it and fix this pr lol
@CyanVoxel Actually yeah! I'll have a look and see if I can't update the code to work.
A very minor nit, but the function name calls the toolbar a clipboard, which usually refers to copy/paste. I feel update_menu_actions or update_toolbar_actions would be a more proper name. Thanks!
- It's not enabling the actions when i close the library and reopen a library. to fix it call
update_clipboard_actions()from theQtDriver.open_library()method so it will enable actions when opening a library.- format with
ruffand do mypy check. (Installpre-commitif you haven’t yet.)
- why are there two options for enabling and disabling? what if there is only one option named
toggle_action_statethat stores a bool value? if it'sTrue, the action should be enabled or disabled and ifFalse, it should not.- I prefer storing actions and/or menus (only if all the menu items are should be toggled) that need to be toggled when opening and closing the library, rather than enabling everything and explicitly disabling unwanted actions with the
data()option.btw, i personally like using
QAction.property()rather thanQAction.data()for its simplicity in this case.
-
weird that issue was not happening for me but i added it there hopefully that will eliminate that bug for anyone else who might be getting it.
-
I will do that ruff thing now.
-
The reason I have 2 separate data options is because they have different behaviors. Autofill needs to be disabled on launch but not kept enabled when closing a library so if I just used 1 with true meaning it should be enabled/disabled then autofill would be kept open outside of libraries for example. I could just have 1 data variable for enable / disable and make it true if you want to keep it enabled after close and mark it false if you want to keep it disabled after open but i felt like that was overly complex and would just lead to a lot of confusion. This is just more explicit. I will look into property() vs data() though and make that change if it fits thank you for the recommendation. If i miss understood your point here about enable/disable let me know.
A very minor nit, but the function name calls the toolbar a clipboard, which usually refers to copy/paste. I feel update_menu_actions or update_toolbar_actions would be a more proper name. Thanks!
This is a good idea. I think i only used update_clipboard_actions name because when i started this it was already a method but not sure. anyways now that the method is just this code I changed it to update_menu_actions.
3. If i miss understood your point here about enable/disable let me know.
i meant only set the data if it’s a library related action. then enable or disable only those actions. alternatively you could just create a list of library related actions and toggle all of them together. it’d be simpler. no need to bother with any properties.
i meant only set the data if it’s a library related action. then enable or disable only those actions. alternatively you could just create a list of library related actions and toggle all of them together. it’d be simpler. no need to bother with any properties.
I feel like 1 lists with properties would be more efficient then 2 lists. and on the off chance that something would need to be both enabled and disabled idk if that is needed ever but this current code would account for this.
