MLH-Fellowship / prep-project-22.OCT.PREP.1

MLH Prep Project for Pod 22.OCT.PREP.1
https://prep-project-22-oct-1.netlify.app/
MIT License
2 stars 15 forks source link

Add time of sunrise and sunset #26

Closed Ciggzy1312 closed 1 year ago

Ciggzy1312 commented 1 year ago

Please, go through these steps before you submit a PR.

Issue Resolved: #18 Description: Added sunrise and sunset timings of the searched place

netlify[bot] commented 1 year ago

Deploy Preview for prep-project-22-oct-1 ready!

Name Link
Latest commit cbd26e82c29c2c1035df66949893c93b39fd7c86
Latest deploy log https://app.netlify.com/sites/prep-project-22-oct-1/deploys/634ebe5f35ca54000861cc74
Deploy Preview https://deploy-preview-26--prep-project-22-oct-1.netlify.app/
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

akshitadixit commented 1 year ago

@Ciggzy1312 @kanchitank can we remove the basic text for the times and add like a small sunrise/sunset image with temp under them on either sides of the search box above the map?

akshitadixit commented 1 year ago

image sorry about the silly drawing but smth like this

Ciggzy1312 commented 1 year ago

@akshitadixit we did make a separate issue #28 regarding this only Can we make the changes there?

akshitadixit commented 1 year ago

Sure, until then please resolve conflicts.

Swatishree-Mahapatra commented 1 year ago

The default city is changed to 'Kolkata' but it shows 'New York' to me in the deployed preview. Can you check that @Ciggzy1312 @kanchitank? Apart from that, I think everything's alright.

Ciggzy1312 commented 1 year ago

@Swatishree-Mahapatra we did set the city name back to what it was, but it won't be having any effect because probably some API call or something is setting the city name to "New York" which is not related to this issue

Everything rest seems to be working fine

Swatishree-Mahapatra commented 1 year ago

Cool! The PR looks good to me now. @akshitadixit kindly look into this.

akshitadixit commented 1 year ago

@Swatishree-Mahapatra new conflicts, welp :)

Swatishree-Mahapatra commented 1 year ago

@Ciggzy1312 @kanchitank kindly look into the new merge conflicts. 🙂

Ciggzy1312 commented 1 year ago

@Swatishree-Mahapatra time for another quick review? :)

Swatishree-Mahapatra commented 1 year ago

The deploy is working fine and the code looks good. I guess we can go ahead with merging the PR.