1
0
mirror of synced 2025-12-19 18:10:59 -05:00

Preview hover cards (#34702)

Co-authored-by: Grace Park <gracepark@github.com>
Co-authored-by: Joe Oak <41307427+joeoak@users.noreply.github.com>
This commit is contained in:
Peter Bengtsson
2023-03-21 15:59:49 -04:00
committed by GitHub
parent bac2f9fdd5
commit 07a3e2a48c
19 changed files with 557 additions and 9 deletions

View File

@@ -0,0 +1 @@
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 16 16" width="16" height="16"><path d="M6.368 1.01a.75.75 0 0 1 .623.859L6.57 4.5h3.98l.46-2.868a.75.75 0 0 1 1.48.237L12.07 4.5h2.18a.75.75 0 0 1 0 1.5h-2.42l-.64 4h2.56a.75.75 0 0 1 0 1.5h-2.8l-.46 2.869a.75.75 0 0 1-1.48-.237l.42-2.632H5.45l-.46 2.869a.75.75 0 0 1-1.48-.237l.42-2.632H1.75a.75.75 0 0 1 0-1.5h2.42l.64-4H2.25a.75.75 0 0 1 0-1.5h2.8l.46-2.868a.75.75 0 0 1 .858-.622ZM9.67 10l.64-4H6.33l-.64 4Z"></path></svg>

After

Width:  |  Height:  |  Size: 479 B

View File

@@ -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...
//
// <body>
// <div id="main">
// <div id="sub" style="position:relative">
// <a href="...">Link</a>
//
// The `<a>` element's offset is based on the `<div id="sub" style="position:relative">`
// 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
// <div>, 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<HTMLDivElement>('#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
}

View File

@@ -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 (
<DefaultLayout>
<LinkPreviewPopover />
{isDev && <ClientSideRefresh />}
<ClientSideHighlight />
{router.pathname.includes('/rest/') && <RestRedirect />}
@@ -57,11 +58,50 @@ export const ArticlePage = () => {
<Breadcrumbs />
</div>
<ArticleGridLayout
topper={<ArticleTitle>{title}</ArticleTitle>}
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' && (
<Flash variant="default" className="mb-3">
<Box sx={{ display: 'flex' }}>
<Box
sx={{
p: 1,
textAlign: 'center',
}}
>
<BeakerIcon className="mr-2 color-fg-muted" />
</Box>
<Box
sx={{
flexGrow: 1,
p: 0,
}}
>
<p>
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{' '}
<a href="https://github.com/github/docs/discussions/24591">discussion</a>.
</p>
</Box>
</Box>
</Flash>
)}
<ArticleTitle>{title}</ArticleTitle>
</>
}
intro={
<>
{intro && (
<Lead data-testid="lead" data-search="lead">
// Note the `_page-intro` is used by the popover preview cards
// when it needs this text for in-page links.
<Lead data-testid="lead" data-search="lead" className="_page-intro">
{intro}
</Lead>
)}

View File

@@ -33,7 +33,9 @@ export const SidebarNav = ({ variant = 'full' }: Props) => {
<Link
data-testid="sidebar-product-xl"
href={currentProductTree.href}
className="d-block pl-1 mb-2 h3 color-fg-default no-underline"
// Note the `_product-title` is used by the popover preview cards
// when it needs this text for in-page links.
className="d-block pl-1 mb-2 h3 color-fg-default no-underline _product-title"
>
{productTitle}
</Link>

View File

@@ -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 <a> 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)
}

View File

@@ -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

View File

@@ -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

View File

@@ -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";

18
stylesheets/links.scss Normal file
View File

@@ -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;
}
}

View File

@@ -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)"

View File

@@ -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.

View File

@@ -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

18
tests/fixtures/content/pages/index.md vendored Normal file
View File

@@ -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
---

View File

@@ -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).

View File

@@ -0,0 +1 @@
{% ifversion fpt or ghec %}For more information, see "[AUTOTITLE](/get-started/quickstart/hello-world)."{% endif %}

View File

@@ -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 %}

View File

@@ -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'

View File

@@ -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()
}
})
})

View File

@@ -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('<svg')).toBeTruthy()
expect(res.text.includes('xmlns="http://www.w3.org/2000/svg"')).toBeTruthy()
})
test('.svg extension is optional', async () => {
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)
})
})