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

Change how relative fragments are handled. #1260

Merged
merged 1 commit into from
May 7, 2020
Merged

Conversation

rogermparent
Copy link
Contributor

@rogermparent rogermparent commented May 7, 2020

Fixes #1140 as I understand it, but somebody please do check for possible regressions.

I can successfully do the following behavior:

Click one the links from the right side content bar.

Click back in your browser.

The same should work and return to the exact link in the document you got the section.

I can't think of any behavior this change would break, as both the previous and current behavior only apply to fragment links within the same page (with the href's first character being #), and other fragment links are left alone.

One thing to check, though, is the previous behavior of combining existing rel props with noopener noreferrer on all external links. The behavior I replaced it with only does so if the link is external and doesn't already specify a rel, as I figure covering basic external links is important but if the writer is going out of their way to add a rel then they should assume full control.
If we depend on the prior behavior and can't do without it, I can revert this branch to have that functionality while keeping the fix.

- Page-relative fragments (specifically, where the whole href starts with '#')
  are now demoted to plain `a` links in our link wrapper.

- The previous behavior that prepended to these links is now removed.

- The link wrapper `a` fallback has been expanded upon with comments, and now
  sets rel specifically on external links that don't already have it.
@shcheklein shcheklein temporarily deployed to dvc-landing-fix-local-f-rah750 May 7, 2020 18:51 Inactive
@rogermparent
Copy link
Contributor Author

Restyled had some sort of auth error that doesn't look related to the PR itself:
https://restyled.io/gh/iterative/repos/dvc.org/jobs/270328/log

@shcheklein
Copy link
Member

👍 cool stuff @rogermparent .

@shcheklein shcheklein merged commit 5c9a37d into master May 7, 2020
@shcheklein shcheklein deleted the fix-local-fragments branch February 28, 2021 19:36
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.

navigation: browser history navigation is broken
2 participants