-
Notifications
You must be signed in to change notification settings - Fork 516
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
Issue#946 open links in new tab #1009
Issue#946 open links in new tab #1009
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than the two suggested changes, this looks and works great. Thanks @stephanmax
js/editor.js
Outdated
shadow.querySelector(relativeLink.hash).scrollIntoView(); | ||
}); | ||
}); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest adding these two functions to our utilities module:
https://github.com/stephanmax/interactive-examples/blob/e669e9f9f637684d371f97ac77ff7dbe27bf7145/js/editor-libs/mce-utils.js
js/editor.js
Outdated
@@ -80,6 +109,8 @@ | |||
|
|||
shadow.appendChild(document.importNode(content, true)); | |||
setOutputHeight(shadow.querySelector('div')); | |||
openLinksInNewTab(shadow.querySelectorAll('a[href^="http"]')); | |||
scrollToAnchors(shadow, shadow.querySelectorAll('a[href^="#"]')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You will then call these with:
mceUtils.openLinksInNewTab(shadow.querySelectorAll('a[href^="http"]'));
mceUtils.scrollToAnchors(shadow, shadow.querySelectorAll('a[href^="#"]'));
That makes sense, thanks for the fast review, @schalkneethling. I updated the PR accordingly. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
r+ Thanks @stephanmax
* upstream/master: (33 commits) Add HTML example for <a> (issue mdn#1014) (mdn#1022) fix(tabbed): load fonts as part of the editor css as @font-face does not work inside shadow dom (mdn#1015) html/input: Lowercase attributes (mdn#1024) Add color-adjust property example (mdn#1008) chore(deps): update dependency eslint to v5.1.0 (mdn#1023) chore(community): add @goodwin64 as contributor (mdn#1021) chore(community): add @dagolinuxoid as contributor (mdn#1020) chore(community): add @arai-a as contributor (mdn#1019) chore(community): add @ro-ka as contributor (mdn#1018) Issue mdn#1013 <style> elements in HTML editor break editor's render method (mdn#1017) chore(deps): update jest monorepo to v23.3.0 (mdn#1012) Issue#946 open links in new tab (mdn#1009) Send metric only during loading event (mdn#1011) fix(performance): only send loading mark inside loading event (mdn#1010) chore(deps): update dependency prettier to v1.13.7 (mdn#1006) fix(address): add name to the provided address (mdn#1004) chore(community): add @Arkangus as contributor (mdn#1002) Correct "expected output" mistake (mdn#1000) fix(performance): send post to kuma for custom ie-load-event-end metric (mdn#1001) chore(deps): update jest monorepo to v23.2.0 (mdn#993) ...
I added two helper functions that are called inside the editor's
render
method.openLinksInNewTab
prevents the default click event on external links (all anchors with anhref
attribute that starts withhttp
) and opens them in a new tab.While I was at it, I stumbled upon the issue that relative links (like
<a href="#someanchor">
) also didn't work. I didn't find a ticket for that so I thought it would make sense to fix in the scope of this ticket.scrollToAnchors
prevents the default click event on relative links and scrolls the targeted anchor into view.I hope that the code structure adheres to your quality standards.