diff --git a/.github/actions-scripts/test-local-dev.js b/.github/actions-scripts/test-local-dev.js index f32865f98c..e78cd455fc 100755 --- a/.github/actions-scripts/test-local-dev.js +++ b/.github/actions-scripts/test-local-dev.js @@ -66,13 +66,6 @@ async function testJSONParameters() { assert(/enterprise-server@\d/.test(info)) } - // activeProducts - { - const res = await get('/en?json=activeProducts') - const activeProducts = JSON.parse(res.body) - assert(Array.isArray(activeProducts)) - } - // currentProduct (home page) { const res = await get('/en?json=currentProduct') diff --git a/components/context/MainContext.tsx b/components/context/MainContext.tsx index 3b48bfaf3b..408a5ed036 100644 --- a/components/context/MainContext.tsx +++ b/components/context/MainContext.tsx @@ -12,6 +12,12 @@ export type ProductT = { versions?: Array } +type SimpleProductT = { + href: string + name: string + external: boolean +} + type VersionItem = { // free-pro-team@latest, enterprise-cloud@latest, enterprise-server@3.3 ... version: string @@ -65,7 +71,7 @@ export type MainContextT = { maptopic?: BreadcrumbT article?: BreadcrumbT } - activeProducts: Array + homepageLinks: Array communityRedirect: { name: string href: string @@ -123,7 +129,7 @@ export const getMainContext = async (req: any, res: any): Promise return { breadcrumbs: req.context.breadcrumbs || {}, - activeProducts: req.context.activeProducts, + homepageLinks: req.context.homepageLinks || null, communityRedirect: req.context.page?.communityRedirect || {}, currentProduct: req.context.productMap[req.context.currentProduct] || null, currentLayoutName: req.context.currentLayoutName, diff --git a/lib/all-products.js b/lib/all-products.js index 0c71575607..e44c76e35e 100644 --- a/lib/all-products.js +++ b/lib/all-products.js @@ -7,7 +7,7 @@ import { ROOT } from './constants.js' // Both internal and external products are specified in content/index.md const homepage = path.posix.join(ROOT, 'content/index.md') -const { data } = frontmatter(await fs.readFile(homepage, 'utf8')) +export const { data } = frontmatter(await fs.readFile(homepage, 'utf8')) export const productIds = data.children @@ -44,40 +44,3 @@ for (const productId of productIds) { } export const productMap = Object.assign({}, internalProducts, externalProducts) - -function getPage(id, lang, pageMap) { - const productId = id.split('/')[0] - const product = productMap[productId] - const href = removeFPTFromPath(path.posix.join('/', lang, product.versions[0], id)) - const page = pageMap[href] - if (!page) { - throw new Error( - `Unable to find a page by the href '${href}'. Review your 'childGroups' frontmatter maybe.` - ) - } - // Return only the props needed for the ProductSelectionCard, since - // that's the only place this is ever used. - return { - id, - name: page.shortTitle || page.title, - href: href.replace(`/${lang}/`, '/'), - versions: page.applicableVersions, - } -} - -export function getProductGroups(pageMap, lang) { - return data.childGroups.map((group) => { - return { - name: group.name, - icon: group.icon || null, - octicon: group.octicon || null, - // Typically the children are product IDs, but we support deeper page paths too - children: group.children.map((id) => productMap[id] || getPage(id, lang, pageMap)), - } - }) -} - -export default { - productIds, - productMap, -} diff --git a/lib/get-product-groups.js b/lib/get-product-groups.js new file mode 100644 index 0000000000..f4bdfd8a6f --- /dev/null +++ b/lib/get-product-groups.js @@ -0,0 +1,55 @@ +import path from 'path' + +import { productMap, data } from './all-products.js' +import { renderContentWithFallback } from './render-with-fallback.js' +import removeFPTFromPath from './remove-fpt-from-path.js' + +async function getPage(id, lang, pageMap, context) { + const productId = id.split('/')[0] + const product = productMap[productId] + const href = removeFPTFromPath(path.posix.join('/', lang, product.versions[0], id)) + const page = pageMap[href] + if (!page) { + throw new Error( + `Unable to find a page by the href '${href}'. Review your 'childGroups' frontmatter maybe.` + ) + } + let name = '' + if (page.rawShortTitle) { + name = await renderContentWithFallback(page, 'rawShortTitle', context, { + textOnly: true, + throwIfEmpty: false, + }) + } + // Either the page didn't have a `rawShortTitle` or it was empty when + // rendered out with Liquid. Either way, have to fall back to `rawTitle`. + if (!name) { + name = await renderContentWithFallback(page, 'rawTitle', context, { + textOnly: true, + }) + } + // Return only the props needed for the ProductSelectionCard, since + // that's the only place this is ever used. + return { + id, + name, + href: href.replace(`/${lang}/`, '/'), + versions: page.applicableVersions, + } +} + +export async function getProductGroups(pageMap, lang, context) { + return await Promise.all( + data.childGroups.map(async (group) => { + return { + name: group.name, + icon: group.icon || null, + octicon: group.octicon || null, + // Typically the children are product IDs, but we support deeper page paths too + children: await Promise.all( + group.children.map((id) => getPage(id, lang, pageMap, context)) + ), + } + }) + ) +} diff --git a/middleware/context.js b/middleware/context.js index d993ff77e1..bfa933aa4e 100644 --- a/middleware/context.js +++ b/middleware/context.js @@ -2,23 +2,18 @@ import languages from '../lib/languages.js' import enterpriseServerReleases from '../lib/enterprise-server-releases.js' import { allVersions } from '../lib/all-versions.js' import { productMap } from '../lib/all-products.js' -import pathUtils from '../lib/path-utils.js' -import productNames from '../lib/product-names.js' -import warmServer from '../lib/warm-server.js' -import searchVersions from '../src/search/lib/versions.js' -import nonEnterpriseDefaultVersion from '../lib/non-enterprise-default-version.js' -import { getDataByLanguage, getUIDataMerged } from '../lib/get-data.js' - -const activeProducts = Object.values(productMap).filter( - (product) => !product.wip && !product.hidden -) -const { +import { getVersionStringFromPath, getProductStringFromPath, getCategoryStringFromPath, getPathWithoutLanguage, getPathWithoutVersion, -} = pathUtils +} from '../lib/path-utils.js' +import productNames from '../lib/product-names.js' +import warmServer from '../lib/warm-server.js' +import searchVersions from '../src/search/lib/versions.js' +import nonEnterpriseDefaultVersion from '../lib/non-enterprise-default-version.js' +import { getDataByLanguage, getUIDataMerged } from '../lib/get-data.js' // This doesn't change just because the request changes, so compute it once. const enterpriseServerVersions = Object.keys(allVersions).filter((version) => @@ -42,7 +37,6 @@ export default async function contextualize(req, res, next) { req.context.currentProduct = getProductStringFromPath(req.pagePath) req.context.currentCategory = getCategoryStringFromPath(req.pagePath) req.context.productMap = productMap - req.context.activeProducts = activeProducts req.context.allVersions = allVersions req.context.currentPathWithoutLanguage = getPathWithoutLanguage(req.pagePath) diff --git a/middleware/contextualizers/homepage-links.js b/middleware/contextualizers/homepage-links.js new file mode 100644 index 0000000000..0092cd75e6 --- /dev/null +++ b/middleware/contextualizers/homepage-links.js @@ -0,0 +1,127 @@ +import fs from 'fs' +import path from 'path' + +import { languageKeys } from '../../lib/languages.js' +import { allVersionKeys } from '../../lib/all-versions.js' + +import frontmatter from '../../lib/read-frontmatter.js' +import { ROOT } from '../../lib/constants.js' +import getApplicableVersions from '../../lib/get-applicable-versions.js' +import removeFPTFromPath from '../../lib/remove-fpt-from-path.js' +import { renderContentWithFallback } from '../../lib/render-with-fallback.js' +import { getPathWithoutVersion } from '../../lib/path-utils.js' + +const isHomepage = (path) => { + const split = path.split('/') + // E.g. `/foo` but not `foo/bar` or `foo/` + if (split.length === 2 && split[1] && !split[0]) { + return languageKeys.includes(split[1]) + } + // E.g. `/foo/possiblyproductname` but not `foo/possiblyproductname` or + // `/foo/something/` + if (split.length === 3 && !split[0] && split[2]) { + return allVersionKeys.includes(split[2]) + } + return false +} + +const isSearchpage = (path) => getPathWithoutVersion(path).split('/')[2] === 'search' + +export default async function productGroups(req, res, next) { + // It's important to use `req.pathPage` instead of `req.path` because + // the request could be the client-side routing from Next where the URL + // might be something like `/_next/data/foo/bar.json` which is translated, + // in another middleware, to what it would equate to if it wasn't + // client-side routing. + const { pagePath } = req + + if (isHomepage(pagePath) || isSearchpage(pagePath)) { + req.context.homepageLinks = await getHomepageLinks(req) + } + + return next() +} + +/** + * Indepedent of languages, the home page Page instance has a list of + * "children" (e.g. the 'children' frontmatter value in content/index.md) + * This is actually always the same for every language because translations + * don't have their own frontmatter keys that we keep. + * + * This function is memoized because we need this list for many different + * URLs such as `/en` and `/en/enterprise-cloud@latest` and `/ja` and + * `/fr/search` and so on. + */ +let cachedChildrenApplicableVersions = null +function getChildrenApplicableVersions(pages) { + if (!cachedChildrenApplicableVersions) { + const children = pages['/en'].children + const applicableVersions = [] + for (const child of children) { + const toc = path.join(ROOT, 'content', child, 'index.md') + const { data } = frontmatter(fs.readFileSync(toc, 'utf8')) + applicableVersions.push([child, getApplicableVersions(data.versions, toc)]) + } + cachedChildrenApplicableVersions = applicableVersions + } + return cachedChildrenApplicableVersions +} + +async function getHomepageLinks(req) { + const { currentVersion, pages } = req.context + const homePage = pages[`/${req.language}`] + if (!homePage) { + throw new Error(`no home page for '${req.language}'`) + } + + const links = [] + for (const [child, applicableVersions] of getChildrenApplicableVersions(pages)) { + // Only use the `currentVersion` if that page exists for that version. + // Otherwise default to the first applicable version. + // For example, if the current version is `enterprise-cloud@latest` + // and the page is `site-policy` we can't use `enterprise-cloud@latest` + // so we default to `applicableVersions[0]`. + const uri = applicableVersions.includes(currentVersion) + ? `/${currentVersion}/${child}` + : `/${applicableVersions[0]}/${child}` + const childHref = removeFPTFromPath(`/${req.language}${uri}`) + const childPage = pages[childHref] + if (!childPage) { + if (req.language === 'en') { + // If this throws, we have a bug in the code that figures out the URL + // from the `child` name. + throw new Error(`Based on the supposed URI '${uri}' there is no such page.`) + } + // Some pages are deliberately not available in non-English, + // like early-access. + continue + } + if (childPage.hidden || childPage.wip) { + continue + } + + let name = await renderContentWithFallback(childPage, 'rawShortTitle', req.context, { + textOnly: true, + }) + if (!name) { + name = await renderContentWithFallback(childPage, 'rawTitle', req.context, { + textOnly: true, + }) + } + links.push({ + name, + href: childHref, + external: false, + }) + } + + for (const { name, href, external } of Object.values(homePage.externalProducts)) { + links.push({ + name, + href, + external, + }) + } + + return links +} diff --git a/middleware/contextualizers/product-groups.js b/middleware/contextualizers/product-groups.js index 74bcb0cc95..560a6f0bbd 100644 --- a/middleware/contextualizers/product-groups.js +++ b/middleware/contextualizers/product-groups.js @@ -1,4 +1,4 @@ -import { getProductGroups } from '../../lib/all-products.js' +import { getProductGroups } from '../../lib/get-product-groups.js' import warmServer from '../../lib/warm-server.js' import { languageKeys } from '../../lib/languages.js' import { allVersionKeys } from '../../lib/all-versions.js' @@ -25,7 +25,7 @@ export default async function productGroups(req, res, next) { // client-side routing. if (isHomepage(req.pagePath)) { const { pages } = await warmServer() - req.context.productGroups = getProductGroups(pages, req.language) + req.context.productGroups = await getProductGroups(pages, req.language, req.context) } return next() diff --git a/middleware/index.js b/middleware/index.js index 9812b6616d..8cb1f8a795 100644 --- a/middleware/index.js +++ b/middleware/index.js @@ -50,6 +50,7 @@ import glossaries from './contextualizers/glossaries.js' import features from './contextualizers/features.js' import productExamples from './contextualizers/product-examples.js' import productGroups from './contextualizers/product-groups.js' +import homepageLinks from './contextualizers/homepage-links.js' import featuredLinks from '../src/landings/middleware/featured-links.js' import learningTrack from './learning-track.js' import next from './next.js' @@ -292,6 +293,7 @@ export default function (app) { app.use(asyncMiddleware(instrument(contextualizeSearch, './search/middleware/contextualize'))) app.use(asyncMiddleware(instrument(featuredLinks, './featured-links'))) app.use(asyncMiddleware(instrument(learningTrack, './learning-track'))) + app.use(asyncMiddleware(instrument(homepageLinks, './homepage-links'))) if (ENABLE_FASTLY_TESTING) { // The fastlyCacheTest middleware is intended to be used with Fastly to test caching behavior. diff --git a/src/landings/components/ProductLandingContext.tsx b/src/landings/components/ProductLandingContext.tsx index d1e27671b8..14d012c6df 100644 --- a/src/landings/components/ProductLandingContext.tsx +++ b/src/landings/components/ProductLandingContext.tsx @@ -105,8 +105,13 @@ export const getProductLandingContextFromRequest = async ( ? await page.renderProp('product_video', req.context, { textOnly: true }) : '' + const title = await page.renderProp('title', req.context, { textOnly: true }) + const shortTitle = (await page.renderProp('shortTitle', req.context, { textOnly: true })) || null + return { - ...pick(page, ['title', 'shortTitle', 'introPlainText', 'beta_product', 'intro']), + title, + shortTitle, + ...pick(page, ['introPlainText', 'beta_product', 'intro']), productVideo, productVideoTranscript: page.product_video_transcript || null, hasGuidesPage, diff --git a/src/landings/components/SidebarHomepage.tsx b/src/landings/components/SidebarHomepage.tsx index f065254e65..b0b19c6ac0 100644 --- a/src/landings/components/SidebarHomepage.tsx +++ b/src/landings/components/SidebarHomepage.tsx @@ -1,59 +1,47 @@ -import { useRouter } from 'next/router' import { LinkExternalIcon } from '@primer/octicons-react' import { TreeView } from '@primer/react' import { Link } from 'components/Link' -import { useVersion } from 'components/hooks/useVersion' import { useMainContext } from 'components/context/MainContext' export const SidebarHomepage = () => { - const router = useRouter() - const { currentVersion } = useVersion() - const { activeProducts, isFPT } = useMainContext() + const { homepageLinks } = useMainContext() + if (!homepageLinks) throw new Error("'homepageLinks' not set in main context") return (
- {activeProducts - .filter( - (product) => isFPT || product.versions?.includes(currentVersion) || product.external - ) - .map((product) => { - const href = `${!product.external ? `/${router.locale}` : ''}${ - product.versions?.includes(currentVersion) && !isFPT - ? `/${currentVersion}/${product.id}` - : product.href - }` - - return ( - { + const { href, external, name } = link + return ( + + { + if (e.nativeEvent instanceof KeyboardEvent && e.nativeEvent.code === 'Enter') { + document.getElementById(href)?.click() + e?.stopPropagation() + } + }} > - { - if (e.nativeEvent instanceof KeyboardEvent && e.nativeEvent.code === 'Enter') { - document.getElementById(href)?.click() - e?.stopPropagation() - } - }} - > - - {product.name} - {product.external && ( - - - - )} - - - - ) - })} + + {name} + {external && ( + + + + )} + + + + ) + })}
) diff --git a/src/pageinfo/tests/pageinfo.js b/src/pageinfo/tests/pageinfo.js index 488ffece04..fb34c41584 100644 --- a/src/pageinfo/tests/pageinfo.js +++ b/src/pageinfo/tests/pageinfo.js @@ -207,7 +207,7 @@ describe('pageinfo api', () => { const res = await get(makeURL('/ja/get-started/quickstart/hello-world')) expect(res.statusCode).toBe(200) const { info } = JSON.parse(res.body) - expect(info.product).toBe('Get started') + expect(info.product).toBe('はじめに') expect(info.title).toBe('こんにちは World') expect(info.intro).toBe('この Hello World 演習に従って、GitHub の使用を開始します。') }) diff --git a/tests/PLAYWRIGHT.md b/tests/PLAYWRIGHT.md index 37c8e16d0b..ee596f3b11 100644 --- a/tests/PLAYWRIGHT.md +++ b/tests/PLAYWRIGHT.md @@ -113,6 +113,38 @@ npm run playwright-test -- -g "view home page" ...will only run tests whose description contains that text. +## Updating browser binaries + +When we upgrade the `@playwright/test` version, if you haven't already +done it yourself, you might get an error from within Playwright that the +browsers aren't up-to-date. + +In VSCode you might get this error: + +```sh +Browser was not installed. Invoke 'Install Playwright Browsers' action to install missing browsers. +``` + +On the CLI you might get this: + +```sh + Error: browserType.launch: Executable doesn't exist at /Users/peterbe/Library/Caches/ms-playwright/webkit-1848/pw_run.sh + ╔═════════════════════════════════════════════════════════════════════════╗ + ║ Looks like Playwright Test or Playwright was just installed or updated. ║ + ║ Please run the following command to download new browsers: ║ + ║ ║ + ║ npx playwright install ║ + ║ ║ + ║ <3 Playwright Team ║ + ╚═════════════════════════════════════════════════════════════════════════╝ +``` + +All you have to do is run: + +```sh +npx playwright install +``` + ## Debugging Writing tests can be difficult until all the locators feel like diff --git a/tests/fixtures/content/index.md b/tests/fixtures/content/index.md index 19092ab3c5..47ec9368da 100644 --- a/tests/fixtures/content/index.md +++ b/tests/fixtures/content/index.md @@ -49,6 +49,11 @@ childGroups: octicon: RocketIcon children: - get-started + - name: CI/CD and DevOps + octicon: GearIcon + children: + - actions + - pages externalProducts: electron: diff --git a/tests/fixtures/content/pages/index.md b/tests/fixtures/content/pages/index.md index b9d366aacc..efa587c4d5 100644 --- a/tests/fixtures/content/pages/index.md +++ b/tests/fixtures/content/pages/index.md @@ -1,6 +1,6 @@ --- title: Pages Documentation -shortTitle: Pages +shortTitle: Pages ({% data variables.product.product_name %}) intro: 'Pages are cool on {% ifversion ghae %}{% data variables.product.product_name %}{% else %}{% data variables.location.product_location %}{% endif %}. ' introLinks: quickstart: /pages/quickstart diff --git a/tests/fixtures/translations/ja-jp/content/get-started/index.md b/tests/fixtures/translations/ja-jp/content/get-started/index.md new file mode 100644 index 0000000000..61fda7b842 --- /dev/null +++ b/tests/fixtures/translations/ja-jp/content/get-started/index.md @@ -0,0 +1,10 @@ +--- +title: GitHub の概要に関するドキュメント +shortTitle: はじめに +intro: '{% data variables.product.prodname_dotcom %} を使用してソフトウェアの構築、出荷、および保守を始める方法を学びます。 当社の製品を探索し、アカウントにサインアップして、世界最大の開発コミュニティと繋がりましょう。' +versions: + fpt: '*' + ghes: '*' + ghae: '*' + ghec: '*' +--- diff --git a/tests/rendering-fixtures/homepage.js b/tests/rendering-fixtures/homepage.js new file mode 100644 index 0000000000..49446f7378 --- /dev/null +++ b/tests/rendering-fixtures/homepage.js @@ -0,0 +1,26 @@ +import { expect } from '@jest/globals' + +import { get, getDOM } from '../helpers/e2etest.js' + +describe('home page', () => { + test('landing area', async () => { + const $ = await getDOM('/') + const container = $('#landing') + expect(container.length).toBe(1) + expect(container.find('h1').text()).toBe('GitHub Docs') + expect(container.find('p').text()).toMatch('Help for wherever you are on your GitHub journey') + }) + + test('product groups can use Liquid', async () => { + const $ = await getDOM('/') + const main = $('[data-testid="product"]') + const links = main.find('a[href*="/"]') + const hrefs = links.map((i, link) => $(link)).get() + for (const href of hrefs) { + const res = await get(href.attr('href')) + expect(res.statusCode).toBe(200) // Not needing to redirect + expect(href.text().includes('{%')).toBe(false) + } + expect.assertions(hrefs.length * 2) + }) +}) diff --git a/tests/rendering-fixtures/landing-hero.js b/tests/rendering-fixtures/landing-hero.js index 80bb8b81db..2f4b4b08c9 100644 --- a/tests/rendering-fixtures/landing-hero.js +++ b/tests/rendering-fixtures/landing-hero.js @@ -5,4 +5,18 @@ describe('product landing page', () => { const $ = await getDOM('/get-started') expect($('h1').first().text()).toMatch(/Getting started with HubGit/) }) + + test('product landing page lists with shortTitle heading (free-pro-team)', async () => { + const $ = await getDOM('/pages') + // Note that this particular page (in the fixtures) has Liquid + // in its shorTitle. + expect($('#all-docs a').first().text()).toMatch('All Pages (GitHub) docs') + }) + + test('product landing page lists with shortTitle heading (enterprise-server)', async () => { + const $ = await getDOM('/enterprise-server@latest/pages') + // Note that this particular page (in the fixtures) has Liquid + // in its shorTitle. + expect($('#all-docs a').first().text()).toMatch('All Pages (GitHub Enterprise Server) docs') + }) }) diff --git a/tests/rendering-fixtures/playwright-rendering.spec.ts b/tests/rendering-fixtures/playwright-rendering.spec.ts index 9eac2d9355..e16976be16 100644 --- a/tests/rendering-fixtures/playwright-rendering.spec.ts +++ b/tests/rendering-fixtures/playwright-rendering.spec.ts @@ -155,7 +155,7 @@ test('navigate with side bar into article inside a map-topic inside a category', // the category, you'll be able to see the map-topic and the article // within. await page.goto('/') - await page.getByRole('link', { name: 'GitHub Actions' }).click() + await page.getByTestId('sidebar').getByRole('link', { name: 'GitHub Actions' }).click() await page.getByTestId('sidebar').getByRole('treeitem', { name: 'Category' }).click() await page.getByText('Map & Topic').click() await page.getByRole('link', { name: '
' }).click() diff --git a/tests/rendering-fixtures/sidebar.js b/tests/rendering-fixtures/sidebar.js index 86770dc18f..37a40d27da 100644 --- a/tests/rendering-fixtures/sidebar.js +++ b/tests/rendering-fixtures/sidebar.js @@ -1,6 +1,6 @@ import { jest } from '@jest/globals' -import { getDOMCached as getDOM } from '../helpers/e2etest.js' +import { getDOMCached as getDOM, get } from '../helpers/e2etest.js' describe('sidebar', () => { jest.setTimeout(10 * 60 * 1000) @@ -26,6 +26,21 @@ describe('sidebar', () => { ) expect(earlyAccessLinks.length).toBe(0) }) + + test('all links have Liquid evaluated', async () => { + const $ = await getDOM('/') + const links = $('[data-testid=sidebar] a[href]') + const hrefs = links + .filter((i, link) => $(link).attr('href').startsWith('/')) + .map((i, link) => $(link)) + .get() + for (const href of hrefs) { + const res = await get(href.attr('href')) + expect(res.statusCode).toBe(200) // Not needing to redirect + expect(href.text().includes('{%')).toBe(false) + } + expect.assertions(hrefs.length * 2) + }) }) describe('nav', () => { diff --git a/tests/rendering-fixtures/translations.js b/tests/rendering-fixtures/translations.js index 09c6e32bac..81cad485b5 100644 --- a/tests/rendering-fixtures/translations.js +++ b/tests/rendering-fixtures/translations.js @@ -22,6 +22,15 @@ describe('translations', () => { expect(notification.length).toBe(1) const toEnglishDoc = notification.find('a#to-english-doc') expect(toEnglishDoc.text()).toBe('English documentation') + + // Sidebar uses the translated shortTitle + const links = $('[data-testid=sidebar] a[href]') + const hrefs = links + .filter((i, link) => $(link).attr('href').startsWith('/')) + .map((i, link) => $(link)) + .get() + const linkTexts = Object.fromEntries(hrefs.map(($link) => [$link.attr('href'), $link.text()])) + expect(linkTexts['/ja/get-started']).toBe('はじめに') }) test('hello world', async () => {