-
Notifications
You must be signed in to change notification settings - Fork 7
Batsheva/display settings #140
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
base: mongo/design-settings-page
Are you sure you want to change the base?
Batsheva/display settings #140
Conversation
|
|
||
| .App-link { | ||
| color: #61dafb; | ||
| /* color: #a2ff00; */ |
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.
please remove comments
| import Footer from './stories/footer/FooterComponent'; | ||
| import { SnackbarProvider } from 'notistack'; | ||
| import './App.scss'; | ||
| import { ThemeProvider} from './themes/ThemeProvider.jsx' |
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.
re-order the imports
| align-items: center; | ||
| justify-content: center; | ||
| font-size: calc(10px + 2vmin); | ||
| color: white; |
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.
please remove all comments and use scss structure
| import axios from 'axios'; | ||
|
|
||
| const url = process.env.REACT_APP_SERVER_URL; | ||
| const url = process.env.REACT_APP_BASE_URL; |
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 do you use client url in axios middleware?
| import { useTranslation } from 'react-i18next'; | ||
| import Select from '../../stories/Select/Select.jsx'; | ||
| import CONSTANTS from './constantSetting.js'; | ||
| import './DisplaySettings.scss'; |
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.
re-order the imports
| req.body.profileImage = req.file.originalname; | ||
|
|
||
|
|
||
| // שינוי פורמט התאריך, נניח אם יש שדה בשם date |
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.
comments for explanation should be in english
| if(!mongoose.Types.ObjectId.isValid(id)) | ||
| return next({message:'id is not valid'}) | ||
| if (!mongoose.Types.ObjectId.isValid(id)) | ||
| return next({ message: 'id is not valid' }); |
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.
add status code
| } catch (err) { | ||
| console.error(err); | ||
| next({message:err.message,status:500}) | ||
| next({ message: err.message, status: 500 }); |
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.
add return statement
| res.json(visitedWebsites); | ||
| } catch (err) { | ||
| next({message:err.message,status:500}) | ||
| res.status(500).json({ message: err.message }); |
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.
add return statement
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.
please remove all necessary mp3 and jpg files
Added the DisplaySettings component, allowing users to change the website's theme and the location of popup notifications. The component includes options for theme selection and notification location with translation support.