-
-
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
refactor(ui): break down config.html into smaller pieces #2491
refactor(ui): break down config.html into smaller pieces #2491
Conversation
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.
Thanks for the PR! This is just my first pass review. I still need to review the bulk of the new files.
4d3ffb6
to
3aa823c
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.
I did not review every line in the new vue files. Hopefully things are in the same order as before and functionally equivalent, but once these couple of comments are addressed I will test the build and do a spot check.
src_assets/common/assets/web/configs/tabs/audiovideo/NewDisplayOutputSelector.vue
Outdated
Show resolved
Hide resolved
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## nightly #2491 +/- ##
=========================================
Coverage 6.94% 6.94%
=========================================
Files 87 87
Lines 17673 17673
Branches 8399 8399
=========================================
Hits 1227 1227
- Misses 13717 15761 +2044
+ Partials 2729 685 -2044
Flags with carried forward coverage won't be shown. Click here to find out 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 tested your changes locally, and visually everything appears to be in place.
These will be my last changes to request.
src_assets/common/assets/web/configs/tabs/encoders/AmdAmfEncoder.vue
Outdated
Show resolved
Hide resolved
src_assets/common/assets/web/configs/tabs/encoders/IntelQuickSyncEncoder.vue
Outdated
Show resolved
Hide resolved
src_assets/common/assets/web/configs/tabs/encoders/NvidiaNvencEncoder.vue
Outdated
Show resolved
Hide resolved
src_assets/common/assets/web/configs/tabs/encoders/SoftwareEncoder.vue
Outdated
Show resolved
Hide resolved
src_assets/common/assets/web/configs/tabs/encoders/VideotoolboxEncoder.vue
Outdated
Show resolved
Hide resolved
@ReenigneArcher if any of the current open PR gets conflict with my changes, you can point to me for help, if needed. |
Breaks that big chungus of a vue file into smaller readable vue files, Also includes some QoL improvements, `$tp` for i18n `$t` but knows platforms, `PlatformLayout` for slotted platform layout, making it clearer what should load per platform and easier to add new as needed.
Co-authored-by: ReenigneArcher <42013603+ReenigneArcher@users.noreply.github.com>
Co-authored-by: ReenigneArcher <42013603+ReenigneArcher@users.noreply.github.com>
Co-authored-by: ReenigneArcher <42013603+ReenigneArcher@users.noreply.github.com>
Co-authored-by: ReenigneArcher <42013603+ReenigneArcher@users.noreply.github.com>
b810b45
to
92dfaa4
Compare
…2491) Co-authored-by: ReenigneArcher <42013603+ReenigneArcher@users.noreply.github.com>
Description
Breaks that big chungus of a vue file into smaller readable vue files, also includes some QoL improvements,
$tp
for i18n$t
but knows platforms,PlatformLayout
for slotted platform layout, making it clearer what should load per platform and easier to add new as needed, refactored some duplicated layouts using those new tools.configs/tabs/audiovideo/NewDisplayOutputSelector.vue
will be used in feat(ui): list available displays withselect
#2490configs/tabs/audiovideo/DisplayDeviceOptions.vue
was created as boilerplate so Add new API to modify display devices for Windows #2032 can use to place the new div with their changesWe can improve further by pushing more logic into each Component, instead of parsing and filtering data in
config.html
, but I wanted to avoid refactoring the load/save part in this PR.PS: Fixed
_common.*_cmd
labels in General Tab.Screenshot
No visual changes.
Type of Change
Checklist
Branch Updates
LizardByte requires that branches be up-to-date before merging. This means that after any PR is merged, this branch
must be updated before it can be merged. You must also
Allow edits from maintainers.