-
-
Notifications
You must be signed in to change notification settings - Fork 767
London 10 - Elena Barker - React Module - CYF-Hotel-React #602
base: master
Are you sure you want to change the base?
Conversation
Render <Restaurant /> component
src/Glasgow.png
Outdated
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.
This is CSS feedback. Make all images same size in the CSS.
src/Footer.js
Outdated
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.
CSS feedback. You could change the text to display in a row instead. In App.css create something like .footer{ display: flex; flex-direction: row; justify-content: space-evenly; }
src/Restaurant.js
Outdated
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.
CSS feedback. You could change it into a row instead using css. Same solution as the code example I gave in Footer
src/App.js
Outdated
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, the code demonstrates a good understanding of React components and their usage. However, there are areas for improvement, such as error handling, code formatting, and consistent naming conventions. Adding comments and documentation would also enhance the code's readability and maintainability.
To think about:
For your CustomerProfile you need a show/hide to handle profiles buttons.
The code could benefit from better code formatting and indentation for improved readability.
Error handling is minimal, and error messages are not displayed to the user, which can make debugging and user experience more challenging.
Some components, like Bookings and Restaurant, are missing proper opening and closing tags that can result in invalid JSX syntax.
The code could benefit from additional comments or documentation to provide more context and clarity, especially for complex logic and fetch requests.
What is great:
The code follows a modular structure with separate files for each component, which promotes code organization and reusability.
Components are imported and used correctly in the App component, making the code easier to read and maintain.
The usage of props in components like Footer and CustomerProfile allows for customization and flexibility.
The Search component handles user input and searching functionality effectively.
The SearchResults component displays search results and provides interactive features like highlighting selected rows and showing customer profiles.
The TouristInfoCard component showcases information about different tourist destinations in a visually appealing way.
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.
Thanks for your work Elena, I left some comments. Let me know if you have any questions.
src/SearchResults.js
Outdated
function highlightSelectedRow(id) { | ||
setIsActive(!isActive); | ||
setSelectedId(id); | ||
} |
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.
I think you're mixing the logic of highlighting the row when it is clicked (i.e. task 15) and clicking on the Show profile button (i.e. tasks 20 and 21).
Clicking on the row should highlight it without making any changes to the customer profile. Additionally, the user should be able to highlight multiple rows at the same time.
Trying clicking on different multiple rows in https://strong-meerkat-359514.netlify.app/ to experience how it should look like. Let me know if you can't spot the differences and I'll try to elaborate more.
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.
Hi @sherif98 , may I ask for help with this issue, please?
test commit
Kudos, SonarCloud Quality Gate passed! |
Netlify: https://cyf-elenabarker-hotel-react.netlify.app/