A11y simpler release notes sidebar (#39392)

Co-authored-by: Peter Bengtsson <peterbe@github.com>

components/hooks/useOnScreen.ts

@@ -1,24 +0,0 @@
-import { useState, useEffect, MutableRefObject, RefObject } from 'react'
-
-export function useOnScreen<T extends Element>(
-  ref: MutableRefObject<T | undefined> | RefObject<T>,
-  options?: IntersectionObserverInit,
-): boolean {
-  const [isIntersecting, setIntersecting] = useState(false)
-  useEffect(() => {
-    let isMounted = true
-    const observer = new IntersectionObserver(([entry]) => {
-      isMounted && setIntersecting(entry.isIntersecting)
-    }, options)
-
-    if (ref.current) {
-      observer.observe(ref.current)
-    }
-
-    return () => {
-      isMounted = false
-      ref.current && observer.unobserve(ref.current)
-    }
-  }, [Object.values(options || {}).join(',')])
-  return isIntersecting
-}

components/release-notes/GHAEReleaseNotePatch.tsx

@@ -1,34 +1,25 @@
-import { useRef, useEffect } from 'react'
+import { useRef } from 'react'
 import dayjs from 'dayjs'
 import cx from 'classnames'
 
 import { useTranslation } from 'components/hooks/useTranslation'
-import { useOnScreen } from 'components/hooks/useOnScreen'
 import { PatchNotes } from './PatchNotes'
 import { CurrentVersion, ReleaseNotePatch } from './types'
 
 import styles from './PatchNotes.module.scss'
 
-type Props = { patch: ReleaseNotePatch; currentVersion: CurrentVersion; didEnterView: () => void }
-export function GHAEReleaseNotePatch({ patch, currentVersion, didEnterView }: Props) {
+type Props = { patch: ReleaseNotePatch; currentVersion: CurrentVersion }
+export function GHAEReleaseNotePatch({ patch, currentVersion }: Props) {
   const { t } = useTranslation('release_notes')
   const containerRef = useRef<HTMLDivElement>(null)
-  const onScreen = useOnScreen(containerRef, { rootMargin: '-40% 0px -50%' })
-  useEffect(() => {
-    if (onScreen) {
-      didEnterView()
-    }
-  }, [onScreen])
-
   const bannerText = t('banner_text')
 
   return (
-    <div
-      ref={containerRef}
-      className={cx(styles.sectionHeading, 'mb-10 pb-6 border-bottom border-top')}
-      id={patch.release}
-    >
-      <header style={{ zIndex: 1 }} className="container-xl border-bottom px-3 pt-4 pb-2">
+    <div ref={containerRef} className={cx(styles.sectionHeading, 'mb-10 pb-6')} id={patch.release}>
+      <header
+        style={{ zIndex: 1, marginTop: -1 }}
+        className="container-xl border-top border-bottom px-3 pt-4 pb-2"
+      >
         <div className="d-flex flex-items-center">
           <h2 className="border-bottom-0 m-0 p-0">
             {currentVersion.versionTitle} {patch.release}

components/release-notes/GHAEReleaseNotes.tsx

@@ -1,95 +1,60 @@
-import { useState } from 'react'
 import cx from 'classnames'
-import dayjs from 'dayjs'
-import { GHAEReleaseNotePatch } from './GHAEReleaseNotePatch'
-import { GHAEReleaseNotesContextT } from './types'
+
+import { Link } from 'components/Link'
 import { MarkdownContent } from 'components/ui/MarkdownContent'
+import { GHAEReleaseNotesContextT } from './types'
+import { GHAEReleaseNotePatch } from './GHAEReleaseNotePatch'
 
 import styles from './PatchNotes.module.scss'
 
-type GitHubAEProps = {
+type Props = {
   context: GHAEReleaseNotesContextT
 }
-export function GHAEReleaseNotes({ context }: GitHubAEProps) {
-  const { releaseNotes, releases, currentVersion } = context
-  const [focusedPatch, setFocusedPatch] = useState('')
+
+export function GHAEReleaseNotes({ context }: Props) {
+  const { currentVersion, releaseNotes, releases } = context
 
   return (
-    <div className="d-flex">
-      <article className="min-width-0 flex-1">
-        <div className="d-flex flex-items-center flex-justify-between color-bg-default px-5 py-2">
-          <h1 id="title-h1" className="f4 py-3 m-0">
-            {currentVersion.planTitle} release notes
-          </h1>
-        </div>
+    <>
+      <h1 id="title-h1" className="f4 p-3 m-0 border-bottom">
+        {currentVersion.planTitle} release notes
+      </h1>
 
-        <MarkdownContent data-search="article-body">
-          {releaseNotes.map((patch) => {
-            return (
-              <GHAEReleaseNotePatch
-                key={patch.version}
-                patch={patch}
-                currentVersion={currentVersion}
-                didEnterView={() => setFocusedPatch(patch.version)}
-              />
-            )
-          })}
-        </MarkdownContent>
-      </article>
-
-      <aside
-        className={cx(
-          'position-sticky d-none d-md-block border-left no-print color-bg-default flex-shrink-0',
-          styles.aside,
+      <div className="d-md-flex flex-md-row-reverse">
+        {releases && (
+          <aside
+            className={cx('position-sticky border-md-left no-print flex-shrink-0', styles.aside)}
+          >
+            <nav className="height-full overflow-auto">
+              <ul className="list-style-none py-2 px-0 my-0">
+                {releases.map((release) => {
+                  return (
+                    <li key={release.version} className="my-2 px-3 f4 d-inline-block d-md-block">
+                      <Link className="text-underline" href={`#${release.version}`}>
+                        {release.version}
+                      </Link>
+                    </li>
+                  )
+                })}
+              </ul>
+            </nav>
+          </aside>
         )}
-      >
-        <nav className="height-full overflow-auto">
-          <MarkdownContent>
-            <ul className="list-style-none pl-0 text-bold">
-              {releases.map((release) => {
-                return (
-                  <CollapsibleReleaseSection
-                    key={release.version}
-                    release={release}
-                    focusedPatch={focusedPatch}
-                  />
-                )
-              })}
-            </ul>
-          </MarkdownContent>
-        </nav>
-      </aside>
-    </div>
-  )
-}
 
-const CollapsibleReleaseSection = ({
-  release,
-  focusedPatch,
-}: {
-  release: GHAEReleaseNotesContextT['releases'][0]
-  focusedPatch: string
-}) => {
-  return (
-    <li key={release.version} className="border-bottom">
-      <ul className="list-style-none py-4 px-0 my-0">
-        {release.patches.map((patch) => {
-          const isActive = patch.release === focusedPatch
-          return (
-            <li key={patch.release} className={cx('px-3 my-0', isActive && 'color-bg-accent')}>
-              <a
-                href={`#${patch.release}`}
-                className="d-flex flex-items-center flex-justify-between"
-              >
-                {patch.release}
-                <span className="color-fg-muted text-mono text-small text-normal">
-                  {dayjs(patch.date).format('MMMM DD, YYYY')}
-                </span>
-              </a>
-            </li>
-          )
-        })}
-      </ul>
-    </li>
+        <article className="flex-1 flex-shrink-0">
+          <MarkdownContent data-search="article-body">
+            {releaseNotes.map((patch) => {
+              return (
+                <GHAEReleaseNotePatch
+                  key={patch.version}
+                  patch={patch}
+                  currentVersion={currentVersion}
+                />
+              )
+            })}
+          </MarkdownContent>
+        </article>
+      </div>
+    </>
   )
 }

components/release-notes/GHESReleaseNotePatch.tsx

@@ -1,4 +1,4 @@
-import { useEffect, useRef } from 'react'
+import { useRef } from 'react'
 import dayjs from 'dayjs'
 import cx from 'classnames'
 
@@ -6,7 +6,6 @@ import { useTranslation } from 'components/hooks/useTranslation'
 import { PatchNotes } from './PatchNotes'
 import { Link } from 'components/Link'
 import { CurrentVersion, ReleaseNotePatch, GHESMessage } from './types'
-import { useOnScreen } from 'components/hooks/useOnScreen'
 
 import styles from './PatchNotes.module.scss'
 
@@ -16,7 +15,6 @@ type Props = {
   latestPatch: string
   latestRelease: string
   message: GHESMessage
-  didEnterView: () => void
 }
 export function GHESReleaseNotePatch({
   patch,
@@ -24,24 +22,16 @@ export function GHESReleaseNotePatch({
   latestPatch,
   latestRelease,
   message,
-  didEnterView,
 }: Props) {
   const { t } = useTranslation('header')
   const containerRef = useRef<HTMLDivElement>(null)
-  const onScreen = useOnScreen(containerRef, { rootMargin: '-40% 0px -50%' })
-  useEffect(() => {
-    if (onScreen) {
-      didEnterView()
-    }
-  }, [onScreen])
 
   return (
-    <div
-      ref={containerRef}
-      className={cx(styles.sectionHeading, 'mb-10 pb-6 border-bottom border-top')}
-      id={patch.version}
-    >
-      <header style={{ zIndex: 1 }} className="container-xl border-bottom px-3 pt-4 pb-2">
+    <div ref={containerRef} className={cx(styles.sectionHeading, 'mb-10 pb-6')} id={patch.version}>
+      <header
+        style={{ zIndex: 1, marginTop: -1 }}
+        className="container-xl border-top border-bottom px-3 pt-4 pb-2"
+      >
         <div className="d-flex flex-justify-between flex-wrap">
           <h2 className="border-bottom-0 m-0 p-0 mt-2">
             {currentVersion.versionTitle}.{patch.patchVersion}
@@ -101,7 +91,7 @@ export function GHESReleaseNotePatch({
       <div className="container-xl px-3">
         <div className="mt-3" dangerouslySetInnerHTML={{ __html: patch.intro }} />
 
-        <PatchNotes patch={patch} withReleaseNoteLabel />
+        <PatchNotes patch={patch} />
       </div>
     </div>
   )

components/release-notes/GHESReleaseNotes.tsx

@@ -1,9 +1,4 @@
-import { SyntheticEvent, useState } from 'react'
 import cx from 'classnames'
-import { ChevronDownIcon, LinkExternalIcon } from '@primer/octicons-react'
-import { useMainContext } from 'components/context/MainContext'
-import dayjs from 'dayjs'
-import { useRouter } from 'next/router'
 
 import { Link } from 'components/Link'
 import { MarkdownContent } from 'components/ui/MarkdownContent'
@@ -11,167 +6,62 @@ import { GHESReleaseNotesContextT } from './types'
 import { GHESReleaseNotePatch } from './GHESReleaseNotePatch'
 
 import styles from './PatchNotes.module.scss'
-import { PlainLink } from './PlainLink'
 
 type Props = {
   context: GHESReleaseNotesContextT
 }
 
 export function GHESReleaseNotes({ context }: Props) {
-  const router = useRouter()
-  const { currentProduct } = useMainContext()
-  const [focusedPatch, setFocusedPatch] = useState('')
   const { latestPatch, latestRelease, currentVersion, releaseNotes, releases, message } = context
+
+  const currentRelease = releases.find(
+    (release) => release.version === currentVersion.currentRelease,
+  )
+
   return (
-    <div className="d-flex">
-      <article className="min-width-0 flex-1">
-        <div className="d-flex flex-items-center flex-justify-center color-bg-default text-bold px-5 py-2">
-          <h1 id="title-h1" className="f4 py-3 m-0">
-            {currentVersion.planTitle} {currentVersion.currentRelease} release notes
-          </h1>
-        </div>
-        <MarkdownContent data-search="article-body">
-          {releaseNotes.map((patch) => {
-            return (
-              <GHESReleaseNotePatch
-                key={patch.version}
-                patch={patch}
-                currentVersion={currentVersion}
-                latestPatch={latestPatch}
-                latestRelease={latestRelease}
-                message={message}
-                didEnterView={() => {
-                  setFocusedPatch(patch.version)
-                }}
-              />
-            )
-          })}
-        </MarkdownContent>
-      </article>
+    <>
+      <h1 id="title-h1" className="f4 p-3 m-0 border-bottom">
+        {currentVersion.planTitle} {currentVersion.currentRelease} release notes
+      </h1>
 
-      <aside
-        className={cx(
-          'position-sticky d-none d-md-block border-left no-print color-bg-default flex-shrink-0',
-          styles.aside,
-        )}
-      >
-        <nav className="height-full overflow-auto">
-          <MarkdownContent>
-            <ul className="list-style-none pl-0 text-bold">
-              {releases.map((release) => {
-                const releaseLink = `/${router.locale}/${currentVersion.plan}@${release.version}/${currentProduct?.id}/release-notes`
-
-                // Use client-side router link component only if it's a supported release.
-                // Otherwise, it will trigger a NextJS data XHR fetch for releases
-                // that are deprecated when in fact you should load it regularly
-                // so it's read as a proxy from the archive.
-                const LinkComponent = currentVersion.releases.includes(release.version)
-                  ? Link
-                  : PlainLink
-
-                if (!release.patches || release.patches.length === 0) {
+      <div className="d-md-flex flex-md-row-reverse">
+        {currentRelease && (
+          <aside
+            className={cx('position-sticky border-md-left no-print flex-shrink-0', styles.aside)}
+          >
+            <nav className="height-full overflow-auto">
+              <ul className="list-style-none py-2 px-0 my-0">
+                {currentRelease.patches.map((patch) => {
                   return (
-                    <li key={release.version} className="border-bottom">
-                      <LinkComponent
-                        href={releaseLink}
-                        className="Link--primary no-underline px-3 py-4 my-0 d-flex flex-items-center flex-justify-between"
-                      >
-                        {release.version}
-                        <LinkExternalIcon aria-label="(external site)" />
-                      </LinkComponent>
+                    <li key={patch.version} className="my-2 px-3 f4 d-inline-block d-md-block">
+                      <Link className="text-underline" href={`#${patch.version}`}>
+                        {patch.version}
+                      </Link>
                     </li>
                   )
-                }
+                })}
+              </ul>
+            </nav>
+          </aside>
+        )}
 
-                if (release.version === currentVersion.currentRelease) {
-                  return (
-                    <CollapsibleReleaseSection
-                      key={release.version}
-                      release={release}
-                      focusedPatch={focusedPatch}
-                      releaseLink={releaseLink}
-                    />
-                  )
-                }
-
-                return (
-                  <li key={release.version} className="border-bottom">
-                    <LinkComponent
-                      className="px-3 py-4 my-0 d-flex flex-items-center flex-justify-between"
-                      href={releaseLink}
-                    >
-                      {release.version}
-                      <span className="color-fg-muted text-small text-normal mr-1">
-                        {release.patches.length}{' '}
-                        {release.patches.length === 1 ? 'release' : 'releases'}
-                      </span>
-                    </LinkComponent>
-                  </li>
-                )
-              })}
-            </ul>
+        <article className="flex-1 flex-shrink-0">
+          <MarkdownContent data-search="article-body">
+            {releaseNotes.map((patch) => {
+              return (
+                <GHESReleaseNotePatch
+                  key={patch.version}
+                  patch={patch}
+                  currentVersion={currentVersion}
+                  latestPatch={latestPatch}
+                  latestRelease={latestRelease}
+                  message={message}
+                />
+              )
+            })}
           </MarkdownContent>
-        </nav>
-      </aside>
-    </div>
-  )
-}
-
-const CollapsibleReleaseSection = ({
-  release,
-  releaseLink,
-  focusedPatch,
-}: {
-  release: GHESReleaseNotesContextT['releases'][0]
-  releaseLink: string
-  focusedPatch: string
-}) => {
-  const defaultIsOpen = true
-  const [isOpen, setIsOpen] = useState(defaultIsOpen)
-
-  const onToggle = (e: SyntheticEvent) => {
-    const newIsOpen = (e.target as HTMLDetailsElement).open
-    setIsOpen(newIsOpen)
-  }
-  return (
-    <li key={release.version} className="border-bottom">
-      <details
-        className="my-0 details-reset release-notes-version-picker"
-        aria-current="page"
-        open={defaultIsOpen}
-        onToggle={onToggle}
-      >
-        <summary className="px-3 py-4 my-0 d-flex flex-items-center flex-justify-between outline-none">
-          {release.version}
-          <div className="d-flex">
-            <span className="color-fg-muted text-small text-normal mr-1">
-              {release.patches.length} {release.patches.length === 1 ? 'release' : 'releases'}
-            </span>
-            <ChevronDownIcon className={isOpen ? 'rotate-180' : ''} />
-          </div>
-        </summary>
-        <ul className="color-bg-subtle border-top list-style-none py-4 px-0 my-0">
-          {release.patches.map((patch) => {
-            const isActive = patch.version === focusedPatch
-            return (
-              <li
-                key={patch.version}
-                className={cx('px-3 my-0 py-1', isActive && 'color-bg-accent')}
-              >
-                <Link
-                  href={`${releaseLink}#${patch.version}`}
-                  className="d-flex flex-items-center flex-justify-between"
-                >
-                  {patch.version}
-                  <span className="color-fg-muted text-mono text-small text-normal">
-                    {dayjs(patch.date).format('MMMM DD, YYYY')}
-                  </span>
-                </Link>
-              </li>
-            )
-          })}
-        </ul>
-      </details>
-    </li>
+        </article>
+      </div>
+    </>
   )
 }

components/release-notes/PatchNotes.module.scss

@@ -1,9 +1,22 @@
+@import "@primer/css/support/index.scss";
+
 .sectionHeading {
-  scroll-margin-top: 70px !important;
+  scroll-margin-top: 105px !important; // 88 + 8 + 8 + 1
+
+  @include breakpoint(xl) {
+    scroll-margin-top: 65px !important; // 48 + 8 + 8 + 1
+  }
 }
 
 .aside {
-  width: 260px;
-  height: calc(100vh - 70px);
-  top: 70px;
+  @include breakpoint(md) {
+    width: 8rem;
+    height: calc(100vh - 105px);
+    top: 105px;
+  }
+
+  @include breakpoint(xl) {
+    height: calc(100vh - 65px);
+    top: 65px;
+  }
 }

components/release-notes/PatchNotes.tsx

@@ -1,4 +1,3 @@
-import { Fragment } from 'react'
 import cx from 'classnames'
 import { slug } from 'github-slugger'
 import { ReleaseNotePatch } from './types'
@@ -19,53 +18,37 @@ const SectionToLabelMap: Record<string, string> = {
 
 type Props = {
   patch: ReleaseNotePatch
-  withReleaseNoteLabel?: boolean
 }
-export function PatchNotes({ patch, withReleaseNoteLabel }: Props) {
+export function PatchNotes({ patch }: Props) {
   return (
     <>
-      {Object.entries(patch.sections).map(([key, sectionItems], i, arr) => {
-        const isLast = i === arr.length - 1
+      {Object.entries(patch.sections).map(([key, sectionItems]) => {
         const sectionSlug = `${patch.version}-${key.replaceAll('_', '-')}`
         return (
-          <div
-            key={key}
-            className={cx(
-              'py-6 d-block d-xl-flex',
-              !withReleaseNoteLabel && 'mx-6',
-              !isLast && 'border-bottom',
-            )}
-          >
+          <div key={key} className={cx('d-block d-xl-flex')}>
             <div>
-              <HeadingLink as="h3" className="pl-4" slug={sectionSlug}>
+              <HeadingLink as="h3" slug={sectionSlug}>
                 {`${patch.version}: ${SectionToLabelMap[key]}` || 'INVALID SECTION'}
               </HeadingLink>
+
               <ul>
                 {sectionItems.map((item, i) => {
                   if (typeof item === 'string') {
-                    return (
-                      <li key={item} className="f4" dangerouslySetInnerHTML={{ __html: item }} />
-                    )
+                    return <li key={item} dangerouslySetInnerHTML={{ __html: item }} />
                   }
 
                   const headingSlug = item.heading ? slug(item.heading) : `heading${i}`
                   return (
-                    <Fragment key={headingSlug}>
-                      <li className="list-style-none">
-                        <h4 id={headingSlug} className={cx(styles.sectionHeading, 'text-bold f4')}>
-                          <a href={`#${headingSlug}`}>{item.heading}</a>
-                        </h4>
-                      </li>
-                      {item.notes.map((note) => {
-                        return (
-                          <li
-                            key={note}
-                            className="f4"
-                            dangerouslySetInnerHTML={{ __html: note }}
-                          />
-                        )
-                      })}
-                    </Fragment>
+                    <li key={headingSlug}>
+                      <h4 id={headingSlug} className={cx(styles.sectionHeading)}>
+                        <a href={`#${headingSlug}`}>{item.heading}</a>
+                      </h4>
+                      <ul>
+                        {item.notes.map((note) => {
+                          return <li key={note} dangerouslySetInnerHTML={{ __html: note }} />
+                        })}
+                      </ul>
+                    </li>
                   )
                 })}
               </ul>