-
-
Notifications
You must be signed in to change notification settings - Fork 761
London10 | Kristina Dubnyk | cyf-hotel-react | React #595
base: master
Are you sure you want to change the base?
Conversation
|
Kudos, SonarCloud Quality Gate passed! |
src/Bookings.js
Outdated
|
|
||
| useEffect(() => { | ||
| fetchData(); | ||
| }, []); |
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.
Nice use of dependencies array, empty means the logic inside the useEffect will be executed once.
| // console.log("CustomerProfile props:", id); | ||
| const [clientData, setClientData] = useState(null); | ||
| const [loading, setLoading] = useState(false); | ||
| const [error, setError] = useState(false); |
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.
Just a minor suggestion here, if the variable is a boolean, naming it as isLoading, setIsLoading, isError ... is more intuitive.
This reference is of Java but I think this convention is widely adapted
https://geosoft.no/javastyle.html#Specific
| } | ||
| }, [id]); | ||
|
|
||
| return ( |
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.
Instead of deciding the rendering within return, do you thiink moving the condition out and returning accordlingly is more readable?
if(loading) {
retrun (
<div>Loading...</div>
)
}
if(error) {
return (
<div>Error</div>
)
}
return (
<>
...
</>
)
| <div className="footer"> | ||
| <ul> | ||
| {location.map((element, index) => ( | ||
| <li key={index}>{element}</li> |
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.
Happy to see you have the key attribute for each li
reference of key https://legacy.reactjs.org/docs/lists-and-keys.html#keys
| const [orders, setOrders] = useState(0); | ||
|
|
||
| function setOrdersFunction() { | ||
| setOrders(orders + 1); |
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.
FYI there is another way to update the state setOrders( (order) => order + 1). in this case the result is the same.
| } | ||
|
|
||
| function Order(props) { | ||
| const [orders, setOrders] = useState(0); |
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.
if the variable is a number type rather than an array, it is better to name it order, orders implies it is an array.
| setSelectedRows(selectedRows.filter((id) => id !== rowId)); | ||
| } else { | ||
| // Row is not selected, add it to selectedRows | ||
| setSelectedRows([...selectedRows, rowId]); |
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.
Nice use of Spread ... to update the state
|
Kudos, SonarCloud Quality Gate passed! |








No description provided.