React-Ebrahim Beiati-Asl cyf-hotel #539

ebrahimbeiati · 2023-02-27T19:57:10Z

No description provided.

JDysiewicz

Looking really good, a few stylistic things I'd change but overall a really nice first set of work for React!

JDysiewicz · 2023-03-01T07:21:39Z

src/App.js

    </div>
  );
 };
-
-export default App;
+export default App();


Careful with this - you want to export the whole function, App, whereas here you're only exporting the return of calling App(). When writing components, React will handle the calling of the function for you when you use the <Component /> syntax.

JDysiewicz · 2023-03-01T07:23:43Z

src/Bookings.js

-// import SearchResults from "./SearchResults.js";
-// import FakeBookings from "./data/fakeBookings.json";
+import SearchResults from "./SearchResults.js";
+import FakeBookings from "./data/fakeBookings.json";


Small stylistic thing, PascalCase is often reserved for React components, with normal JS variables being camelCase - you're importing a JSON file here so this should be camelCase as it is not a React component (it can look confusing as people will assume FakeBookings is a React component). This should be import fakeBookings from "./data/fakeBookings.json"

JDysiewicz · 2023-03-01T07:27:17Z

src/Footer.jsx

+  return (
+    <footer>
+      <ul className="footer-contact">
+        {props.map((props) => (


The variable name props is reserved for the argument passed to all React components (e.g. const Footer = (props) => {...}. Try not to call other variables props except for that one specific case, as it can be confusing. In this case, if you want to have default arguments for your props, you could use this syntax:

const Footer = (props = {address: "123 Fake Street", email: "[email protected]", phoneNumber: "0123 456789"}) => {...}

or

const defaultProps = {address: "123 Fake Street", email: "[email protected]", phoneNumber: "0123 456789"}; const Footer = (props = defaultProps) => {...}

JDysiewicz · 2023-03-01T07:28:01Z

src/Heading.jsx

@@ -0,0 +1,14 @@
+import React from "react";
+const Heading = () => {


JDysiewicz · 2023-03-01T07:28:43Z

src/Order.jsx

+  function orderOne() {
+    setOrders(orders + 1);
+  }


Perfect usage of useState, good job!

JDysiewicz · 2023-03-01T07:30:31Z

src/RestaurantButton.jsx

+
+const RestaurantButton = props => {
+    return (
+        <button onClick={props.orderOne} className="btn btn-primary">Add</button>


Small thing, de-structuring out your props is normally good practice e.g.

const RestaurantButton = ({orderOne}) => { return ( <button onClick={orderOne} className="">Add</button> ) }

JDysiewicz · 2023-03-01T07:31:16Z

src/SearchResults.js

@@ -0,0 +1,36 @@
+import React from "react";
+
+const SearchResults = ({ bookings }) => {


Nice use of destructuring here

JDysiewicz · 2023-03-01T07:32:28Z

src/TouristInfoCards.jsx

+      <div className="card">
+        <img
+          src="https://upload.wikimedia.org/wikipedia/commons/thumb/1/16/Kingston_Bridge_in_Glasgow.jpg/800px-Kingston_Bridge_in_Glasgow.jpg"
+          className="card-img-top"
+          alt="Glasgow on a budget"
+        />
+        <div className="card-body">
+          <h5 className="card-title">Glasgow</h5>
+          <p className="card-text">
+            Glasgow's city centre is home to flagship stores, impressive
+            shopping centres and designer within an easily walkable area.
+          </p>
+          <a href="http://peoplemakeglasgow.com" className="btn btn-primary">
+            Visit Glasgow
+          </a>
+        </div>
+      </div>


It looks like this set of HTML is repeated a few times - this could be extracted into a separate component and then rendered multiple times rather than repeating all this HTML

JDysiewicz

Some really nice work, good job :)

JDysiewicz · 2023-03-16T22:26:06Z

src/Bookings.js

+  useEffect(() => {
+    console.log("Fetching Information  be patient ... ");
+    fetch("https://cyf-react.glitch.me")
+      .then((response) => response.json())
+      .then((data) => setBookings(data))
+      .catch((err) => {
+        console.log(err);
+      });
+  }, []);
+
+  const [bookings, setBookings] = useState([]);
+  const [showList, setShowList] = useState(true);


Usually you'd put all the useStates above the useEffects, though that's just a stylistic thing

JDysiewicz · 2023-03-16T22:27:00Z

src/CustomerProfile.jsx

+  const [customerData, setCustomerData] = useState("");
+
+  useEffect(() => {
+    fetch(`https://cyf-react.glitch.me/customers/${props.id}`)
+      .then((res) => res.json())
+      .then((data) => setCustomerData(data));
+  }, [props.id]);


This could definitely be a good use for a custom hook, have a go at making one! :) (Don't worry, this is beyond what is expected)

JDysiewicz · 2023-03-16T22:28:02Z

src/Nights.jsx

+export const numberOfNight = ({ checkInDate, checkOutDate }) => {
+  const checkIn = moment(checkInDate);
+  const checkOut = moment(checkOutDate);
+  const nights = checkOut.diff(checkIn, "days");
+  return nights;
+};


Because you're not returning any jsx (this isn't a react component), this should just be a regular .js file instead of a .jsx file

JDysiewicz · 2023-03-16T22:29:12Z

src/Restaurant.js

+        <Order orderType="Pizza" />
+        <Order orderType="Cesar-Salad" />
+        <Order orderType="Chocolate cake" />
+        <Order orderType="Ice-Cream"/>


You could instead create an array of order types const orders = ["Pizza", "Cesar sald", "Ice cream"] and {orders.map(order => <Order orderType={order} />)} to save some repetition

JDysiewicz · 2023-03-16T22:30:04Z

src/SearchResults.js

+      fetch(`https://cyf-react.glitch.me/customers/${selectedRow}`)
+        .then((response) => response.json())
+        .then((data) => setCustomerData(data));


make sure to add a .catch as this network request could fail, and you'll want some way of letting the user know that the request has failed

JDysiewicz · 2023-03-16T22:30:37Z

src/SearchResults.js

+              <tr
+                key={result.id}
+                className={result.id === selectedRow ? "selected-colour" : ""}
+                onClick={() => handleRowClick(result.id)}
+              >
+                <td>{result.id}</td>
+                <td>{result.title}</td>
+                <td>{result.firstName}</td>
+                <td>{result.lastName}</td>
+                <td>{result.email}</td>
+                <td>{result.roomId}</td>
+                <td>{result.checkInDate}</td>
+                <td>{result.checkOutDate}</td>
+                <td>
+                  {numberOfNight({
+                    checkInDate: result.checkInDate,
+                    checkOutDate: result.checkOutDate,
+                  })}
+                </td>
+                <td>
+                  <button onClick={() => handleRowClick(result.id)}>
+                    Show profile
+                  </button>
+                </td>
+              </tr>


This could all be its own TableRow component

React

5852c13

JDysiewicz reviewed Mar 1, 2023

View reviewed changes

ebrahimbeiati added 2 commits March 2, 2023 19:41

editing

2cf737a

react hotel

50cd66a

JDysiewicz added the reviewed A volunteer has reviewed this PR label Mar 15, 2023

JDysiewicz reviewed Mar 16, 2023

View reviewed changes

update hotel react

d23bf39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

React-Ebrahim Beiati-Asl cyf-hotel #539

React-Ebrahim Beiati-Asl cyf-hotel #539

ebrahimbeiati commented Feb 27, 2023

JDysiewicz left a comment

JDysiewicz Mar 1, 2023

JDysiewicz Mar 1, 2023

JDysiewicz Mar 1, 2023

JDysiewicz Mar 1, 2023

JDysiewicz Mar 1, 2023

JDysiewicz Mar 1, 2023

JDysiewicz Mar 1, 2023

JDysiewicz Mar 1, 2023

JDysiewicz left a comment

JDysiewicz Mar 16, 2023

JDysiewicz Mar 16, 2023

JDysiewicz Mar 16, 2023

JDysiewicz Mar 16, 2023

JDysiewicz Mar 16, 2023

JDysiewicz Mar 16, 2023

		@@ -0,0 +1,14 @@
		import React from "react";
		const Heading = () => {

		@@ -0,0 +1,36 @@
		import React from "react";

		const SearchResults = ({ bookings }) => {

React-Ebrahim Beiati-Asl cyf-hotel #539

Are you sure you want to change the base?

React-Ebrahim Beiati-Asl cyf-hotel #539

Conversation

ebrahimbeiati commented Feb 27, 2023

JDysiewicz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JDysiewicz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment