1
0
mirror of synced 2025-12-25 02:17:36 -05:00

Never hide intro on featured product links (#38640)

This commit is contained in:
Peter Bengtsson
2023-07-19 14:43:46 -04:00
committed by GitHub
parent 36d942900f
commit 9381cc1408
8 changed files with 88 additions and 82 deletions

View File

@@ -60,7 +60,7 @@ export const ArticleList = ({
href={link.href}
className="py-3"
title={
!link.hideIntro && link.intro ? (
link.intro ? (
<h3 className="f4" data-testid="link-with-intro-title">
<span>{link.fullTitle ? link.fullTitle : link.title}</span>
</h3>
@@ -71,7 +71,7 @@ export const ArticleList = ({
)
}
>
{!link.hideIntro && link.intro && (
{link.intro && (
<TruncateLines as="p" maxLines={2} className="color-fg-muted mb-0 mt-1">
<span data-testid="link-with-intro-intro">{link.intro}</span>
</TruncateLines>

View File

@@ -15,7 +15,6 @@ export type FeaturedLink = {
href: string
intro?: string
authors?: Array<string>
hideIntro?: boolean
date?: string
fullTitle?: string
}
@@ -154,7 +153,6 @@ export const getProductLandingContextFromRequest = async (
: '',
articles: links.map((link: any) => {
return {
hideIntro: key === 'popular',
href: link.href,
title: link.title,
intro: link.intro || null,

View File

@@ -1,6 +1,28 @@
import getLinkData from '#src/learning-track/lib/get-link-data.js'
import { renderContent } from '#src/content-render/index.js'
/**
* This is the max. number of featured links, by any category, that we
* display on the product landing pages.
* The reason it's variable is that some featured links are conditional.
* For example:
*
* '/authentication/troubleshooting-ssh',
* '/authentication/connecting-to-github-with...',
* '/authentication/connecting-to-github-with-ssh/a...',
* '{% ifversion ghae %}/authentication/connecting-to-...{% endif %}',
* '/authentication/managing-commit-signature-verif...'
*
* In this case, if we'd "prematurely" sliced that list to the first 4,
* the final result might be 3 items because that conditional one
* would end up being blank and thus omitted.
*
* The reason we don't want to display too many is because it might
* make the product landing page columns that lists links far too
* long ("high").
*/
const MAX_FEATURED_LINKS = 4
// this middleware adds properties to the context object
export default async function featuredLinks(req, res, next) {
if (!req.context.page) return next()
@@ -24,18 +46,17 @@ export default async function featuredLinks(req, res, next) {
// the provided string title or an empty title. When the title is empty,
// it indicates the video is not versioned for the current version
req.context.featuredLinks[key] = []
for (let i = 0; i < req.context.page.featuredLinks[key].length; i++) {
const title = await renderContent(
req.context.page.featuredLinks[key][i].title,
req.context,
{
textOnly: true,
},
)
const item = { title, href: req.context.page.featuredLinks[key][i].href }
for (const featuredLink of req.context.page.featuredLinks[key]) {
const title = await renderContent(featuredLink.title, req.context, {
textOnly: true,
})
const item = { title, href: featuredLink.href }
if (item.title) {
req.context.featuredLinks[key].push(item)
if (req.context.featuredLinks[key].length >= MAX_FEATURED_LINKS) {
break
}
}
}
} else {
@@ -43,6 +64,7 @@ export default async function featuredLinks(req, res, next) {
req.context.page.featuredLinks[key],
req.context,
{ title: true, intro: true, fullTitle: true },
MAX_FEATURED_LINKS,
)
}
}

View File

@@ -7,7 +7,7 @@ describe('curated homepage links', () => {
test('English', async () => {
const $ = await getDOM('/en')
const $links = $('[data-testid=bump-link]')
expect($links.length).toBeGreaterThanOrEqual(8)
expect($links.length).toBeGreaterThanOrEqual(6)
// Check that each link is localized and includes a title and intro
$links.each((i, el) => {

View File

@@ -4,69 +4,49 @@ import { getDOM } from '../../../tests/helpers/e2etest.js'
import enterpriseServerReleases from '../../../lib/enterprise-server-releases.js'
describe('featuredLinks', () => {
jest.setTimeout(3 * 60 * 1000)
jest.setTimeout(60 * 1000)
describe('rendering', () => {
test('non-TOC pages do not have intro links', async () => {
const $ = await getDOM('/en/get-started/quickstart/set-up-git')
expect($('[data-testid=article-list]')).toHaveLength(0)
})
test('landing page intro links have expected properties', async () => {
const $ = await getDOM('/en')
const $featuredLinks = $('[data-testid=article-list] a')
expect($featuredLinks).toHaveLength(9)
expect($featuredLinks.eq(0).attr('href')).toBe('/en/get-started/quickstart/set-up-git')
expect($featuredLinks.eq(0).children('h3').text().startsWith('Set up Git')).toBe(true)
expect($featuredLinks.eq(0).children('p').text().startsWith('At the heart of GitHub')).toBe(
true,
)
expect($featuredLinks.eq(8).attr('href')).toBe('/en/pages')
expect($featuredLinks.eq(8).children('h3').text().startsWith('GitHub Pages')).toBe(true)
expect(
$featuredLinks.eq(8).children('p').text().startsWith('Learn how to create a website'),
).toBe(true)
})
test('Enterprise user intro links have expected values', async () => {
const $ = await getDOM(`/en/enterprise/${enterpriseServerReleases.latest}/user/get-started`)
const $featuredLinks = $('[data-testid=article-list] a')
expect($featuredLinks.length > 0).toBeTruthy()
expect($featuredLinks.eq(0).attr('href')).toBe(
`/en/enterprise-server@${enterpriseServerReleases.latest}/get-started/learning-about-github/githubs-products`,
)
expect($featuredLinks.eq(0).children('h3').text().startsWith('GitHubs products')).toBe(true)
expect(
$featuredLinks
.eq(0)
.children('p')
.text()
.startsWith("An overview of GitHub's products and pricing plans."),
).toBe(true)
})
// If any of these tests fail, check to see if the content has changed and update text if needed.
test('product articles links respect versioning', async () => {
const enterpriseVersionedLandingPage = `/en/enterprise-server@${enterpriseServerReleases.latest}/billing`
const $ = await getDOM(enterpriseVersionedLandingPage)
const $productArticlesLinks = $('[data-testid=product-articles-list] a')
let msg = `Product article links are not rendered as expected on ${enterpriseVersionedLandingPage}`
expect($productArticlesLinks.length, msg).toBeGreaterThan(2)
// Confirm that the following Enterprise link IS included on this Enterprise page.
msg = `Enterprise article link is not rendered as expected on ${enterpriseVersionedLandingPage}`
expect(
$productArticlesLinks.text().includes('About licenses for GitHub Enterprise'),
msg,
).toBe(true)
// Confirm that the following Dotcom-only links are NOT included on this Enterprise page.
msg = `Dotcom-only article link is rendered, but should not be, on ${enterpriseVersionedLandingPage}`
expect($productArticlesLinks.text().includes('Adding or editing a payment method')).toBe(
false,
)
expect($productArticlesLinks.text().includes('Setting your billing email'), msg).toBe(false)
})
test('non-TOC pages do not have intro links', async () => {
const $ = await getDOM('/en/get-started/quickstart')
expect($('[data-testid=article-list]')).toHaveLength(0)
})
test('landing page intro links have expected properties', async () => {
const $ = await getDOM('/en')
const $featuredLinks = $('[data-testid=article-list] a')
expect($featuredLinks).toHaveLength(6)
expect($featuredLinks.eq(0).attr('href')).toBe('/en/get-started/quickstart/hello-world')
expect($featuredLinks.eq(0).children('h3').text()).toMatch('Hello World')
expect($featuredLinks.eq(0).children('p').text()).toMatch('Follow this Hello World exercise')
})
test('Enterprise intro links have expected values', async () => {
const $ = await getDOM('/enterprise-server@latest/get-started')
const $featuredLinks = $('[data-testid=article-list] a')
expect($featuredLinks.length).toBeGreaterThan(0)
expect($featuredLinks.eq(0).attr('href')).toBe(
`/en/enterprise-server@${enterpriseServerReleases.latest}/get-started/foo/bar`,
)
expect($featuredLinks.eq(0).children('h3').text()).toMatch('Bar Usually Comes After Foo')
expect($featuredLinks.eq(0).children('p').text()).toMatch(
"This page doesn't really have an intro",
)
})
// This is an important test because one of the popular links,
// in the front matter of `tests/fixtures/content/index.md`, uses
// Liquid to conditionally include with `{% ifversion ghec %}`.
test.each(['', '/enterprise-cloud@latest'])(
'never more than 4 links per category in %a',
async (version) => {
const $ = await getDOM(`/en${version}`)
const columns = $('[data-testid=article-list]')
expect(columns.length).toBe(2)
for (const column of columns) {
const $featuredLinks = $('a', column)
// See MAX_FEATURED_LINKS constant in featured-links.js middleware
expect($featuredLinks.length).toBeLessThanOrEqual(4)
}
},
)
})

View File

@@ -10,6 +10,7 @@ export default async (
rawLinks,
context,
option = { title: true, intro: true, fullTitle: false },
maxLinks = Infinity,
) => {
if (!rawLinks) return
@@ -17,9 +18,9 @@ export default async (
return await processLink(rawLinks, context, option)
}
const links = (
await Promise.all(rawLinks.map((link) => processLink(link, context, option)))
).filter(Boolean)
const links = (await Promise.all(rawLinks.map((link) => processLink(link, context, option))))
.filter(Boolean)
.slice(0, maxLinks)
return links
}