-
Notifications
You must be signed in to change notification settings - Fork 2
Admin general settings #340
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: staging
Are you sure you want to change the base?
Conversation
}, | ||
{ | ||
id: '90', | ||
name: 'Delete after 90 days', |
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.
What: It looks like the log deletion options are hardcoded, which could lead to issues if these values need to change frequently.
Why: Hardcoding can make your application less flexible and more difficult to maintain. If the deletion periods change, you will have to modify the code instead of just changing a configuration or database entry.
How: Consider moving these options to a configuration file or fetching them from a server if they are subject to change. This would allow you to modify them without altering the code.
id: 'sendgrid', | ||
name: 'SendGrid - api@sendgrid.com', | ||
value: 'sendgrid', | ||
}, |
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.
What: The currently designed component does not have any error handling or feedback for the user on actions like Save.
Why: User feedback is crucial in UI components to improve usability. The user has no indication of whether their actions (like saving settings) were successful or resulted in an error.
How: Consider implementing success/error messages based on the form state or action outcomes. You can use notification components or modals to inform users about the success or failure of their actions.
}, | ||
]; | ||
|
||
const CONNECTION_OPTIONS = [ |
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.
What: Multiple states are managed with useState but without a reducer or similar state management approach.
Why: Using multiple state variables can lead to complex and hard-to-manage components. It can create problems with data synchronization and readability as the component grows.
How: Consider using the useReducer hook if state logic becomes complex or adopt a centralized state management approach for managing the form states efficiently.
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.
- The template does not match the Figma mockup.
- No topbar.
- The icon size and gap do not match.
- Use htmlFor and required attributes to link/associate the label with the first Select.
- Text Color does not match.
- Code not visible after clicking on the "Show code".
Description
This PR implements the design for this figma link -> https://www.figma.com/design/5PxINaOCAK9NdEHMlQk382/Force-UI-Components?node-id=13545-78045&t=k57dZs83BZS8v7th-4
Screenshots
https://tinyurl.com/ynz3smwz
https://tinyurl.com/yv8cqzd4
Types of changes
How has this been tested?
Checklist: