-
Notifications
You must be signed in to change notification settings - Fork 14
Create the discussion board and the option to add a new discussion #44
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: dev
Are you sure you want to change the base?
Conversation
DvoraShechter1
left a comment
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.
LOTM!
please wait for @RutiRozenberg approve
|
Link the PR to the appropriate Issue, and attach a screenshot of the final result. |
RutiRozenberg
left a comment
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.
Overall, your code is excellent and works flawlessly. The comments provided are meant to enhance clarity and maintainability. I appreciate the effort you've put in, and with a few adjustments, it will be even better. Great job!
| borderRadius: '182.5px', | ||
| paddingTop:0.2, | ||
| paddingBottom:0.2, | ||
| 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.
Use the ThemeColor Object.
| paddingBottom:0.2, | ||
| color:'white', | ||
| paddingRight:2.4, | ||
| paddingLeft:2.4 |
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.
It seems that you've defined the padding properties in multiple lines. To improve readability and make the code more concise, combine them into a single line.
For example: padding: '0.2 2.4 0.2 2.4'
This way, it will be cleaner and more professional.
| export const Discussion = ({ props }) => { | ||
|
|
||
| const theme = useTheme(); | ||
|
|
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 unnecessary line.
| fontWeight: 700, | ||
| letterSpacing: 1.3, | ||
| fontSize: '1.42em', | ||
|
|
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 unnecessary line.
| import { Button, Typography, useTheme } from '@mui/material'; | ||
| import { useNavigate } from 'react-router'; | ||
|
|
||
| const typographyStyle = { |
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.
Great job using global reusable code! This aligns with the DRY principle and enhances code readability and maintainability. Keep it up!
|
|
||
| <ActiveControl props={{ title: 'יצירת דיון חדש' }} onOpen={Open} sx={{ | ||
| // position: 'absolute', top: '10%' | ||
| }}></ActiveControl> |
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.
Each property should be written on a separate line for better readability.
| </Typography> | ||
| <Typography | ||
| sx={{ | ||
| ...typhographyStyle, ml: '36.4vw' |
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.
Each property should be written on a separate line for better readability.
| /> | ||
| <Typography | ||
| sx={{ | ||
| ...typhographyStyle, ml: '32vw' |
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.
Each property should be written on a separate line for better readability.
| rows={7} | ||
| className="css-16wblaj-MuiInputBase-input-MuiOutlinedInput" | ||
| sx={{ | ||
| ...textFieldStyle, mb: 3 |
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.
Each property should be written on a separate line for better readability.
| 0 0 200px #49ACEF, | ||
| 0 0 300px #49ACEF`, | ||
| }} | ||
| > |
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 update the textShadow property to ensure proper indentation and readability
The task contains a few components:
Create Page to Display Discussions on a Specific Subject with New Discussion Option #41