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

update React form component urls #928

Merged
merged 1 commit into from
Feb 20, 2025
Merged

Conversation

ktravers
Copy link
Contributor

I noticed that the component links in the React Forms doc were broken while reviewing https://github.com/github/github/pull/362188.

The url scheme seems to have changed since these docs were last updated. Looks like it now follows the pattern https://primer.style/components/:component-name/ for interface guidelines and https://primer.style/components/:component-name/react/latest for React implementation.

Questions for reviewers:

  • Are there any url helpers available? Hard-coding the urls seems brittle and could break again if/when the url scheme changes.
  • I've seen "dead link" linters in other repos that fail if any urls in the PR return a >4XX response (example). Does something like that seem useful for this repo?

url scheme seems to have changed since these docs were last updated. Now follows the pattern `https://primer.style/components/:component-name/` for interface guidelines and `https://primer.style/components/:component-name/react/latest` for React implementation.
@TylerJDev
Copy link
Member

Are there any url helpers available? Hard-coding the urls seems brittle and could break again if/when the url scheme changes.

I agree. I believe there was discussion around this recently for our new docs. I'm forgetting what came of it, but @joshblack might know!

Thanks for the PR too ✨

@ktravers ktravers merged commit 5dc4401 into main Feb 20, 2025
6 checks passed
@ktravers ktravers deleted the ktravers/update-react-forms-urls branch February 20, 2025 21:08
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.

2 participants