Skip to content

Conversation

@validbeck
Copy link
Collaborator

Internal Notes for Reviewers

For sc-4381, I adjusted the behaviour of our anchor links to accommodate for the fixed top navigation. There's so much room because the mobile version of the top-nav is deeper than the desktop version.

Old New
old-anchor-spans new-anchor-spans

Span highlighting

I adjusted the <span> anchors I found to have this neat highlighting that draws attention to the active span!

span-highlighting

@validbeck validbeck self-assigned this Jul 4, 2024
@validbeck validbeck requested review from noosheenv and nrichers July 4, 2024 22:55
@github-actions
Copy link
Contributor

github-actions bot commented Jul 4, 2024

Pull requests must include at least one of the required labels: internal, highlight, enhancement, bug, deprecation, documentation. Except for internal, pull requests must also include a description in the release notes section.

@validbeck validbeck added the internal Not to be externalized in the release notes label Jul 4, 2024
Copy link
Collaborator

@nrichers nrichers left a comment

Choose a reason for hiding this comment

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

This PR does a nice job of fixing the anchor offset issue we were having. My only request would be that we not add highlighting around link targets, I don't think it adds anything that a user would need to know.

@validbeck validbeck requested a review from nrichers July 4, 2024 23:57
Copy link
Collaborator

@nrichers nrichers left a comment

Choose a reason for hiding this comment

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

My apologies for asking you to remove the cool pink colours from the PR. (Just commented them out, eh? 😁 )

Boring people say LGTM! :shipit:

@validbeck validbeck merged commit c9adc2c into main Jul 5, 2024
@validbeck validbeck deleted the beck/sc-4381/anchor-links-on-docs-site-offset-incorrectly branch July 5, 2024 00:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

internal Not to be externalized in the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants