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

Intra-page links should not open in a new tab/window #494

Open
Mr0grog opened this issue Feb 25, 2020 · 2 comments
Open

Intra-page links should not open in a new tab/window #494

Mr0grog opened this issue Feb 25, 2020 · 2 comments

Comments

@Mr0grog
Copy link
Member

Mr0grog commented Feb 25, 2020

In #405 we made links open in new browser tabs/windows rather than navigating the diff view. That was a good improvement, but it also happens when clicking intra-page links, like the “Proposed Changes - A Holistic Approach to Closure Part B” link here:

https://monitoring.envirodatagov.org/page/c7a491b7-b296-4bdf-965e-372a87a7ac05/6f12e08a-a7f7-4ed1-8775-8c3791614fba..1fdefab4-bf6e-4829-892a-32c35466ae3d

That link should really just scroll down the page.

To fix this, we probably just need to check whether the href starts with # before adding target="_blank" to a link here:

/**
* Prevents navigation from within a diff by forcing links to open in a new
* tab when clicked. This helps ensure we don’t get in a state where one side
* of a side-by-side diff has been navigated and viewer does not realize they
* are no longer actually looking at a *diff*.
*
* NOTE: This requires the iframe displaying the diff to allow popups with the
* `sandbox="allow-popups"` attribute.
* @param {HTMLDocument} document The html document to transform.
* @returns {HTMLDocument}
*/
export function addTargetBlank (document) {
// Add target="_blank" to all <a>tags
document.querySelectorAll('a').forEach(node => {
node.setAttribute('target', '_blank');
});
return document;
}

@Mr0grog Mr0grog added this to Icebox in Web Monitoring via automation Mar 4, 2020
@Mr0grog Mr0grog moved this from Icebox to Ready in Web Monitoring Mar 4, 2020
@Mr0grog
Copy link
Member Author

Mr0grog commented Mar 17, 2020

Update: this also applies to javascript: links!

@SYU15 SYU15 self-assigned this Apr 3, 2020
@SYU15
Copy link
Contributor

SYU15 commented Apr 3, 2020

I'll take this @Mr0grog

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Web Monitoring
  
Ready
Development

No branches or pull requests

2 participants