-
-
Notifications
You must be signed in to change notification settings - Fork 9k
frontend: Replace add source dropdown with dialog #12067
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
Conversation
0c8caa9 to
370391e
Compare
|
Testing on Ubuntu 24.04,
one visual bug : |
4c4c9e7 to
f08d92b
Compare
80e620b to
2501fb3
Compare
08da97b to
5bf01aa
Compare
3e0730e to
abddce3
Compare
348d8f2 to
b738ffd
Compare
|
This has been updated with a bunch of cleanup and optimizations by @Lain-B. It is ready for proper testing and review now |
b738ffd to
4d25de7
Compare
PatTheMav
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First very superficial review, the Singleton pattern in the ThumbnailManager is an issue and appears at first glance like a responsibility inversion, will have to analyse this more deeply first.
There a few Qt-isms and C-isms that should use modern C++ style instead.
fb6ec34 to
b5e91c7
Compare
5c8532e to
6bea4f1
Compare
|
Fixed the above visual issues
I toyed with this idea, but since the items are multi-selectable and can be unselected by clicking, double clicking was a bit confusing.
I aimed to distinguish it with the lack of an icon. Adding a separator is also non-trivial due to how simplistic QListWidgets are heh |
Can confirm, also did a round of all themes to check for other visual issues, found none.
While I agree it is a visual distinction, it does, in my opinion, make it stand out less, as the eye is naturally drawn to the icons as "markers" of the list. Without the presence of such a marker, the "Recently added" does not impart that it is a list item at first sight.
Would it be possible to make it not be the same list but in a separate one(that's what I had in mind in the mockup), or not a list item at all, but still behave like one? |
RytoEX
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sure that new code complies with #12567.
daab453 to
8dac818
Compare
770713b to
d89948c
Compare
ac2c819 to
04997b6
Compare
cb2ed0d to
e3b1a73
Compare
0efd472 to
515cd04
Compare
RytoEX
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nits.
Co-Authored-By: Lain <134130700+Lain-B@users.noreply.github.com>
515cd04 to
0fe8ec5
Compare







Description
Replaces the add source context menu with a brand new window.
Adds new FlowFrame and FlowLayout for automatic wrapping widget frame
Adds ThumbnailManager class
Adds new dialog for adding sources
Sources Dock Menu
New Window
obs64_E0SOtuhy93.mp4
Motivation and Context
The current process for adding a source is rather cumbersome, especially when adding a source that already exists. Ideally users should be re-using Sources whenever it makes sense, so one of the goals of this PR is exposing that better than the old radio button and list selection.
How Has This Been Tested?
Created a number of different sources.
Types of changes
Checklist: