London10 | Kristina Dubnyk | cyf-hotel-react | React#595
London10 | Kristina Dubnyk | cyf-hotel-react | React#595KristinaDudnyk wants to merge 5 commits intoCodeYourFuture:masterfrom
Conversation
|
Kudos, SonarCloud Quality Gate passed!
|
src/Bookings.js
Outdated
|
|
||
| useEffect(() => { | ||
| fetchData(); | ||
| }, []); |
There was a problem hiding this comment.
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.
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.
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.
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.
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.
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.
Nice use of Spread ... to update the state
|
Kudos, SonarCloud Quality Gate passed!
|
2 participants
No description provided.