-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Saving settings to local storage #3017
Changes from all commits
15bea7e
529bd92
9963bb2
961993e
14719bc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,17 +1,22 @@ | ||
// Copyright (C) 2020 Intel Corporation | ||
// Copyright (C) 2020-2021 Intel Corporation | ||
// | ||
// SPDX-License-Identifier: MIT | ||
|
||
import './styles.scss'; | ||
import React from 'react'; | ||
import React, { useEffect } 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 } from '@ant-design/icons'; | ||
|
||
import { setSettings } from 'actions/settings-actions'; | ||
import WorkspaceSettingsContainer from 'containers/header/settings-modal/workspace-settings'; | ||
import PlayerSettingsContainer from 'containers/header/settings-modal/player-settings'; | ||
import Button from 'antd/lib/button'; | ||
import { CombinedState } from 'reducers/interfaces'; | ||
|
||
interface SettingsModalProps { | ||
visible: boolean; | ||
|
@@ -21,6 +26,44 @@ interface SettingsModalProps { | |
const SettingsModal = (props: SettingsModalProps): JSX.Element => { | ||
const { visible, onClose } = props; | ||
|
||
const settings = useSelector((state: CombinedState) => state.settings); | ||
const dispatch = useDispatch(); | ||
|
||
const onSaveSettings = (): void => { | ||
const settingsForSaving: any = {}; | ||
for (const [key, value] of Object.entries(settings)) { | ||
if (typeof value === 'object') { | ||
settingsForSaving[key] = value; | ||
} | ||
} | ||
localStorage.setItem('clientSettings', JSON.stringify(settingsForSaving)); | ||
notification.success({ | ||
message: 'Settings was successfully saved', | ||
}); | ||
}; | ||
|
||
useEffect(() => { | ||
try { | ||
const newSettings: any = {}; | ||
const loadedSettings = JSON.parse(localStorage.getItem('clientSettings') as string); | ||
for (const [sectionKey, section] of Object.entries(settings)) { | ||
for (const [key, value] of Object.entries(section)) { | ||
let settedValue = value; | ||
if (sectionKey in loadedSettings && key in loadedSettings[sectionKey]) { | ||
settedValue = loadedSettings[sectionKey][key]; | ||
} | ||
if (!(sectionKey in newSettings)) newSettings[sectionKey] = {}; | ||
newSettings[sectionKey][key] = settedValue; | ||
} | ||
} | ||
dispatch(setSettings(newSettings)); | ||
} catch { | ||
notification.error({ | ||
message: 'Failed to load settings from local storage', | ||
}); | ||
} | ||
}, []); | ||
|
||
return ( | ||
<Modal | ||
title='Settings' | ||
|
@@ -29,9 +72,16 @@ const SettingsModal = (props: SettingsModalProps): JSX.Element => { | |
width={800} | ||
className='cvat-settings-modal' | ||
footer={( | ||
<Button type='primary' onClick={onClose}> | ||
Close | ||
</Button> | ||
<> | ||
<Tooltip title='Will save settings from this page and appearance settings on standard workspace page in browser'> | ||
<Button type='primary' onClick={onSaveSettings}> | ||
Save | ||
</Button> | ||
</Tooltip> | ||
<Button type='default' onClick={onClose}> | ||
Close | ||
</Button> | ||
Comment on lines
+76
to
+83
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Need to update dock. Do we need button close? We have "X" and also click everywhere out of the modal closes the modal There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's default behaviour for the modal element. |
||
</> | ||
)} | ||
> | ||
<div className='cvat-settings-tabs'> | ||
|
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.
Why not to do it in defaultState for settings reducer?
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 of all, code for saving and restoring settings is placed in the same place. Secondly, we will try to load settings after first render, so it's a very small time gain for the first render, i guess. And it will be easier to modify this solution for server side config loading, for example.