Skip to content

Conversation

@ImMPrada
Copy link
Owner

@ImMPrada ImMPrada commented Aug 29, 2022

This PR:

  • uses .env to store API endpoint into REACT_APP_API_ENDPOINT
  • upgrades useGithub hook for the new API endpoint and response data structure
  • upgrade components for the new response data structure

image

image

@vercel
Copy link

vercel bot commented Aug 29, 2022

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

Name Status Preview Updated
github-profile-viewer ✅ Ready (Inspect) Visit Preview Sep 1, 2022 at 8:35PM (UTC)

@ImMPrada ImMPrada changed the title 020 implement data from new api [WIP] 020 implement data from new api Aug 30, 2022
@ImMPrada ImMPrada force-pushed the 020-implement-data-from-new-api branch from 101d513 to 093f64d Compare September 1, 2022 20:35
@ImMPrada ImMPrada requested a review from mapra99 September 1, 2022 20:40
@ImMPrada ImMPrada changed the title [WIP] 020 implement data from new api 020 implement data from new api Sep 1, 2022
<div className="card-label__head__legend">
<div className="card-label__head__legend__info">
<h3>{state.profile.name}<br/><a href={state.profile.html_url}>@{state.profile.login}</a></h3>
<h3>{state.profile.name || 'no name user :('}<br/><a href={state.profile.url}>@{state.profile.login}</a></h3>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<h3>{state.profile.name || 'no name user :('}<br/><a href={state.profile.url}>@{state.profile.login}</a></h3>
<h3>{state.profile.name || 'No user name :('}<br/><a href={state.profile.url}>@{state.profile.login}</a></h3>

Comment on lines +16 to +25
<p>Gists</p>
<p>{state.profile.publicGistsCount}</p>
</div>
<div className="data-box__info">
<p>Followers</p>
<p>{state.profile.followers}</p>
<p>{state.profile.followersCount}</p>
</div>
<div className="data-box__info">
<p>Following</p>
<p>{state.profile.following}</p>
<p>{state.profile.followingsCount}</p>
Copy link
Collaborator

Choose a reason for hiding this comment

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

imo these <p> tags should be <span> because they are not actually paragraphs

<div className={`git-card-footer__element${state.profile.twitter ? '' : '-null'}`}>
<Twitter />
<p>{state.profile.twitter_username || 'Not Available'}</p>
<p>{state.profile.twitter || 'Not Available'}</p>
Copy link
Collaborator

Choose a reason for hiding this comment

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

same here

if(!reposList) setReposList(repos(state))
}

if(state.repos) {
Copy link
Collaborator

@mapra99 mapra99 Sep 4, 2022

Choose a reason for hiding this comment

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

I think it would be easier to understand if you say

if (!state.repos) return null

return (
 THE COMPONENT
)

instead of wrapping the actual component in a if clause and at the end return null in any other case

payload: curatedRepos,
})
const response = res.data
console.log(res.data)
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove console.log?

})
const response = res.data
console.log(res.data)
if(response.profile == null) {
Copy link
Collaborator

@mapra99 mapra99 Sep 4, 2022

Choose a reason for hiding this comment

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

Is it really necessary to use ==? Also is it necessary to compare with null? Is if(response.profile) enough?

Suggested change
if(response.profile == null) {
if(response.profile === null) {

)
}

return null

Choose a reason for hiding this comment

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

Codacy has a fix for the issue: Insert ;

Suggested change
return null
return null;

if(!reposList) setReposList(repos(state))
}

if(state.repos) {

Choose a reason for hiding this comment

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

Codacy has a fix for the issue: Insert ·

Suggested change
if(state.repos) {
if (state.repos) {

type: 'RESULT_OF_REPOS',
payload: curatedRepos,
})
const response = res.data

Choose a reason for hiding this comment

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

Codacy has a fix for the issue: Insert ;

Suggested change
const response = res.data
const response = res.data;

return (
<div className = {`error-message`}>
<h4>ERROR!</h4>
<h4>{state.message ? `Error: ${state.message}` : 'ERROR!'}</h4>

Choose a reason for hiding this comment

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

Codacy has a fix for the issue: Replace 'ERROR!' with "ERROR!"

Suggested change
<h4>{state.message ? `Error: ${state.message}` : 'ERROR!'}</h4>
<h4>{state.message ? `Error: ${state.message}` : "ERROR!"}</h4>

repos: payload,
},
'ADD_REPO': {
'LOAD_RESULTS': {

Choose a reason for hiding this comment

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

Codacy has a fix for the issue: Replace 'LOAD_RESULTS' with LOAD_RESULTS

Suggested change
'LOAD_RESULTS': {
LOAD_RESULTS: {

})
const response = res.data
console.log(res.data)
if(response.profile == null) {

Choose a reason for hiding this comment

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

Codacy has a fix for the issue: Insert ·

Suggested change
if(response.profile == null) {
if (response.profile == null) {

})
} else {
dispatch({
type: 'LOAD_RESULTS',

Choose a reason for hiding this comment

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

Codacy has a fix for the issue: Replace 'LOAD_RESULTS' with "LOAD_RESULTS"

Suggested change
type: 'LOAD_RESULTS',
type: "LOAD_RESULTS",

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.

3 participants