Skip to content
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

Merged
merged 14 commits into from
Feb 13, 2025
Merged
8 changes: 8 additions & 0 deletions changelog.d/20250203_125000_klakhov_refactor_settings.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
### Changed

- Client settings are now saved automatically
(<https://github.com/cvat-ai/cvat/pull/9032>)

### Added
- Gamma filter settings are now automatically saved and restored upon reload
(<https://github.com/cvat-ai/cvat/pull/9032>)
120 changes: 67 additions & 53 deletions cvat-ui/src/components/header/settings-modal/settings-modal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,13 @@

import './styles.scss';
import _ from 'lodash';
import React, { useCallback, useEffect } from 'react';
import React, { useEffect, useState } from 'react';
import { useSelector, useDispatch } from 'react-redux';
import Tabs from 'antd/lib/tabs';
import Text from 'antd/lib/typography/Text';
import Modal from 'antd/lib/modal/Modal';
import Button from 'antd/lib/button';
import notification from 'antd/lib/notification';
import Tooltip from 'antd/lib/tooltip';
import { PlayCircleOutlined, LaptopOutlined, BuildOutlined } from '@ant-design/icons';

import { setSettings } from 'actions/settings-actions';
Expand All @@ -22,6 +21,8 @@ import PlayerSettingsContainer from 'containers/header/settings-modal/player-set
import ShortcutsSettingsContainer from 'containers/header/settings-modal/shortcuts-settings';
import { CombinedState } from 'reducers';
import { conflict, conflictDetector } from 'utils/conflict-detector';
import { ImageFilter, ImageFilterAlias } from 'utils/image-processing';
import GammaCorrection, { GammaFilterOptions } from 'utils/fabric-wrapper/gamma-correciton';

interface SettingsModalProps {
visible: boolean;
Expand All @@ -33,18 +34,32 @@ function SettingsModal(props: SettingsModalProps): JSX.Element {

const settings = useSelector((state: CombinedState) => state.settings);
const shortcuts = useSelector((state: CombinedState) => state.shortcuts);
const [settingsInitialized, setSettingsInitialized] = useState(false);
const dispatch = useDispatch();

const onSaveSettings = useCallback(() => {
useEffect(() => {
if (!settingsInitialized) return;

const settingsForSaving: any = {
shortcuts: {
keyMap: {},
},
imageFilters: [],
};
for (const [key, value] of Object.entries(settings)) {
if (['player', 'workspace'].includes(key)) {
settingsForSaving[key] = value;
}
if (key === 'imageFilters') {
const filters = [];
for (const filter of value) {
filters.push({
alias: filter.alias,
params: filter.modifier.serialize(),
});
}
settingsForSaving.imageFilters = filters;
}
}
for (const [key] of Object.entries(shortcuts.keyMap)) {
if (key in shortcuts.defaultState) {
Expand All @@ -55,18 +70,12 @@ function SettingsModal(props: SettingsModalProps): JSX.Element {
}

localStorage.setItem('clientSettings', JSON.stringify(settingsForSaving));
notification.success({
message: 'Settings were successfully saved',
className: 'cvat-notification-notice-save-settings-success',
});

onClose();
}, [onClose, settings, shortcuts]);
}, [setSettingsInitialized, settings, shortcuts]);

useEffect(() => {
try {
dispatch(shortcutsActions.setDefaultShortcuts(structuredClone(shortcuts.keyMap)));
const newSettings = _.pick(settings, 'player', 'workspace');
const newSettings = { ..._.pick(settings, 'player', 'workspace'), imageFilters: [] as ImageFilter[] };
const settingsString = localStorage.getItem('clientSettings') as string;
if (!settingsString) return;
const loadedSettings = JSON.parse(settingsString);
Expand All @@ -79,6 +88,26 @@ function SettingsModal(props: SettingsModalProps): JSX.Element {
}
}
}

newSettings.imageFilters = [];
if ('imageFilters' in loadedSettings) {
for (const filter of loadedSettings.imageFilters) {
switch (filter.alias) {
case ImageFilterAlias.GAMMA_CORRECTION: {
const modifier = new GammaCorrection(filter.params as GammaFilterOptions);
newSettings.imageFilters.push({
modifier,
alias: ImageFilterAlias.GAMMA_CORRECTION,
});
break;
}
default: {
break;
}
}
}
}

dispatch(setSettings(newSettings));
if ('shortcuts' in loadedSettings) {
const updateKeyMap = structuredClone(shortcuts.keyMap);
Expand Down Expand Up @@ -116,9 +145,32 @@ function SettingsModal(props: SettingsModalProps): JSX.Element {
message: 'Failed to load settings from local storage',
className: 'cvat-notification-notice-load-settings-fail',
});
} finally {
setSettingsInitialized(true);
}
}, []);

const tabItems = [
{
key: 'player',
label: <Text>Player</Text>,
icon: <PlayCircleOutlined />,
children: <PlayerSettingsContainer />,
},
{
key: 'workspace',
label: <Text>Workspace</Text>,
icon: <LaptopOutlined />,
children: <WorkspaceSettingsContainer />,
},
{
key: 'shortcuts',
label: <Text>Shortcuts</Text>,
icon: <BuildOutlined />,
children: <ShortcutsSettingsContainer />,
},
];

return (
<Modal
title='Settings'
Expand All @@ -127,51 +179,13 @@ function SettingsModal(props: SettingsModalProps): JSX.Element {
width={800}
className='cvat-settings-modal'
footer={(
<>
<Tooltip title='Will save settings to restore them after the app is reopened'>
<Button className='cvat-save-settings-button' type='primary' onClick={onSaveSettings}>
Save
</Button>
</Tooltip>
<Button className='cvat-close-settings-button' type='default' onClick={onClose}>
Close
</Button>
</>
<Button className='cvat-close-settings-button' type='default' onClick={onClose}>
Close
</Button>
)}
>
<div className='cvat-settings-tabs'>
<Tabs
type='card'
tabBarStyle={{ marginBottom: '0px', marginLeft: '-1px' }}
items={[{
key: 'player',
label: (
<span>
<PlayCircleOutlined />
<Text>Player</Text>
</span>
),
children: <PlayerSettingsContainer />,
}, {
key: 'workspace',
label: (
<span>
<LaptopOutlined />
<Text>Workspace</Text>
</span>
),
children: <WorkspaceSettingsContainer />,
}, {
key: 'shortcuts',
label: (
<span>
<BuildOutlined />
<Text>Shortcuts</Text>
</span>
),
children: <ShortcutsSettingsContainer />,
}]}
/>
<Tabs defaultActiveKey='player' type='card' items={tabItems} />
</div>
</Modal>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,15 @@ function ShortcutsSettingsComponent(props: Props): JSX.Element {
if (currentSettings) {
try {
const parsedSettings = JSON.parse(currentSettings);
delete parsedSettings.shortcuts;
parsedSettings.shortcuts = {};
for (const [key] of Object.entries(shortcuts.defaultState)) {
if (key in shortcuts.defaultState) {
parsedSettings.shortcuts.keyMap[key] = {
sequences: shortcuts.defaultState[key].sequences,
};
}
}

localStorage.setItem('clientSettings', JSON.stringify(parsedSettings));
} catch (error) {
localStorage.removeItem('clientSettings');
Expand Down
6 changes: 6 additions & 0 deletions cvat-ui/src/utils/fabric-wrapper/gamma-correciton.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,12 @@ export default class GammaCorrection extends FabricFilter {
this.#gamma = newGamma;
}

public serialize(): GammaFilterOptions {
return {
gamma: this.#gamma,
};
}

get gamma(): number {
return this.#gamma[0];
}
Expand Down
5 changes: 5 additions & 0 deletions cvat-ui/src/utils/image-processing.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export interface ImageProcessing {

processImage: (src: ImageData, frameNumber: number) => ImageData;
configure: (options: object) => void;
serialize: () => any;
}

/* eslint @typescript-eslint/no-unused-vars: ["error", { "argsIgnorePattern": "^_" }] */
Expand All @@ -23,6 +24,10 @@ export class BaseImageFilter implements ImageProcessing {
}

configure(_options: object): void {}

serialize(): any {
return {};
}
}

export interface ImageFilter {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,7 @@ context('Settings. Default number of points in polygon approximation.', () => {
const sliderAttrValueNow = slider.attr('aria-valuenow');
const sliderAttrValuemin = slider.attr('aria-valuemin');
const sliderAttrValuemax = slider.attr('aria-valuemax');
cy.saveSettings();
cy.closeNotification('.cvat-notification-notice-save-settings-success');
cy.closeSettings();
cy.reload();
testCheckSliderAttrValuenow(sliderAttrValueNow);
cy.contains('strong', 'less').click();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ context('Saving setting to local storage.', () => {
cy.get('.cvat-workspace-settings-show-interpolated').find('[type="checkbox"]')[method]();
cy.get('.cvat-workspace-settings-show-text-always').find('[type="checkbox"]')[method]();
cy.get('.cvat-workspace-settings-autoborders').find('[type="checkbox"]')[method]();
cy.saveSettings();
cy.get('.cvat-notification-notice-save-settings-success')
.should('exist')
.find('[data-icon="close"]')
.click();
cy.closeSettings();
cy.window().then((window) => {
const { localStorage } = window;
cy.wrap(localStorage.getItem('clientSettings')).should('exist').and('not.be.null');
});
}

function testCheckedSettings(checked = false) {
Expand Down
8 changes: 5 additions & 3 deletions tests/cypress/e2e/features/shortcuts.js
Original file line number Diff line number Diff line change
Expand Up @@ -155,8 +155,6 @@ context('Customizable Shortcuts', () => {

describe('Saving, Clearing and Restoring to Default', () => {
it('Saving shortcuts and checking if they persist', () => {
cy.openSettings();
cy.saveSettings();
cy.reload();
cy.openSettings();
cy.contains('Shortcuts').click();
Expand All @@ -183,7 +181,11 @@ context('Customizable Shortcuts', () => {
cy.get(
'.cvat-shortcuts-settings-collapse-item .cvat-shortcuts-settings-select .ant-select-selection-overflow-item').first().should('exist').and('be.visible');
cy.get('.cvat-shortcuts-settings-collapse-item .cvat-shortcuts-settings-select .ant-select-selection-overflow-item').first().contains('f1');
cy.saveSettings();
cy.closeSettings();
cy.window().then((window) => {
const { localStorage } = window;
cy.wrap(localStorage.getItem('clientSettings')).should('exist').and('not.be.null');
});
});
it('Modifying a shortcut via local storage and testing if its conflict is resolved', () => {
cy.window().then((window) => {
Expand Down
6 changes: 0 additions & 6 deletions tests/cypress/support/commands.js
Original file line number Diff line number Diff line change
Expand Up @@ -717,12 +717,6 @@ Cypress.Commands.add('closeSettings', () => {
cy.get('.cvat-settings-modal').should('not.be.visible');
});

Cypress.Commands.add('saveSettings', () => {
cy.get('.cvat-settings-modal').within(() => {
cy.contains('button', 'Save').click();
});
});

Cypress.Commands.add('changeWorkspace', (mode) => {
cy.get('.cvat-workspace-selector').click();
cy.get('.cvat-workspace-selector-dropdown').within(() => {
Expand Down
Loading