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

Remove extraneous headers and nav item from versioned docs #2692

Merged
merged 2 commits into from
Feb 22, 2025

Conversation

im2nguyen
Copy link
Contributor

@im2nguyen im2nguyen commented Feb 20, 2025

πŸ”— Relevant links

πŸ—’οΈ What

🀷 Why

πŸ› οΈ How

πŸ“Έ Design Screenshots

πŸ§ͺ Testing

UI

  • Go to a versioned doc in the deploy preview.
    image

  • Select a different version and verify the sidebar navigation is the same. In previous versions, there would be an extraneous header.
    image

    Previous version (extra "Framework" and "Overview")
    image

Alternatively, run the test suite.

  • Run npm test and confirm all the tests pass.

πŸ’­ Anything else?

Copy link

vercel bot commented Feb 20, 2025

The latest updates on your projects. Learn more about Vercel for Git β†—οΈŽ

Name Status Preview Comments Updated (UTC)
dev-portal βœ… Ready (Inspect) Visit Preview πŸ’¬ Add feedback Feb 21, 2025 6:30pm

@im2nguyen im2nguyen requested review from a team, rmainwork and LeahMarieBush and removed request for a team February 20, 2025 00:54
Copy link

github-actions bot commented Feb 20, 2025

πŸ“¦ Next.js Bundle Analysis

This analysis was generated by the next.js bundle analysis action πŸ€–

This PR introduced no changes to the javascript bundle πŸ™Œ

Copy link
Collaborator

@RubenSandwich RubenSandwich left a comment

Choose a reason for hiding this comment

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

@im2nguyen because this change is a quite big one, we should write some tests.

You can see an example of testing remote-content's loadStaticProps function here:

await versionedDocsLoader.loadStaticProps({

(Also we didn't write any tests in it for latest. 😒

Copy link
Collaborator

@RubenSandwich RubenSandwich left a comment

Choose a reason for hiding this comment

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

🚒

But next time let's add some testing instructions, I did test is myself. But I have a lot of context on this change that others don't and we should make sure anyone on the team can approve PRs.

@im2nguyen
Copy link
Contributor Author

Thanks @RubenSandwich! I updated with testing instructions so folks in the future can test.

@im2nguyen im2nguyen merged commit 525e0e1 into main Feb 22, 2025
9 checks passed
@im2nguyen im2nguyen deleted the dos/fix-extraneous-headers-versioned-docs branch February 22, 2025 00:01
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