obsproject
UI: Add Manage Scene Collections dialog
Description
Adds a Scene Collection Manager dialog and moves the scene collection menu to the scenes dock.
Screenshots as of: September 8th, 2023
This solves three major UX problems with current scene collection management:
- The place where you'd manage scene collections was visually disconnected from the scene tree
- All actions were in a context menu, making them a bit cumbersome
- You could only act on the collection you're currently on. You couldn't rename, export, delete, or duplicate any other collection without switching to it first.
The new dialog solves these issues. Scene collection that aren't active can now be renamed, duplicated, exported, or deleted. However, the currently active scene collection can no longer be deleted as that would then require switching to some other collection, which can be a bit unpredictable to the user.
It also adds the ability to perform bulk actions on scene collections. This means that it's possible to duplicate, export, or delete multiple scene collections at once. The collections shown in the menu is now capped at the 10 most recent ones to avoid unreasonably large context menus, other collections can still be switched to by double-clicking them in the dialog.
Driven by a design first approach, based on this mockup. Thanks to @GeorgesStavracas for creating the mockup and to @Warchamp7 for his assistance.
This PR utilizes new widgets by @derrod. It will stay draft until #8447 is merged. However, reviews are still very much desired.
Motivation and Context
See the problems mentioned above.
How Has This Been Tested?
macOS 13.2.1
Tested all new features. This includes:
- Opening the importer and importing
- Creating a new collection
- Renaming a collection (both current and not-current)
- Duplicating a collection (both current and not-current)
- Exporting a collection (both current and not-current) and making sure it's still sorted and pretty-printed (see #8407)
- Deleting a not-current collection
- Making sure the "Delete" field is greyed out and that the collection doesn't get deleted even if it isn't (by temporarily changing other code)
- Exporting multiple collections and making sure potential existing collections in the target folder aren't deleted
- Duplicating multiple collections
- Deleting multiple collections
- Switching scene collections via the menu in the scenes dock
- Switching scene collections by double-clicking a scene collection in the dialog
- Sorting works both by last used and alphabetically by name, and is remembered even after the dialog or OBS altogether is closed
Types of changes
- New feature (non-breaking change which adds functionality)
Checklist:
- [x] My code has been run through clang-format.
- [x] I have read the contributing document.
- [x] My code is not on the master branch.
- [x] The code has been tested.
- [x] All commit messages are properly formatted and commits squashed where appropriate.
- [x] I have included updates to all appropriate documentation.
Added Seeking Testers and re-triggered CI to get downloadable artifacts for easier testing. Edit: This will need rebasing before artifacts actually get generated.
Rebased on the updated #8447.
New screenshot:
With the updated base branch, this should now actually have downloadable build artifacts once CI finishes.
Noting down feedback I left on Discord
Having the scene collection dropdown in the bottom toolbar was the proposed design but don't feel it works in practice. If it's possible to instead have this as the dock titlebar itself, that would be great.
Since we allow the Scenes dock to be hidden, we should probably keep the Scene Collection menu in the window top bar for accessibility reasons. Same entries as the dropdown menu.
https://cdn.discordapp.com/attachments/940868766345084928/1130713502407991406/obs64_5lTW7Kg94K.gif
Left border disappears when hovering (Suspected issue with the widgetry itself in #8447)
We should add Open to the top of this list
The padding to the left of the collection names is too large right now (Approximately 80px~), it should be reduced to match the padding on the right of the row (Approximately 28px~)
When the checkboxes are visible, they should have the 28px~ on the left and right
Examples:
We should add Open to the top of this list
Added an "Open" menu entry to the menu.
The padding to the left of the collection names is too large right now (Approximately 80px~), it should be reduced to match the padding on the right of the row (Approximately 28px~)
I suspect this might be an issue with the widgetry as well, but will have to take a closer look.
Having the scene collection dropdown in the bottom toolbar was the proposed design but don't feel it works in practice. If it's possible to instead have this as the dock titlebar itself, that would be great.
As noted off-thread, this requires further investigation on dock titles. I'll take a look later this week, probably.
Having the scene collection dropdown in the bottom toolbar was the proposed design but don't feel it works in practice. If it's possible to instead have this as the dock titlebar itself, that would be great. Since we allow the Scenes dock to be hidden, we should probably keep the Scene Collection menu in the window top bar for accessibility reasons. Same entries as the dropdown menu.
@Warchamp7 Added back the Scene Collection menu to the menu bar. As noted off-thread, adding the dropdown to the title bar itself involves work much beyond the scope of this PR. For the concern of crowding the toolbar, I removed the name of the scene collection from the button. Before completely removing the button and the associated code changes from this PR, I'd like to have an explicit "please remove the button" from you or @GeorgesStavracas if it's still too crowed (as adding it back in at a later stage would be quite a bit of work fighting either Qt or git).
New screenshots (also updated PR main description):
Adjusted the margins to match better.
As suspected this involves changes to the widgetry (and I believe is not fixable without them), so I'd definitely want @derrod's opinion on what I'm doing here
That seems fine to me, I can incorporate those changes in the widgets PR itself.
@Warchamp7 addressed part of the feedback. Will investigate the other things soon :tm:. Please see the last question.
Final verdict on the toolbar button is to remove it
Gone
Last Used time doesn't work yet, always says Just now for all collections
Will still have to debug this. Works fine on macOS, this is probably operating system related.
Let's also have the current collection list Currently Open for the last used time
Done
Left border indicator is missing from the currently active collection (Even on the Grey version of Yami now)
This seems to have gone lost in the Idian PR. Maybe I'll just add it back here.
When filtering the collection list, there are gaps in the list where the non-matches are
Will investigate why this happens.
Let's move the multi-select button to the left side of the search bar
Done
In multi-selection mode, there's no indicator of the currently active scene collection
I think this is also the problem with the left border indicator having gone missing - or are you also looking for the checkmark-thing on the right?
Right now if you select the current collection in multi-select mode, we disable the Delete button and it's not immediately clear as to why. Instead we should leave the Delete button enabled and display an error when they try to Delete with the current collection selected.
Would it maybe make sense to display such a message somewhere in the window and leave the button disabled? It feels a bit wrong to me to have an enabled button only for that button to be disabled which the user will only know when they click it.
Would it maybe make sense to display such a message somewhere in the window and leave the button disabled? It feels a bit wrong to me to have an enabled button only for that button to be disabled which the user will only know when they click it.
Pulling in off-thread discussion on this below:
If the only action here was Delete, I would agree with disabling the button and displaying a notice. Since there's multiple possible actions in Export/Duplicate/Delete, an error notice makes less sense. If a user is only trying to export a few collections, it doesn't make sense to warn them they can't delete the selection.
Similarly if it was only possible to duplicate one collection at a time, disabling Duplicate upon making two or more selections would be fine, but because Deletion is selection dependent I don't think it's appropriate to take the same approach
If they've ONLY selected the current collection, that is fine to disable the Delete button
As soon as two or more are selected, it gets murky, so I'd rather enable the button and then inform the user their selection is invalid when trying to perform the Delete action
Rebased on latest Idian branch. There are a few TODOs still left which I will fix later, plus a conflict to current master but that would require me to rebase the Idian branch as well.