Article Layout Improvements (#22424)

* moving breadcrumbs to sticky header

* update scroll

* update color-bg-primary to color-bg-default

* change back to primary since were on primer 17

* add effect dependency

* not changing

* clip left box shadow of header and unmount

* update to default

* update breadcrumbs tests

* sticky editor

components/article/ArticleGridLayout.tsx

@@ -56,6 +56,7 @@ const Container = styled(Box)`
 
   @media (min-width: ${themeGet('breakpoints.3')}) {
     max-width: none;
+    padding-top: ${themeGet('space.4')};
     grid-template-rows: auto 1fr;
     grid-template-columns: minmax(500px, 720px) minmax(220px, 1fr);
     grid-template-areas:
@@ -71,7 +72,7 @@ const SidebarContent = styled(Box)`
   @media (min-width: ${themeGet('breakpoints.3')}) {
     position: sticky;
     padding-top: ${themeGet('space.4')};
-    top: 0;
+    top: 4em;
     max-height: calc(100vh - ${themeGet('space.4')});
     overflow-y: auto;
     padding-bottom: ${themeGet('space.4')};

components/article/ArticlePage.tsx

@@ -15,7 +15,6 @@ import { MarkdownContent } from 'components/ui/MarkdownContent'
 import { Lead } from 'components/ui/Lead'
 import { ArticleGridLayout } from './ArticleGridLayout'
 import { VersionPicker } from 'components/VersionPicker'
-import { Breadcrumbs } from 'components/Breadcrumbs'
 
 // Mapping of a "normal" article to it's interactive counterpart
 const interactiveAlternatives: Record<string, { href: string }> = {
@@ -60,12 +59,10 @@ export const ArticlePage = () => {
     <DefaultLayout>
       <div className="container-xl px-3 px-md-6 my-4">
         <ArticleGridLayout
-          topper={<Breadcrumbs />}
+          topper={<ArticleTitle>{title}</ArticleTitle>}
           topperSidebar={<VersionPicker />}
           intro={
             <>
-              <ArticleTitle>{title}</ArticleTitle>
-
               {contributor && (
                 <Callout variant="info" className="mb-3">
                   <p>

components/article/ArticleTitle.tsx

@@ -4,7 +4,7 @@ type Props = {
 export const ArticleTitle = ({ children }: Props) => {
   return (
     <div className="d-flex flex-items-baseline flex-justify-between">
-      <h1 className="mt-4 border-bottom-0">{children}</h1>
+      <h1 className="border-bottom-0">{children}</h1>
     </div>
   )
 }

components/context/MainContext.tsx

@@ -1,7 +1,7 @@
 import { createContext, useContext } from 'react'
 import pick from 'lodash/pick'
 
-import type { BreadcrumbT } from 'components/Breadcrumbs'
+import type { BreadcrumbT } from 'components/page-header/Breadcrumbs'
 import type { FeatureFlags } from 'components/hooks/useFeatureFlags'
 import { ExcludesNull } from 'components/lib/ExcludesNull'
 

components/landing/TocLanding.tsx

@@ -2,7 +2,6 @@ import { DefaultLayout } from 'components/DefaultLayout'
 import { TableOfContents } from 'components/landing/TableOfContents'
 import { useTocLandingContext } from 'components/context/TocLandingContext'
 import { VersionPicker } from 'components/VersionPicker'
-import { Breadcrumbs } from 'components/Breadcrumbs'
 import { ArticleTitle } from 'components/article/ArticleTitle'
 import { MarkdownContent } from 'components/ui/MarkdownContent'
 import { ArticleList } from 'components/landing/ArticleList'
@@ -28,7 +27,7 @@ export const TocLanding = () => {
   return (
     <DefaultLayout>
       <div className="container-xl px-3 px-md-6 my-4">
-        <ArticleGridLayout topper={<Breadcrumbs />} topperSidebar={<VersionPicker />}>
+        <ArticleGridLayout topperSidebar={<VersionPicker />}>
           <ArticleTitle>{title}</ArticleTitle>
 
           {introPlainText && <Lead>{introPlainText}</Lead>}

components/Breadcrumbs.tsx → components/page-header/Breadcrumbs.tsx

@@ -1,6 +1,6 @@
 import cx from 'classnames'
 import { useRouter } from 'next/router'
-import { useMainContext } from './context/MainContext'
+import { useMainContext } from '../context/MainContext'
 import { Link } from 'components/Link'
 
 export type BreadcrumbT = {

components/page-header/Header.module.scss

@@ -0,0 +1,3 @@
+.header {
+  clip-path: inset(-5px -5px -5px 0px);
+}

components/page-header/Header.tsx

@@ -1,4 +1,4 @@
-import { useState } from 'react'
+import { useEffect, useState } from 'react'
 import cx from 'classnames'
 import { useRouter } from 'next/router'
 import { MarkGithubIcon, ThreeBarsIcon, XIcon } from '@primer/octicons-react'
@@ -11,12 +11,15 @@ import { ProductPicker } from 'components/page-header/ProductPicker'
 import { useTranslation } from 'components/hooks/useTranslation'
 import { Search } from 'components/Search'
 import { VersionPicker } from 'components/VersionPicker'
+import { Breadcrumbs } from './Breadcrumbs'
+import styles from './Header.module.scss'
 
 export const Header = () => {
   const router = useRouter()
   const { relativePath, currentLayoutName, error } = useMainContext()
   const { t } = useTranslation(['header', 'homepage'])
   const [isMenuOpen, setIsMenuOpen] = useState(false)
+  const [scroll, setScroll] = useState(false)
 
   // the graphiql explorer utilizes `?query=` in the url and we don't want our search bar to mess that up
   const updateSearchParams = router.asPath !== 'graphql/overview/explorer'
@@ -26,16 +29,38 @@ export const Header = () => {
     currentLayoutName === 'product-sublanding' ||
     currentLayoutName === 'release-notes'
 
+  useEffect(() => {
+    function onScroll() {
+      setScroll(window.scrollY > 10)
+    }
+    window.addEventListener('scroll', onScroll)
+    return () => {
+      window.removeEventListener('scroll', onScroll)
+    }
+  }, [])
+
   return (
-    <div className="border-bottom color-border-muted no-print">
+    <div
+      className={`${
+        scroll
+          ? cx(
+              styles.header,
+              'border-bottom color-border-muted no-print position-sticky top-0 z-3 color-shadow-medium color-bg-default'
+            )
+          : 'border-bottom color-border-muted no-print position-sticky top-0 z-3 color-shadow-small'
+      }`}
+    >
       {error !== '404' && <HeaderNotifications />}
 
-      <header className={cx('container-xl px-3 px-md-6 pt-3 pb-3 position-relative z-3')}>
+      <header className={cx('container-xl px-3 px-md-6 pt-3 pb-3 z-3')}>
         {/* desktop header */}
         <div
           className="d-none d-lg-flex flex-justify-end flex-items-center"
           data-testid="desktop-header"
         >
+          <div className="mr-auto">
+            <Breadcrumbs />
+          </div>
           {showVersionPicker && (
             <div className="mr-2">
               <VersionPicker hideLabel={true} variant="compact" />
@@ -89,6 +114,9 @@ export const Header = () => {
               )}
             >
               <div className="mt-3 mb-2">
+                <div className="mb-3 ml-2">
+                  <Breadcrumbs />
+                </div>
                 <h4 className="f5 text-normal color-fg-muted ml-3">{t('explore_by_product')}</h4>
 
                 <ProductPicker />

components/sublanding/SubLandingHero.tsx

@@ -1,5 +1,4 @@
 import cx from 'classnames'
-import { Breadcrumbs } from '../Breadcrumbs'
 import { useProductSubLandingContext } from 'components/context/ProductSubLandingContext'
 import { ArrowRightIcon, StarFillIcon } from '@primer/octicons-react'
 import { useTranslation } from 'components/hooks/useTranslation'
@@ -46,7 +45,6 @@ export const SubLandingHero = () => {
     <div>
       <header className="d-flex gutter mb-6">
         <div className="col-12">
-          <Breadcrumbs />
           <h1 className="my-3">{title} guides</h1>
           {intro && <Lead>{intro}</Lead>}
         </div>

pages/[versionId]/actions/automating-builds-and-tests/building-and-testing-nodejs-or-python.tsx

@@ -63,7 +63,7 @@ function PageInner() {
         </article>
 
         <div className="col-6">
-          <div className="fix position-sticky top-0 mt-3">
+          <div className="fix position-sticky mt-3" style={{ top: '6.5em' }}>
             <div className="d-flex flex-justify-between flex-items-center mb-3">
               <CodeLanguagePicker variant="tabs" />
               <div className="flash">

pages/[versionId]/graphql/overview/explorer.tsx

@@ -1,7 +1,6 @@
 import { GetServerSideProps } from 'next'
 
 import { MainContextT, MainContext, getMainContext } from 'components/context/MainContext'
-import { Breadcrumbs } from 'components/Breadcrumbs'
 import { DefaultLayout } from 'components/DefaultLayout'
 import { useEffect, useRef } from 'react'
 
@@ -23,8 +22,6 @@ export default function GQLExplorer({ mainContext, graphqlExplorerUrl }: Props)
     <MainContext.Provider value={mainContext}>
       <DefaultLayout>
         <div className="container-xl px-3 px-md-6 my-4 my-lg-4">
-          <Breadcrumbs />
-
           <h1>{page.title}</h1>
         </div>
 

tests/rendering/breadcrumbs.js

@@ -3,14 +3,14 @@ import { jest } from '@jest/globals'
 
 const describeInternalOnly =
   process.env.GITHUB_REPOSITORY === 'github/docs-internal' ? describe : describe.skip
-
+// Breadcrumbs were moved to the Header and in the Menu for mobile, so there are now double the Breadcrumbs
 describe('breadcrumbs', () => {
   jest.setTimeout(300 * 1000)
 
   describe('rendering', () => {
     test('top-level product pages have breadcrumbs', async () => {
       const $ = await getDOM('/github')
-      expect($('[data-testid=breadcrumbs]')).toHaveLength(1)
+      expect($('[data-testid=breadcrumbs]')).toHaveLength(2)
     })
 
     test('article pages have breadcrumbs with product, category, maptopic, and article', async () => {
@@ -19,7 +19,7 @@ describe('breadcrumbs', () => {
       )
       const $breadcrumbs = $('[data-testid=breadcrumbs] a')
 
-      expect($breadcrumbs).toHaveLength(4)
+      expect($breadcrumbs).toHaveLength(8)
       expect($breadcrumbs[0].attribs.title).toBe('product: Account and profile')
       expect($breadcrumbs[1].attribs.title).toBe('category: User accounts')
       expect($breadcrumbs[2].attribs.title).toBe('mapTopic: Manage email preferences')
@@ -32,7 +32,7 @@ describe('breadcrumbs', () => {
       )
       const $breadcrumbs = $('[data-testid=breadcrumbs] a')
 
-      expect($breadcrumbs).toHaveLength(3)
+      expect($breadcrumbs).toHaveLength(6)
       expect($breadcrumbs[0].attribs.title).toBe('product: Account and profile')
       expect($breadcrumbs[1].attribs.title).toBe('category: User accounts')
       expect($breadcrumbs[2].attribs.title).toBe('mapTopic: Manage email preferences')
@@ -44,7 +44,7 @@ describe('breadcrumbs', () => {
         '/en/enterprise-server/account-and-profile/setting-up-and-managing-your-github-user-account/managing-email-preferences/adding-an-email-address-to-your-github-account'
       )
       const $breadcrumbs = $('[data-testid=breadcrumbs] a')
-      expect($breadcrumbs).toHaveLength(4)
+      expect($breadcrumbs).toHaveLength(8)
       expect($breadcrumbs[0].attribs.title).toBe('product: Account and profile')
     })
 
@@ -53,7 +53,7 @@ describe('breadcrumbs', () => {
         '/enterprise-cloud@latest/billing/managing-billing-for-your-github-account/about-billing-for-your-enterprise'
       )
       const $breadcrumbs = $('[data-testid=breadcrumbs] a')
-      expect($breadcrumbs).toHaveLength(3)
+      expect($breadcrumbs).toHaveLength(6)
       expect($breadcrumbs[0].attribs.title).toBe('product: Billing and payments')
     })
 
@@ -63,7 +63,7 @@ describe('breadcrumbs', () => {
         '/en/github-cli/github-cli/about-github-cli'
       )
       const $breadcrumbs = $('[data-testid=breadcrumbs] a')
-      expect($breadcrumbs).toHaveLength(3)
+      expect($breadcrumbs).toHaveLength(6)
       expect($breadcrumbs[0].attribs.title).toBe('product: GitHub CLI')
       expect($breadcrumbs[1].attribs.title).toBe('category: GitHub CLI')
       expect($breadcrumbs[2].attribs.title).toBe('article: About GitHub CLI')
@@ -72,7 +72,7 @@ describe('breadcrumbs', () => {
     test('parses Liquid variables inside titles', async () => {
       const $ = await getDOM('/en/enterprise/admin/enterprise-support')
       const $breadcrumbs = $('[data-testid=breadcrumbs] a')
-      expect($breadcrumbs).toHaveLength(2)
+      expect($breadcrumbs).toHaveLength(4)
       expect($breadcrumbs[1].attribs.title).toBe('category: Working with support')
     })
 
@@ -92,7 +92,7 @@ describe('breadcrumbs', () => {
   describeInternalOnly('early access rendering', () => {
     test('top-level product pages have breadcrumbs', async () => {
       const $ = await getDOM('/early-access/github/articles/using-gist-playground')
-      expect($('[data-testid=breadcrumbs]')).toHaveLength(1)
+      expect($('[data-testid=breadcrumbs]')).toHaveLength(2)
     })
 
     test('early access article pages have breadcrumbs with product, category, and article', async () => {
@@ -102,8 +102,8 @@ describe('breadcrumbs', () => {
       const $breadcrumbTitles = $('[data-testid=breadcrumbs] [data-testid=breadcrumb-title]')
       const $breadcrumbLinks = $('[data-testid=breadcrumbs] a')
 
-      expect($breadcrumbTitles).toHaveLength(2)
-      expect($breadcrumbLinks).toHaveLength(2)
+      expect($breadcrumbTitles).toHaveLength(4)
+      expect($breadcrumbLinks).toHaveLength(4)
       expect($breadcrumbTitles[0].children[0].data).toBe('Early Access documentation')
       expect($breadcrumbTitles[1].children[0].data).toBe('GitHub')
       expect($breadcrumbLinks[0].attribs.title).toBe(