-
-
Notifications
You must be signed in to change notification settings - Fork 768
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.