MLH-Fellowship / prep-project-4.1.3

MLH Prep Project for Pod 4.1.3
https://mlh-prep-4-1-3.netlify.app/
MIT License
3 stars 10 forks source link

Fetched geolocation API to get user's location Closes #3 #9

Closed sumana2001 closed 2 years ago

sumana2001 commented 2 years ago

Used the Geolocation API to get the user's location as the first weather results displayed, instead of just New York City. Closes #3

https://user-images.githubusercontent.com/63084088/141899273-c3622608-74c2-4482-9007-a266b5af67d3.mov

netlify[bot] commented 2 years ago

✔️ Deploy Preview for mlh-prep-4-1-3 ready!

🔨 Explore the source changes: 8ee253c4f464537d495348c06d089ef391eecda8

🔍 Inspect the deploy log: https://app.netlify.com/sites/mlh-prep-4-1-3/deploys/6194d3cdccf48b0008f2143e

😎 Browse the preview: https://deploy-preview-9--mlh-prep-4-1-3.netlify.app/

grace-omotoso commented 2 years ago

@sumana2001 testing this on my system does not display any location even the default new york location. @PriyaBihani let me know how the testing goes on your end.

sumana2001 commented 2 years ago

@grace-omotoso It is not visible in the deploy version because I think Netlify doesn't have the env keys. This is the error I saw in the deploy preview. Can you add an API key please?

Screenshot 2021-11-16 at 6 07 29 PM
PriyaBihani commented 2 years ago

@grace-omotoso its working fine on my local machine when I added an API key. @sumana2001 just wondering can we make this a little cleaner it seems like we are crowding app.js file with functions and useEffect.😅 Otherwise it's perfect.

sumana2001 commented 2 years ago

@PriyaBihani Actually I had a doubt in it. Since we are using the same states in both the use Effects on the App.js file, is it okay if I make a file called custom hooks and shift the entire thing there? I feel the App.js will be much cleaner that way. Let me know what you think

PriyaBihani commented 2 years ago

@PriyaBihani Actually I had a doubt in it. Since we are using the same states in both the use Effects on the App.js file, is it okay if I make a file called custom hooks and shift the entire thing there? I feel the App.js will be much cleaner that way. Let me know what you think

I think its a great idea. Just create a structure like this helpers/customHooks/useWeather.js and create that hook there. @grace-omotoso What do you think?

sumana2001 commented 2 years ago

@PriyaBihani Please have a look now.

grace-omotoso commented 2 years ago

@sumana2001 what API_KEY did you use? I thought you are supposed to add it to the env variable file. The API_KEY we have on netlify might be different from what you used.

sumana2001 commented 2 years ago

@grace-omotoso I added my API key to the env file but it is ignored by git as it contains sensitive information. I don't think it matters if the API key is different as long as it is a valid API key but I can send it to you on discord personally.

Also I think maybe the issue is in deploy preview as it says here built without sensitive information with the following link given https://docs.netlify.com/configure-builds/environment-variables/#sensitive-variable-policy

This is what is says in brief: Some environment variables you may want to keep private. This can pose a challenge for sites connected to public repositories, where anyone can trigger a Deploy Preview by making a pull/merge request from a fork. We call these deploys untrusted deploys. Untrusted deploys will build automatically, but variables identified as sensitive will not be passed to the deploy environment.

Screenshot 2021-11-17 at 8 35 13 AM