jenkins-bot has submitted this change and it was merged. ( https://gerrit.wikimedia.org/r/379520 )
Change subject: New: Improve Link component ...................................................................... New: Improve Link component * Don't handle external links * Inspired on [1] * Don't handle links without href, prevented events, clicks with modifier keys pressed. Only handle left button clicks * Inspired on [2] This allows now to Cmd+click links (broken before) among other things and makes the link component more robust over all. [1]: https://github.com/joakin/protowiki/blob/master/src/components/capture-clicks/index.js#L27-L29 [2]: https://github.com/ReactTraining/react-router/blob/7c0481966e9f79da47ea483e083a4f694ecf35fe/packages/react-router-dom/modules/Link.js#L44-L49 Change-Id: I573e39f4695b436da28b2621f982117b2aa252f5 --- M src/common/components/link.tsx 1 file changed, 28 insertions(+), 3 deletions(-) Approvals: Niedzielski: Looks good to me, approved jenkins-bot: Verified diff --git a/src/common/components/link.tsx b/src/common/components/link.tsx index c815a9d..92b580c 100644 --- a/src/common/components/link.tsx +++ b/src/common/components/link.tsx @@ -6,9 +6,13 @@ href: string; } +function isModifiedEvent(event: MouseEvent): boolean { + return event.metaKey || event.altKey || event.ctrlKey || event.shiftKey; +} + /** * A single page app link that intercepts navigation (click) events and passes - * control to History. All local hyperlinks should use a Link component. + * control to History. All hyperlinks should use a Link component. */ export default function Link( { href, children, ...props }: Props, @@ -19,8 +23,29 @@ class={classOf("Link", props.class)} href={href} onClick={event => { - // todo: check that link is internal (relative). - if (context.history) { + const origin = window.location.origin; + const link = event.target as HTMLAnchorElement; + // TODO: Move all the logic to check if an event should be captured to a + // DOM utilities module and add unit tests + if ( + // Check if the href is internal + // + // N.B. link.href is used because browsers will transform relative + // paths to the full path when set on the HTML anchor, which is what + // is used to check against the current origin + link.href && + link.href.indexOf(origin) === 0 && + // onClick not prevented default + !event.defaultPrevented && + // Ignore everything but left clicks + event.button === 0 && + // Let browser handle "target=_blank" etc. + !link.target && + // Ignore clicks with modifier keys + !isModifiedEvent(event) && + // We have a context history + context.history + ) { event.preventDefault(); context.history.push(href); } -- To view, visit https://gerrit.wikimedia.org/r/379520 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I573e39f4695b436da28b2621f982117b2aa252f5 Gerrit-PatchSet: 2 Gerrit-Project: marvin Gerrit-Branch: master Gerrit-Owner: Jhernandez <[email protected]> Gerrit-Reviewer: Jhernandez <[email protected]> Gerrit-Reviewer: Niedzielski <[email protected]> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
