Skip to content
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

[website] Add star count fallback #3278

Merged
merged 6 commits into from
Mar 8, 2024

Conversation

bharatkashyap
Copy link
Member

@bharatkashyap bharatkashyap commented Mar 6, 2024

  • Seeing this on the landing page Screenshot 2024-03-06 at 2 48 40 PM

  • With a rate limiting API error being the reason for it:

{
    "message": "API rate limit exceeded for 49.228.103.47. (But here's the good news: Authenticated requests get a higher rate limit. Check out the documentation for more details.)",
    "documentation_url": "https://docs.github.com/rest/overview/resources-in-the-rest-api#rate-limiting"    
}
  • Tweak the Button UI to make the star count optional (empty if an error occurs):
Screenshot 2024-03-07 at 10 26 38 PM
  • Prevent a NaN on the star count

  • Log to console and do not cache in case of error

Live: https://deploy-preview-3278--mui-toolpad-docs.netlify.app/toolpad/

@bharatkashyap bharatkashyap added the website Related to the marketing pages label Mar 6, 2024
@Janpot
Copy link
Member

Janpot commented Mar 6, 2024

We should check whether the request actually returned a 200.

// initialize stars to -1 and display an error state when (!fetching && stars < 0)

setFetching(true);
try {
  const url = 'https://api.github.com/repos/mui/mui-toolpad'
  const response = await fetch(url);
  if (!response.ok) {
    // check for errors
    throw new Error(`HTTP ${response.status} while fetching ${url}`)
  }
  const data = await response.json();
  setStars(data.stargazers_count);
} finally {
  // make sure this always! runs by placing it in a finally block
  setFetching(false);
}

If the request fails we should either retry, or show an error. I'm not in favour of showing arbitrary fake star counts.

@bharatkashyap
Copy link
Member Author

bharatkashyap commented Mar 7, 2024

fake

I wouldn't say this is fake necessarily. Just outdated. What would you be in favour of showing in place of the star count if the request fails:

  • an error message such as this?
Screenshot 2024-03-07 at 10 09 20 AM
  • Or, let it be empty instead with perhaps an error border around it and a label showing the error message beneath the button?
Screenshot 2024-03-07 at 10 18 45 AM

@Janpot
Copy link
Member

Janpot commented Mar 7, 2024

Without a star count, this is just a link to our repository. We say "Star" and an empty star icon. Similar to:

Screenshot 2024-03-07 at 09 53 49

We just print an error to the console, the user doesn't care.

edit: in fact, we could use "{github icon} Star {empty star icon} {optional star count}" for all situations. Now it looks like I already starred the repository, while I didn't.

@bharatkashyap bharatkashyap merged commit 94a29cf into mui:master Mar 8, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
website Related to the marketing pages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants