Skip to content

First commit #2

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 36 commits into
base: main
Choose a base branch
from

Conversation

RyanOD
Copy link

@RyanOD RyanOD commented May 3, 2022

Week 3 HW Submission

Please fill out the information below in order to complete your assignment. Feel free to update this comment later if necessary.

  • Comfort rating on this assignment (1-5): 5
  • Completion rating on this assignment: Complete

Comments
A great project with loads of opportunities for trying and learning new things.

Questions
I used useMemo() in my Map component to prevent the map from rendering on clicks. Can you weigh in on my approach? I'm pretty sure useMemo() could also have been used around the actual map return, but I figured if the map depends on the center point, memoizing the center point should do the job.

I considered digging into useContext as I'm passing the cartItems to all my children components. In the end, I didn't get to that, but am still curious whether that would be a good use for the context API?

Admin and others added 30 commits May 3, 2022 10:21
… height and object-fit on large prop image.
…s and Maps components. Added PropType checking.
lat: lat,
lng: lng,
};
}, [lat, lng]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works fine. It'll also work if you pass {lat, lng} directly where you're passing center, but that's a bit repetitive. Otherwise, useCallback will throw a lint error wanting you to use useMemo for center.

}) {
useEffect(() => {
window.scrollTo(0, 0);
}, []);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this should be a reusable hook, considering how often it needs to be used with react-router 😅

import locationData from "./_data/locations.json";

export default function App() {
let [cartItems, setCartItems] = useState([]);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job! And yes, this would be a good case to use the Context API + useContext. Try it out if you have some time!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants