Skip to content
This repository was archived by the owner on Sep 29, 2023. It is now read-only.

Conversation

stnguyen90
Copy link
Contributor

@stnguyen90 stnguyen90 commented Oct 5, 2022

What does this PR do?

Update the SDK and Version dropdowns to include location hash. Prior to this, if you:

  1. Browse to API docs like the Databases API docs
  2. Click on a link on the right side like Get Document
  3. Switch your SDK

You would jump to the top of the page instead of still being on the Get Document API.

Screen.Recording.2022-10-24.at.8.48.18.AM.mov

After this, you would stay on the API you last clicked on after switching the version or SDK.

Test Plan

Manual

Related PRs and Issues

Based off the following PR:

Have you read the Contributing Guidelines on issues?

Yes

@stnguyen90 stnguyen90 changed the title Feat version sdk control Update the SDK and Version dropdowns to include location hash Oct 5, 2022
@gewenyu99
Copy link
Contributor

This does preserve location for the most part. When we change the name of endpoints, it'll jump back to the top of the page because there's no matching hash, which is fine.

It works really well when I switch SDKs, which is great.

Now the real question is, since we're already adding JS, who don't we track scroll location?

@stnguyen90
Copy link
Contributor Author

Now the real question is, since we're already adding JS, who don't we track scroll location?

@gewenyu99, that seems much higher effort 😅

Copy link
Contributor

@gewenyu99 gewenyu99 left a comment

Choose a reason for hiding this comment

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

I tested this on homepage, works great. Idk if the implementation is optional, I'll leave it to engineering to decide :)

@stnguyen90 stnguyen90 force-pushed the feat-version-sdk-control branch from 8e9d57d to d229afb Compare October 6, 2022 18:43
@stnguyen90 stnguyen90 requested a review from gewenyu99 October 6, 2022 18:49
Copy link
Contributor

@gewenyu99 gewenyu99 left a comment

Choose a reason for hiding this comment

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

Tested again, lgtm

@eldadfux
Copy link
Member

Would love to get some more context are a short example to fully understand the new suggested behavior.

@stnguyen90
Copy link
Contributor Author

stnguyen90 commented Oct 24, 2022

Would love to get some more context are a short example to fully understand the new suggested behavior.

@eldadfux, I've updated the description.

@gewenyu99
Copy link
Contributor

@stnguyen90 The scroll bars look so weird there! is this what it looks like on windows?

@stnguyen90 stnguyen90 marked this pull request as draft October 24, 2022 16:19
@stnguyen90 stnguyen90 requested a review from eldadfux October 24, 2022 17:36
@stnguyen90 stnguyen90 marked this pull request as ready for review October 24, 2022 17:37
@eldadfux eldadfux merged commit 2b05c55 into main Oct 25, 2022
@TorstenDittmann TorstenDittmann deleted the feat-version-sdk-control branch October 25, 2022 06:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants