-
Notifications
You must be signed in to change notification settings - Fork 3.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
Added automatic saving for settings and persistent storage for image filter settings #9032
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #9032 +/- ##
========================================
Coverage 73.95% 73.95%
========================================
Files 428 428
Lines 44532 44582 +50
Branches 3881 3890 +9
========================================
+ Hits 32933 32972 +39
- Misses 11599 11610 +11
|
tests/cypress/e2e/actions_tasks/case_68_saving_settings_local_storage.js
Show resolved
Hide resolved
cvat-ui/src/components/header/settings-modal/shortcut-settings.tsx
Outdated
Show resolved
Hide resolved
cvat-ui/src/components/header/settings-modal/settings-modal.tsx
Outdated
Show resolved
Hide resolved
cvat-ui/src/components/header/settings-modal/settings-modal.tsx
Outdated
Show resolved
Hide resolved
|
||
export function restoreSettingsAsync(): ThunkAction { | ||
return async (dispatch, getState): Promise<void> => { | ||
const state: CombinedState = getState(); |
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 understand that you just copypasted the code, but the current implementation looks mind blowing. So, if you have ideas how it can be refactored, you are welcome
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 refactored parts related to the player, workspace, and image filter settings restoration—now the code looks cleaner.
As for the part that is handling shortcuts, where we currently have four nested loops—yes, it definitely needs refactoring. Ideally, conflictDetector should find all conflicts without requiring additional iterations. However, tackling this now feels a bit overwhelming. I'll work on it in a separate PR.
For now, let's focus on merging this customer request.
|
Motivation and context
save
button on the settings modal, now settings are auto savedHow has this been tested?
Checklist
develop
branchLicense
Feel free to contact the maintainers if that's a concern.