-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
feat(display): Configure display device based on user config #3441
base: master
Are you sure you want to change the base?
Conversation
6daf175
to
3f37821
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3441 +/- ##
==========================================
+ Coverage 11.17% 11.83% +0.66%
==========================================
Files 100 100
Lines 17310 17621 +311
Branches 8069 8220 +151
==========================================
+ Hits 1934 2086 +152
+ Misses 12812 12780 -32
- Partials 2564 2755 +191
Flags with carried forward coverage won't be shown. Click here to find out more.
|
fad840c
to
e3d6659
Compare
e3d6659
to
2215927
Compare
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.
Thank you for the PR!
This is a first pass review for which I only looked at the html/vue, and docs.
docs/configuration.md
Outdated
<tr> | ||
<td>Example</td> | ||
<td colspan="2">@code{} | ||
dd_wa_hdr_toggle = true |
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.
dd_wa_hdr_toggle = true | |
dd_wa_hdr_toggle = enabled |
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.
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.
I really like the idea of check boxes, but currently we aren't using those anywhere else. So I think that should come in a separate PR changing them universally from the select menus.
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.
I really like the idea of check boxes, but currently we aren't using those anywhere else. So I think that should come in a separate PR changing them universally from the select menus.
I have fixed this issue already. The enabled
works with them like a charm. I hope that I don't need to change the checkbox to the menu (your remark leaves it open for interpretation)?
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.
It would be quite weird to have one checkbox in the ui, while everywhere else is a select menu.
I know you don't like to open more than one PR at once, but changing them out globally would be better. If a separate PR is created to change that pattern, then it's fine to use it here as well.
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.
I'll create a separate PR then
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.
I'll prioritize reviewing and merging it whenever it's ready. Thanks!
src_assets/common/assets/web/configs/tabs/audiovideo/DisplayDeviceOptions.vue
Outdated
Show resolved
Hide resolved
Quality Gate failedFailed conditions See analysis details on SonarQube Cloud Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE |
Description
This is a part 2 of #2894.
This PR adds support for configuring display via the display device library from the previous commit.
This is completely optional feature, which can be disabled with a single config switch.
This includes:
The changes are best understood from screenshots below.
Screenshot
New advanced settings panel:
Device config options:
Resolution drop-down 1:
Resolution drop-down 2:
Refresh rate drop-down 1:
Refresh rate drop-down 2:
HDR drop-down:
The "hard reset" option in case the previous display config can no longer be restored (display no longer available/firmware was updated so the device id has changed/etc.):
Type of Change
.github/...
)Checklist