react-play icon indicating copy to clipboard operation
react-play copied to clipboard

[Play] - News Feed Application

Open NagarjunShroff opened this issue 3 years ago • 17 comments

First thing, PLEASE READ THIS: ReactPlay Code Review Checklist

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Fixes # (issue) #670

Type of change

Please delete options that are not relevant.

  • [ ] Bug fix (non-breaking change which fixes an issue)
  • [x] New feature (non-breaking change which adds functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • [ ] This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce.

Checklist:

  • [x] I have performed a self-review of my own code
  • [ ] I have commented my code, particularly in hard-to-understand areas
  • [ ] I have made corresponding changes to the documentation
  • [x] My changes generate no new warnings
  • [ ] I have added tests that prove my fix is effective or that my feature works
  • [ ] New and existing unit tests pass locally with my changes
  • [ ] Any dependent changes have been merged and published in downstream modules

NagarjunShroff avatar Oct 11 '22 09:10 NagarjunShroff

@NagarjunShroff is attempting to deploy a commit to a Personal Account owned by @reactplay on Vercel.

@reactplay first needs to authorize it.

vercel[bot] avatar Oct 11 '22 09:10 vercel[bot]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
react-play ✅ Ready (Inspect) Visit Preview Oct 20, 2022 at 11:41AM (UTC)

vercel[bot] avatar Oct 11 '22 11:10 vercel[bot]

@atapas @koustov here email is shown instead of name, is it because of github or user entry?

chrome_AXkCHDBcEY

murtuzaalisurti avatar Oct 11 '22 11:10 murtuzaalisurti

@NagarjunShroff I am getting a 426 error from the API.

chrome_wU49i7MYru

@murtuzaalisurti Checking...

NagarjunShroff avatar Oct 11 '22 12:10 NagarjunShroff

@atapas @koustov here email is shown instead of name, is it because of github or user entry?

chrome_AXkCHDBcEY

@NagarjunShroff Did you provide your github id while creating the play?

atapas avatar Oct 11 '22 13:10 atapas

@atapas @koustov here email is shown instead of name, is it because of github or user entry? chrome_AXkCHDBcEY

@NagarjunShroff Did you provide your github id while creating the play?

@atapas, yes i did

NagarjunShroff avatar Oct 11 '22 16:10 NagarjunShroff

Let's wrap up the review and fixes for merge.

atapas avatar Oct 14 '22 11:10 atapas

@NagarjunShroff Have we added the API key in vercel?

atapas avatar Oct 18 '22 09:10 atapas

@NagarjunShroff Have we added the API key in vercel?

No we haven't added it - Could you please add it?

NagarjunShroff avatar Oct 18 '22 10:10 NagarjunShroff

@atapas api key needs to be added I guess

murtuzaalisurti avatar Oct 18 '22 13:10 murtuzaalisurti

Visit Preview

I remember adding it. I shall check again. Irrespective of it, we need to handle the error messages correctly!

Due to some reasons, the newsData is undefined and breaks! The reason can be anything, including there are no API keys. Let's handle the error message correctly @NagarjunShroff

 <div className="card-container">
              {newsData.length > 0 ? (
                newsData.map((news, i) => <NewsCard news={news} key={i} />)
              ) : (
                <CircularProgress />
              )}
 </div>

atapas avatar Oct 18 '22 13:10 atapas

{"status":"error","code":"corsNotAllowed","message":"Requests from the browser are not allowed on the Developer plan, except from localhost."}

The free key cannot be used on a production server. So @NagarjunShroff what needs to be done for this play?

koustov avatar Oct 19 '22 18:10 koustov

{"status":"error","code":"corsNotAllowed","message":"Requests from the browser are not allowed on the Developer plan, except from localhost."}

The free key cannot be used on a production server. So @NagarjunShroff what needs to be done for this play?

@koustov, if APIs don't work- We can ignore it play.

NagarjunShroff avatar Oct 20 '22 05:10 NagarjunShroff

@NagarjunShroff

Why shelve the hard work?

Can you repurpose it using the free API?

Here is an example: https://saurav.tech/NewsAPI/

atapas avatar Oct 20 '22 06:10 atapas

@NagarjunShroff

Why shelve the hard work?

Can you repurpose it using the free API?

Here is an example: https://saurav.tech/NewsAPI/

Thanks!!! @atapas, Sure will do it.

NagarjunShroff avatar Oct 20 '22 07:10 NagarjunShroff

Ready for review.

NagarjunShroff avatar Oct 20 '22 11:10 NagarjunShroff

@atapas Please review.

NagarjunShroff avatar Oct 20 '22 11:10 NagarjunShroff