diff --git a/assets/images/site/hash.svg b/assets/images/site/hash.svg new file mode 100644 index 0000000000..c4a1aa8fee --- /dev/null +++ b/assets/images/site/hash.svg @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/components/LinkPreviewPopover.tsx b/components/LinkPreviewPopover.tsx new file mode 100644 index 0000000000..6370d02fb3 --- /dev/null +++ b/components/LinkPreviewPopover.tsx @@ -0,0 +1,248 @@ +import { useEffect } from 'react' + +// We delay the closing over the popover slightly in case the mouse +// movement either comes back (mouseover, mouseout, and back to mouseover) +// or if the user moves the mouse from the link to the popover itself +// and vice versa. +const DELAY = 300 + +// A global that is used for a slow/delayed closing of the popovers. +// It can be global because there's only 1 popover DOM node that gets +// created the first time it's needed. +let popoverCloseTimer: number | null = null + +function getOrCreatePopoverGlobal() { + let popoverGlobal = document.querySelector('div.Popover') as HTMLDivElement | null + if (!popoverGlobal) { + const wrapper = document.createElement('div') + wrapper.classList.add('Popover', 'position-absolute') + wrapper.style.display = 'none' + wrapper.style.outline = 'none' + wrapper.style.zIndex = `100` + const inner = document.createElement('div') + inner.classList.add( + ...'Popover-message Popover-message--large p-3 Box color-shadow-large Popover-message--bottom-left'.split( + /\s+/g + ) + ) + inner.style.width = `360px` + + const product = document.createElement('p') + product.classList.add('product') + product.classList.add('f6') + product.classList.add('color-fg-muted') + inner.appendChild(product) + inner.appendChild(product) + + const title = document.createElement('h4') + title.classList.add('h5') + title.classList.add('my-1') + inner.appendChild(title) + + const intro = document.createElement('p') + intro.classList.add('intro') + intro.classList.add('f6') + intro.classList.add('color-fg-muted') + inner.appendChild(intro) + + const anchor = document.createElement('p') + anchor.classList.add('anchor') + anchor.classList.add('hover-card-anchor') + anchor.classList.add('f6') + anchor.classList.add('color-fg-muted') + inner.appendChild(anchor) + + wrapper.appendChild(inner) + document.body.appendChild(wrapper) + + wrapper.addEventListener('mouseover', () => { + if (popoverCloseTimer) { + window.clearTimeout(popoverCloseTimer) + } + }) + wrapper.addEventListener('mouseout', () => { + popoverCloseTimer = window.setTimeout(() => { + wrapper.style.display = 'none' + }, DELAY) + }) + + popoverGlobal = wrapper + } + return popoverGlobal +} + +function popoverWrap(element: HTMLLinkElement) { + if (element.parentElement && element.parentElement.classList.contains('Popover')) { + return + } + let title = element.dataset.title + let product = element.dataset.productTitle || '' + let intro = element.dataset.intro || '' + let anchor = '' + + if (!title) { + // But, is it an in-page anchor link? If so, get the title, intro + // and product from within the DOM. But only if we can use the anchor + // destination to find a DOM node that has text. + if ( + element.href.includes('#') && + element.href.split('#')[1] && + element.href.startsWith(`${window.location.href}#`) + ) { + const domID = element.href.split('#')[1] + const domElement = document.querySelector(`#${domID}`) + if (domElement && domElement.textContent) { + anchor = domElement.textContent + // Now we have to make up the product, intro, and title + const domTitle = document.querySelector('h1') + if (domTitle && domTitle.textContent) { + title = domTitle.textContent + intro = '' + product = '' + const domProduct = document.querySelector('._product-title') + if (domProduct && domProduct.textContent) { + product = domProduct.textContent + } + const domIntro = document.querySelector('._page-intro') + if (domIntro && domIntro.textContent) { + intro = domIntro.textContent + } + } + } + } + } + + if (!title) return + + const popover = getOrCreatePopoverGlobal() + const productHead = popover.querySelector('p.product') as HTMLParagraphElement | null + if (productHead) { + if (product) { + productHead.textContent = product + productHead.style.display = 'block' + } else { + productHead.style.display = 'none' + } + } + + const anchorElement = popover.querySelector('p.anchor') as HTMLParagraphElement | null + if (anchorElement) { + if (anchor) { + anchorElement.textContent = anchor + anchorElement.style.display = 'block' + } else { + anchorElement.style.display = 'none' + } + } + + if (popoverCloseTimer) { + window.clearTimeout(popoverCloseTimer) + } + + const header = popover.querySelector('h4') + if (header) header.textContent = title + + const paragraph: HTMLParagraphElement | null = popover.querySelector('p.intro') + if (paragraph) { + if (intro) { + paragraph.textContent = intro + paragraph.style.display = 'block' + } else { + paragraph.style.display = 'none' + } + } + + const [top, left] = getOffset(element) + + // We can't know what the height of the popover element is when it's + // `display:none` so we guess offset to the offset and adjust it later. + popover.style.top = `${top - 100}px` + popover.style.left = `${left}px` + popover.style.display = 'block' + + popover.style.top = `${top - popover.offsetHeight - 10}px` +} + +// The top/left offset of an element is only relative to its parent. +// So if you have... +// +// +//
+//
+// Link +// +// The `` element's offset is based on the `
` +// and not the body as the user sees it relative to the viewport. +// So you have to traverse the offsets till you get to the root. +function getOffset(element: HTMLElement) { + let top = element.offsetTop + let left = element.offsetLeft + let offsetParent = element.offsetParent as HTMLElement | null + while (offsetParent) { + left += offsetParent.offsetLeft + top += offsetParent.offsetTop + offsetParent = offsetParent.offsetParent as HTMLElement | null + } + return [top, left] +} + +function popoverHide() { + // Important to use `window.setTimeout` instead of `setTimeout` so + // that TypeScript knows which kind of timeout we're talking about. + // If you use plain `setTimeout` TypeScript might think it's a + // Node eventloop kinda timer. + popoverCloseTimer = window.setTimeout(() => { + const popover = getOrCreatePopoverGlobal() + popover.style.display = 'none' + }, DELAY) +} + +function testTarget(target: HTMLLinkElement) { + // Return true if the element is an A tag, whose `href` starts with + // a `/`, and it's not one of those permalink ones next to headings + // (with the chain looking icon). + return ( + target.tagName === 'A' && + target.href.startsWith(window.location.origin) && + !target.classList.contains('doctocat-link') + ) +} + +export function LinkPreviewPopover() { + useEffect(() => { + // This event handler function is used for clicks anywhere in + // the `#article-contents` div. So we need to filter within. + function showPopover(event: MouseEvent) { + const target = event.target as HTMLLinkElement + if (testTarget(target)) { + popoverWrap(target) + } + } + function hidePopover(event: MouseEvent) { + const target = event.target as HTMLLinkElement + if (testTarget(target)) { + popoverHide() + } + } + + // The reason we have an event listener for ALL things within the + //
, instead of one for every `a[href]` element, is because + // this way we're prepared for the fact that new `a` elements + // might get introduced some other way. For example, if there's + // some any other code that does a `container.appendChild(newLink)` + const container = document.querySelector('#article-contents') + if (container) { + container.addEventListener('mouseover', showPopover) + container.addEventListener('mouseout', hidePopover) + } + + return () => { + if (container) { + container.removeEventListener('mouseover', showPopover) + container.removeEventListener('mouseout', hidePopover) + } + } + }) // Note that this runs on every single mount + + return null +} diff --git a/components/article/ArticlePage.tsx b/components/article/ArticlePage.tsx index a0630a2b50..67b3a93d6f 100644 --- a/components/article/ArticlePage.tsx +++ b/components/article/ArticlePage.tsx @@ -1,9 +1,10 @@ import { useRouter } from 'next/router' import dynamic from 'next/dynamic' import cx from 'classnames' +import { Box, Flash } from '@primer/react' +import { LinkExternalIcon, BeakerIcon } from '@primer/octicons-react' import { Callout } from 'components/ui/Callout' - import { DefaultLayout } from 'components/DefaultLayout' import { ArticleTitle } from 'components/article/ArticleTitle' import { useArticleContext } from 'components/context/ArticleContext' @@ -21,8 +22,7 @@ import { RestRedirect } from 'components/RestRedirect' import { Breadcrumbs } from 'components/page-header/Breadcrumbs' import { Link } from 'components/Link' import { useTranslation } from 'components/hooks/useTranslation' - -import { LinkExternalIcon } from '@primer/octicons-react' +import { LinkPreviewPopover } from 'components/LinkPreviewPopover' const ClientSideRefresh = dynamic(() => import('components/ClientSideRefresh'), { ssr: false, @@ -49,6 +49,7 @@ export const ArticlePage = () => { return ( + {isDev && } {router.pathname.includes('/rest/') && } @@ -57,11 +58,50 @@ export const ArticlePage = () => {
{title}} + topper={ + <> + {/* This is a temporary thing for the duration of the + feature-flagged release of hover preview cards on /$local/pages/ + articles. + Delete this whole thing when hover preview cards is + available on all articles independent of path. + */} + {router.query.productId === 'pages' && ( + + + + + + +

+ Hover over a link to another article to get more details. If you have ideas + for how we can improve this page, let us know in the{' '} + discussion. +

+ + + + )} + + {title} + + } intro={ <> {intro && ( - + // Note the `_page-intro` is used by the popover preview cards + // when it needs this text for in-page links. + {intro} )} diff --git a/components/sidebar/SidebarNav.tsx b/components/sidebar/SidebarNav.tsx index c7597b6feb..4eebe37de2 100644 --- a/components/sidebar/SidebarNav.tsx +++ b/components/sidebar/SidebarNav.tsx @@ -33,7 +33,9 @@ export const SidebarNav = ({ variant = 'full' }: Props) => { {productTitle} diff --git a/lib/render-content/plugins/rewrite-local-links.js b/lib/render-content/plugins/rewrite-local-links.js index 19976a6ff6..24a185ae72 100644 --- a/lib/render-content/plugins/rewrite-local-links.js +++ b/lib/render-content/plugins/rewrite-local-links.js @@ -22,6 +22,20 @@ const AUTOTITLE = /^\s*AUTOTITLE\s*$/ // which we use to know that we need to fall back to English. export class TitleFromAutotitleError extends Error {} +// E.g. `/en/pages/foo` or `/pt/pages/any/thing` +// But not `/en/pages` because that's not an article page anyway. +const ENABLED_PATHS_REGEX = /^\/\w{2}\/pages\// + +// Return true if we should bother setting data attributes. +// This becomes our feature-flag switch for enabling/disabling the +// hover preview cards. +// If you're on a page where we don't want hover preview cards, we +// can just omit these data attributes then the client-side will +// simply not be able to display them because the data isn't there. +function setDataAttributesOnCurrentPath(path) { + return ENABLED_PATHS_REGEX.test(path) +} + // Matches any tags with an href that starts with `/` const matcher = (node) => node.type === 'element' && @@ -33,7 +47,7 @@ const matcher = (node) => // Content authors write links like `/some/article/path`, but they need to be // rewritten on the fly to match the current language and page version export default function rewriteLocalLinks(context) { - const { currentLanguage, currentVersion } = context + const { currentLanguage, currentVersion, currentPath } = context // There's no languageCode or version passed, so nothing to do if (!currentLanguage || !currentVersion) return @@ -44,6 +58,9 @@ export default function rewriteLocalLinks(context) { const newHref = getNewHref(node, currentLanguage, currentVersion) if (newHref) { node.properties.href = newHref + if (setDataAttributesOnCurrentPath(currentPath)) { + promises.push(dataAttributesSetter(node, context)) + } } for (const child of node.children) { if (child.value) { @@ -73,6 +90,43 @@ export default function rewriteLocalLinks(context) { } } +async function dataAttributesSetter(node, context) { + const href = node.properties.href + const page = findPage(href, context.pages, context.redirects) + if (!page) { + // If this happens, it's a link that might be broken or links to + // something that might not actually be an internal link to a + // existing page such as link to archived enterprise version. + return + } + let productPage = null + for (const permalink of page.permalinks) { + const rootHref = permalink.href + .split('/') + .slice(0, permalink.pageVersion === 'free-pro-team@latest' ? 3 : 4) + .join('/') + const rootPage = context.pages[rootHref] + if (rootPage) { + productPage = rootPage + break + } + } + + if (productPage) { + let productTitle = await productPage.renderProp('shortTitle', context, { + textOnly: true, + }) + if (!productTitle) { + productTitle = await productPage.renderProp('title', context, { + textOnly: true, + }) + } + node.properties['data-product-title'] = productTitle + } + node.properties['data-title'] = await page.renderProp('title', context, { textOnly: true }) + node.properties['data-intro'] = await page.renderProp('intro', context, { textOnly: true }) +} + async function getNewTitleSetter(child, href, context) { child.value = await getNewTitle(href, context) } diff --git a/middleware/index.js b/middleware/index.js index f5d203061a..e1a283521a 100644 --- a/middleware/index.js +++ b/middleware/index.js @@ -30,6 +30,7 @@ import archivedEnterpriseVersionsAssets from './archived-enterprise-versions-ass import api from './api/index.js' import healthz from './healthz.js' import anchorRedirect from './anchor-redirect.js' +import productIcons from './product-icons.js' import remoteIP from './remote-ip.js' import buildInfo from './build-info.js' import archivedEnterpriseVersions from './archived-enterprise-versions.js' @@ -237,6 +238,7 @@ export default function (app) { app.use('/anchor-redirect', instrument(anchorRedirect, './anchor-redirect')) app.get('/_ip', instrument(remoteIP, './remoteIP')) app.get('/_build', instrument(buildInfo, './buildInfo')) + app.use('/producticons', instrument(productIcons, './product-icons')) // Things like `/api` sets their own Fastly surrogate keys. // Now that the `req.language` is known, set it for the remaining endpoints diff --git a/middleware/product-icons.js b/middleware/product-icons.js new file mode 100644 index 0000000000..86522f017d --- /dev/null +++ b/middleware/product-icons.js @@ -0,0 +1,43 @@ +import express from 'express' + +import octicons from '@primer/octicons' + +import { defaultCacheControl } from './cache-control.js' + +const router = express.Router() + +const SVG_CONTENT_TYPE = 'image/svg+xml' + +// Returns a client side redirect if one exists for the given path. +router.get('/react/:name', function productIcons(req, res) { + let { name } = req.params + if (name.endsWith('.svg')) { + name = name.replace(/\.svg$/, '') + } + // If the name is `FooBarIcon`, as a key in `octicons` it becomes + // `foo-bar`. The `Icon` is dropped and the capitalization is replaced with + // hyphens. + const asOcticonName = name + .replace(/Icon$/, '') + .replaceAll(/[a-z]([A-Z])/g, (whole, firstLetter) => { + return whole.replace(firstLetter, `-${firstLetter.toLowerCase()}`) + }) + .toLowerCase() + + // To avoid a "object injection attack", don't just use the square + // bracket. E.g. `someObject['constructor']` is truthy even + // when `const someObject = {foo: "bar"}`. + if (!Object.prototype.hasOwnProperty.call(octicons, asOcticonName)) { + return res.status(404).send('not found') + } + const asIcon = octicons[asOcticonName] + const asSVG = asIcon.toSVG({ + xmlns: 'http://www.w3.org/2000/svg', + }) + + defaultCacheControl(res) + res.set('content-type', SVG_CONTENT_TYPE) + res.status(200).send(Buffer.from(asSVG.trim())) +}) + +export default router diff --git a/stylesheets/index.scss b/stylesheets/index.scss index a8a9d4aa0e..7a4017b650 100644 --- a/stylesheets/index.scss +++ b/stylesheets/index.scss @@ -6,6 +6,7 @@ @import "@primer/css/labels/index.scss"; @import "@primer/css/avatars/avatar.scss"; @import "@primer/css/alerts/index.scss"; +@import "@primer/css/popover/index.scss"; @import "extended-markdown.scss"; @import "markdown-overrides.scss"; @@ -14,3 +15,4 @@ @import "shadows.scss"; @import "syntax-highlighting.scss"; @import "utilities.scss"; +@import "links.scss"; diff --git a/stylesheets/links.scss b/stylesheets/links.scss new file mode 100644 index 0000000000..2ee8a56c58 --- /dev/null +++ b/stylesheets/links.scss @@ -0,0 +1,18 @@ +p.hover-card-anchor { + border-top: 1px solid var(--color-border-default); + margin-top: 10px; + margin-bottom: 0; + padding-top: 16px; + + &:before { + content: ""; + background-color: var(--color-fg-muted); + display: inline-flex; + height: 16px; + width: 16px; + margin-right: 8px; + mask-image: url("/assets/cb-479/images/site/hash.svg"); + mask-size: cover; + vertical-align: middle; + } +} diff --git a/tests/fixtures/content/get-started/quickstart/hello-world.md b/tests/fixtures/content/get-started/quickstart/hello-world.md index ec583930f0..6b9dcb6712 100644 --- a/tests/fixtures/content/get-started/quickstart/hello-world.md +++ b/tests/fixtures/content/get-started/quickstart/hello-world.md @@ -24,3 +24,9 @@ Apparently, a Saab it is. Try changing the current version from "Free, Pro, & Team" to something like "Enterprise Server X.Y". It should change the above sentence. + +## Link to a page with variable title + +"[AUTOTITLE](/get-started/quickstart/dynamic-title)" + +"[AUTOTITLE](/get-started/foo/cross-version-linking)" diff --git a/tests/fixtures/content/get-started/quickstart/link-rewriting.md b/tests/fixtures/content/get-started/quickstart/link-rewriting.md index 9bac58724f..6387522ea5 100644 --- a/tests/fixtures/content/get-started/quickstart/link-rewriting.md +++ b/tests/fixtures/content/get-started/quickstart/link-rewriting.md @@ -11,7 +11,7 @@ type: quick_start ## Internal links never need language prefix -[AUTOTITLE](/get-started/foo/cross-version-linking) already tests things +"[AUTOTITLE](/get-started/foo/cross-version-linking)" already tests things like `/enterprise-server@latest/` becomes `/enterprise-server@X.Y/` where `X.Y` is the latest Enterprise server version. diff --git a/tests/fixtures/content/index.md b/tests/fixtures/content/index.md index 8b1b8bcb83..86be79835f 100644 --- a/tests/fixtures/content/index.md +++ b/tests/fixtures/content/index.md @@ -18,6 +18,7 @@ children: - search - get-started - early-access + - pages - code-security # - account-and-profile # - authentication @@ -40,7 +41,6 @@ children: # - discussions # - sponsors # - communities - # - pages # - education # - desktop # - support diff --git a/tests/fixtures/content/pages/index.md b/tests/fixtures/content/pages/index.md new file mode 100644 index 0000000000..b9d366aacc --- /dev/null +++ b/tests/fixtures/content/pages/index.md @@ -0,0 +1,18 @@ +--- +title: Pages Documentation +shortTitle: Pages +intro: 'Pages are cool on {% ifversion ghae %}{% data variables.product.product_name %}{% else %}{% data variables.location.product_location %}{% endif %}. ' +introLinks: + quickstart: /pages/quickstart +layout: product-landing +product: '{% data reusables.gated-features.pages %}' +versions: + fpt: '*' + ghes: '*' + ghae: '*' + ghec: '*' +topics: + - Pages +children: + - /quickstart +--- diff --git a/tests/fixtures/content/pages/quickstart.md b/tests/fixtures/content/pages/quickstart.md new file mode 100644 index 0000000000..88dc5e4b8d --- /dev/null +++ b/tests/fixtures/content/pages/quickstart.md @@ -0,0 +1,31 @@ +--- +title: Quickstart for GitHub Pages +intro: 'You can use {% data variables.product.prodname_pages %} to showcase some open source projects, host a blog, or even share your résumé. This guide will help get you started on creating your next website.' +allowTitleToDifferFromFilename: true +versions: + fpt: '*' + ghes: '*' + ghae: '*' + ghec: '*' +type: quick_start +topics: + - Pages +shortTitle: Quickstart +product: '{% data reusables.gated-features.pages %}' +--- + +## Introduction + +This page has 3 links. + +## Internal link + +But learn more on "[AUTOTITLE](/get-started/quickstart)." + +## In-page anchor link + +Go back to the [introduction](#introduction). + +## External link + +The repo [`github.com/github/docs`](https://github.com/github/docs). diff --git a/tests/fixtures/data/reusables/gated-features/more-info.md b/tests/fixtures/data/reusables/gated-features/more-info.md new file mode 100644 index 0000000000..8feede93e2 --- /dev/null +++ b/tests/fixtures/data/reusables/gated-features/more-info.md @@ -0,0 +1 @@ +{% ifversion fpt or ghec %}For more information, see "[AUTOTITLE](/get-started/quickstart/hello-world)."{% endif %} diff --git a/tests/fixtures/data/reusables/gated-features/pages.md b/tests/fixtures/data/reusables/gated-features/pages.md new file mode 100644 index 0000000000..73355f222f --- /dev/null +++ b/tests/fixtures/data/reusables/gated-features/pages.md @@ -0,0 +1 @@ +{% ifversion ghae %}{% data variables.product.prodname_pages %} is available in internal and private repositories with {% data variables.product.prodname_ghe_managed %}. {% endif %} diff --git a/tests/fixtures/data/variables/location.yml b/tests/fixtures/data/variables/location.yml new file mode 100644 index 0000000000..2101b7ab67 --- /dev/null +++ b/tests/fixtures/data/variables/location.yml @@ -0,0 +1,7 @@ +## Use these variables when referring specifically to a location within a product +product_location: >- + {% ifversion ghes %}your GitHub Enterprise Server instance{% elsif ghae %}your enterprise{% else %}GitHub.com{% endif %} + +# Used ONLY when you need to refer to a GHES instance in an article that is versioned for non-GHES versions. +# Do not use in other situations! +product_location_enterprise: 'your GitHub Enterprise Server instance' diff --git a/tests/rendering-fixtures/internal-links.js b/tests/rendering-fixtures/internal-links.js index b6f87eb07b..397e755298 100644 --- a/tests/rendering-fixtures/internal-links.js +++ b/tests/rendering-fixtures/internal-links.js @@ -105,3 +105,40 @@ describe('link-rewriting', () => { ) }) }) + +describe('data attributes for hover preview cards', () => { + test('check that internal links have the data attributes', async () => { + const $ = await getDOM('/pages/quickstart') + const links = $('#article-contents a[href]') + + // The internal link + { + const link = links.filter((i, element) => + $(element).attr('href').includes('/get-started/quickstart') + ) + expect(link.attr('data-title')).toBe('Quickstart') + expect(link.attr('data-product-title')).toBe('Get started') + // See tests/fixtures/content/get-started/quickstart/index.md + expect(link.attr('data-intro')).toBe( + 'Get started using GitHub to manage Git repositories and collaborate with others.' + ) + } + + // The anchor link has none + { + const link = links.filter((i, element) => $(element).attr('href') === '#introduction') + expect(link.attr('data-title')).toBeUndefined() + expect(link.attr('data-product-title')).toBeUndefined() + expect(link.attr('data-intro')).toBeUndefined() + } + // The external link has none + { + const link = links.filter((i, element) => + $(element).attr('href').startsWith('https://github.com') + ) + expect(link.attr('data-title')).toBeUndefined() + expect(link.attr('data-product-title')).toBeUndefined() + expect(link.attr('data-intro')).toBeUndefined() + } + }) +}) diff --git a/tests/routing/product-icons.js b/tests/routing/product-icons.js new file mode 100644 index 0000000000..0b8075099d --- /dev/null +++ b/tests/routing/product-icons.js @@ -0,0 +1,37 @@ +import { describe, jest, test } from '@jest/globals' + +import { get, head } from '../helpers/e2etest.js' + +describe('product-icons', () => { + jest.setTimeout(60 * 1000) + + test('happy path', async () => { + const res = await get('/producticons/react/CommentDiscussionIcon.svg') + expect(res.statusCode).toBe(200) + expect(res.headers['content-type']).toBe('image/svg+xml') + expect(res.text.startsWith(' { + const res = await get('/producticons/react/DeviceMobileIcon') + expect(res.statusCode).toBe(200) + expect(res.headers['content-type']).toBe('image/svg+xml') + }) + + test('head', async () => { + const res = await head('/producticons/react/CommentDiscussionIcon.svg') + expect(res.statusCode).toBe(200) + expect(res.text).toBe('') + }) + + test('object "keys" that aren\'t icons should fail', async () => { + const res = await head('/producticons/react/ConstructorIcon.svg') + expect(res.statusCode).toBe(404) + }) + + test('404 on unrecognized icons', async () => { + const res = await head('/producticons/react/FooBaringIcon.svg') + expect(res.statusCode).toBe(404) + }) +})