add list of libraries into sidebar #151

yedpodtrzitko · 2024-05-09T18:11:23Z

superseding #127

show list of previously opened libraries on bottom of sidebar:
add option (default True) to decide if previous library should be opened on app start
there's a minor refactoring to keep working with variables localized. So instead of this:

root_layout = QHBoxLayout(self)
# -50 various lines of code-
root_layout.setContentsMargins(0, 0, 0, 0)
# -more unrelated code-
root_layout.addWidget(splitter)

...working with the variable is now moved into the same place.

CyanVoxel · 2024-05-09T22:44:43Z

I like the ability to toggle the automatic library loading on start. I saw in #127 you said you motivation for this was the thumbnail threads spooling up, which is more of an issue with the current lack of thumbnail caching than anything else. That being said, there will always be some resources being used when opening a library and having the choice to toggle that off is valuable regardless.

While I largely agree with @Loran425 about the approach of having an "Open Recent Libraries" menu over the idea of having a persistent panel on screen, I can see the desire to be able to switch between libraries faster. As long as it's able to be hidden for users who don't want it, I don't see the harm in it. With that being said though, I can't seem to get it to even appear on my end:

In addition, the splash screen seems to be broken for me now, which could be related to the changes in ts_qt.py or could just be some Qt resource shenanigans.

yedpodtrzitko · 2024-05-11T14:59:59Z

While I largely agree with @Loran425 about the approach of having an "Open Recent Libraries" menu over the idea of having a persistent panel on screen, I can see the desire to be able to switch between libraries faster. As long as it's able to be hidden for users who don't want it, I don't see the harm in it.

that's not configurable via the following menu item:

With that being said though, I can't seem to get it to even appear on my end:

That should be fixed now. It got lost when checking in new commits. Thanks for catching that.

In addition, the splash screen seems to be broken for me now, which could be related to the changes in ts_qt.py or could just be some Qt resource shenanigans.

I never see the splash screen (even before), so I cant test it, but I made a speculative change which could fix it.

Let me know if it works for you now as expected, thanks.

CyanVoxel · 2024-05-12T22:06:31Z

Currently there's an issue on Windows where the path separators aren't normalized list the library list, resulting in duplicate libraries showing up in the list:

Normally I would recommend wrapping the paths in os.path.normpath(), however #156 is trying to remove the use of os.path in favor of the pathlib library.

Also, the splash screen is still broken for me but I'll see if I can make changes to get that functional on at least my end.

CyanVoxel · 2024-05-12T22:07:10Z

tagstudio/src/qt/widgets/preview_panel.py

+            button.setStyleSheet(
+                "background-color: #ffffff; color: #000000; text-align: left; padding-left: 10px;"
+            )


Proposed style changes to the library section to better match the look and feel of the rest of the program (screenshot includes changes to the section title not featured in the changes below)

Suggested change

button.setStyleSheet(

"background-color: #ffffff; color: #000000; text-align: left; padding-left: 10px;"

)

button.setStyleSheet(

"QPushButton{"

"background-color:#65000000;"

"border-radius:6px;"

"text-align: left;"

"padding-top: 3px;"

"padding-left: 6px;"

"padding-bottom: 4px;"

"}"

"QPushButton::hover{background-color:#65AAAAAA;}"

"QPushButton::pressed{background-color:#65EEEEEE;}"

)

button.setCursor(Qt.CursorShape.PointingHandCursor)

CyanVoxel · 2024-05-12T22:12:18Z

tagstudio/src/qt/widgets/preview_panel.py

+            # remove from GUI
+            widget.setParent(None)
+
+        label = QLabel("Select a library")


Suggestion to rename this section to "Recent Libraries". I feel this is more descriptive of the section, and is more consistent with similar "File -> Open Recent Thing" functionality found in other programs, and likely this program in the future.

Suggested change

label = QLabel("Select a library")

label = QLabel("Recent Libraries")

label.setStyleSheet("font-weight:bold;")

CyanVoxel · 2024-05-12T22:18:21Z

tagstudio/src/qt/ts_qt.py

@@ -416,6 +431,20 @@ def start(self):
        macros_menu.addAction(self.sort_fields_action)

        folders_to_tags_action = QAction("Create Tags From Folders", menu_bar)
+        show_libs_list_action = QAction("Show Libraries List", menu_bar)


Similar suggestion to the main "Recent Libraries" one, just changing the menu action to match.

Suggested change

show_libs_list_action = QAction("Show Libraries List", menu_bar)

show_libs_list_action = QAction("Show Recent Libraries", menu_bar)

CyanVoxel · 2024-05-12T22:36:57Z

I'll be honest, I was skeptical at first of whether I would personally use this list vs just having it tucked away in the File menu, but as soon as I actually used it I didn't want to live without it 😁 Fantastic work on this so far!

Other than the issues and styling changes commented above, I wanted to suggest a few other changes/additions. These tie into the idea of this being more of a "Recent Libraries" list rather than a list of all libraries you've historically opened.

Proposed Changes/Additions

Automatic reordering of libraries based on the order of last opening. For example if you were to click on a library in the middle of the list to open it, then it would move to the top.
Right-click context menu option to remove a library from the list.
A limit for how many libraries show up in this list. I think a hardcoded limit of 5 would work well, then once we get a proper settings panel then this can be user-configurable.

Thank you again for all your work on this!

CyanVoxel · 2024-05-13T04:31:37Z

tagstudio/src/qt/ts_qt.py

@@ -90,7 +92,7 @@
    FixDupeFilesModal,
    FoldersToTagsModal,
 )
-import src.qt.resources_rc


Removing this import is what causes in the splash screen to break, and is currently the pinpoint of a merge conflict

Ah so it's not unused import, but it has side-effect of executing code. Not a good thing to do, but looks like there's no way around it. I'll add comment so it's obvious why is it here.

yedpodtrzitko · 2024-05-13T15:46:29Z

Currently there's an issue on Windows where the path separators aren't normalized list the library list, resulting in duplicate libraries showing up in the list:

I'm afraid I dont have any windows machine around here, so I cant reproduce this issue. I just hope the Pathlib PR will solve this automagically.

yedpodtrzitko · 2024-05-13T15:52:36Z

Right-click context menu option to remove a library from the list.

I was thinking to implement this in future, but rather than using the context menu, I was considering to add there another button, the same way as Tags have it (➖ ), or Fields have it (🗑️ ), so the design language is somehow consistent.

CyanVoxel · 2024-05-13T23:24:21Z

Right-click context menu option to remove a library from the list.

I was thinking to implement this in future, but rather than using the context menu, I was considering to add there another button, the same way as Tags have it (➖ ), or Fields have it (🗑️ ), so the design language is somehow consistent.

Makes sense to me, I thing the (➖) would be the better route.

CyanVoxel · 2024-05-13T23:30:42Z

Not sure if you're running into this issue as well, but when using the refactored color variable enums:

scroll_area.setStyleSheet(
            "QWidget#entryScrollContainer{"
            f"background: {Theme.COLOR_BG};"
            "border-radius:6px;"
            "}"
        )

Qt throws an error trying to read from the stylesheets because it's using the literal name "Theme.COLOR_BG" as the color value. If I change these instances to Theme.COLOR_BG.value then it works as intended.

CyanVoxel · 2024-05-13T23:53:49Z

tagstudio/src/qt/ts_qt.py

+        all_libs = {current_time: path}
+
+        for item_key in self.settings.allKeys():
+            item_path = self.settings.value(item_key)
+            if item_path != path:
+                all_libs[item_key] = item_path


This fixes the inconsistent path separator bug while keeping in line with #156. Not really a fan of the double casting, but it works. I've also tested this after manually modifying the .ini to use different separators, and it all loads and displays as you'd expect - on Windows at least.

Suggested change

all_libs = {current_time: path}

for item_key in self.settings.allKeys():

item_path = self.settings.value(item_key)

if item_path != path:

all_libs[item_key] = item_path

all_libs = {current_time: str(Path(path))}

for item_key in self.settings.allKeys():

item_path = Path(self.settings.value(item_key))

if item_path != Path(path):

all_libs[item_key] = str(item_path)

I achieved this with fewer casting, hopefully it will still work as expected.

Paths with non-native separators are no longer normalized when loading from the .ini, but honestly this is probably a niche use case anyway and doesn't impact the actual functionality of the program. It's just a small visual quirk at this point. As long as nobody complains, I'm okay with the reduced number of casts.

yedpodtrzitko · 2024-05-14T01:47:27Z

Not sure if you're running into this issue as well, but when using the refactored color variable enums:
scroll_area.setStyleSheet(
            "QWidget#entryScrollContainer{"
            f"background: {Theme.COLOR_BG};"
            "border-radius:6px;"
            "}"
        )
Qt throws an error trying to read from the stylesheets because it's using the literal name "Theme.COLOR_BG" as the color value. If I change these instances to Theme.COLOR_BG.value then it works as intended.

No issue on my side, but adding the .value explicitly wont hurt.

CyanVoxel

Looks great!! I attempted to find a way to make the library splitter section only take up the minimum amount of space it needs instead of letting it grow along the title label, but couldn't find anything quickly that would do it. Given that it would only be a minor visual enhancement I won't worry about it too much right now. If you or I can't find a solution to it soon then perhaps opening up an issue for it after the fact would be best. After all, I think we'd both rather move onto more important fixes and additions than spend too much time fighting with Qt ;)

yedpodtrzitko · 2024-05-14T06:59:42Z

Looks great!! I attempted to find a way to make the library splitter section only take up the minimum amount of space it needs instead of letting it grow along the title label, but couldn't find anything quickly that would do it. Given that it would only be a minor visual enhancement I won't worry about it too much right now. If you or I can't find a solution to it soon then perhaps opening up an issue for it after the fact would be best. After all, I think we'd both rather move onto more important fixes and additions than spend too much time fighting with Qt ;)

seems like that got lost during rebasing and I didnt check after last push.
It should be fixed now by button_remove.setFixedWidth(30)

CyanVoxel · 2024-05-14T07:20:28Z

seems like that got lost during rebasing and I didnt check after last push. It should be fixed now by button_remove.setFixedWidth(30)

The button_remove width issue wasn't actually present until I pulled just now to see what fix you were talking about. I was referring to the excess space that can surround the "Recent Libraries" label instead of having the maximum height of the section be equal to it's minimum height at any given time.

yedpodtrzitko · 2024-05-14T07:53:19Z

The button_remove width issue wasn't actually present until I pulled just now to see what fix you were talking about. I was referring to the excess space that can surround the "Recent Libraries" label instead of having the maximum height of the section be equal to it's minimum height at any given time.

ok I see what you mean now. Check the behaviour after the latest commit I pushed.

CyanVoxel · 2024-05-14T19:06:29Z

ok I see what you mean now. Check the behaviour after the latest commit I pushed.

Awesome!! Works exactly how I'd expect! Every now and again it won't update the size when when I remove a library, but I can't predictably reproduce it. Overall though I'm more than happy to go ahead and pull this. Thank you again for all your hard work!

CyanVoxel added enhancement New feature or request UI/UX User interface and/or user experience labels May 9, 2024

yedpodtrzitko force-pushed the yed/libs-sidebar branch from 72672b5 to 80b58ba Compare May 11, 2024 14:52

CyanVoxel reviewed May 12, 2024

View reviewed changes

CyanVoxel reviewed May 13, 2024

View reviewed changes

yedpodtrzitko force-pushed the yed/libs-sidebar branch 2 times, most recently from 10a09af to fcb23d3 Compare May 13, 2024 16:05

yedpodtrzitko requested a review from CyanVoxel May 13, 2024 16:06

CyanVoxel added this to the Alpha 9.3.x milestone May 13, 2024

CyanVoxel reviewed May 13, 2024

View reviewed changes

yedpodtrzitko force-pushed the yed/libs-sidebar branch 2 times, most recently from 402fc54 to 3c40c8a Compare May 14, 2024 04:27

yedpodtrzitko added 2 commits May 14, 2024 12:37

add list of libraries into sidebar

94a0b00

CR feedback

06f528f

yedpodtrzitko force-pushed the yed/libs-sidebar branch 4 times, most recently from c499902 to 46ae3a6 Compare May 14, 2024 04:47

add button for removing recent lib

a71ed7c

yedpodtrzitko force-pushed the yed/libs-sidebar branch from 46ae3a6 to a71ed7c Compare May 14, 2024 04:52

yedpodtrzitko requested a review from CyanVoxel May 14, 2024 04:54

CyanVoxel approved these changes May 14, 2024

View reviewed changes

keep remove button small

f60a93f

dont expand recent libs

5d21375

CyanVoxel merged commit 5d85417 into TagStudioDev:main May 14, 2024
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add list of libraries into sidebar #151

add list of libraries into sidebar #151

yedpodtrzitko commented May 9, 2024

CyanVoxel commented May 9, 2024

yedpodtrzitko commented May 11, 2024

CyanVoxel commented May 12, 2024

CyanVoxel May 12, 2024

CyanVoxel May 12, 2024

CyanVoxel May 12, 2024

CyanVoxel commented May 12, 2024

CyanVoxel May 13, 2024

yedpodtrzitko May 13, 2024

yedpodtrzitko commented May 13, 2024 •

edited

yedpodtrzitko commented May 13, 2024

CyanVoxel commented May 13, 2024

CyanVoxel commented May 13, 2024

CyanVoxel May 13, 2024

yedpodtrzitko May 14, 2024

CyanVoxel May 14, 2024

yedpodtrzitko commented May 14, 2024

CyanVoxel left a comment

yedpodtrzitko commented May 14, 2024

CyanVoxel commented May 14, 2024

yedpodtrzitko commented May 14, 2024

CyanVoxel commented May 14, 2024

-            button.setStyleSheet(
-                "background-color: #ffffff; color: #000000; text-align: left; padding-left: 10px;"
-            )
+            button.setStyleSheet(
+                "QPushButton{"
+                "background-color:#65000000;"
+                "border-radius:6px;"
+                "text-align: left;"
+                "padding-top: 3px;"
+                "padding-left: 6px;"
+                "padding-bottom: 4px;"
+                "}"
+                "QPushButton::hover{background-color:#65AAAAAA;}"
+                "QPushButton::pressed{background-color:#65EEEEEE;}"
+            )
+            button.setCursor(Qt.CursorShape.PointingHandCursor)

	label = QLabel("Select a library")
	label = QLabel("Recent Libraries")
	label.setStyleSheet("font-weight:bold;")

	show_libs_list_action = QAction("Show Libraries List", menu_bar)
	show_libs_list_action = QAction("Show Recent Libraries", menu_bar)

add list of libraries into sidebar #151

add list of libraries into sidebar #151

Conversation

yedpodtrzitko commented May 9, 2024

CyanVoxel commented May 9, 2024

yedpodtrzitko commented May 11, 2024

CyanVoxel commented May 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CyanVoxel commented May 12, 2024

Proposed Changes/Additions

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yedpodtrzitko commented May 13, 2024 • edited

yedpodtrzitko commented May 13, 2024

CyanVoxel commented May 13, 2024

CyanVoxel commented May 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yedpodtrzitko commented May 14, 2024

CyanVoxel left a comment

Choose a reason for hiding this comment

yedpodtrzitko commented May 14, 2024

CyanVoxel commented May 14, 2024

yedpodtrzitko commented May 14, 2024

CyanVoxel commented May 14, 2024

yedpodtrzitko commented May 13, 2024 •

edited