Refactor display-platform-specific-content (#22665)

* refactor display-platform-specific-content * update PlatformPicker tests and cleanup
2021-11-12 08:40:35 -06:00 · 2021-11-12 08:40:35 -06:00 · 7310fc998a
--- a/components/article/ArticlePage.tsx
+++ b/components/article/ArticlePage.tsx
@ -14,6 +14,7 @@ import { LearningTrackNav } from './LearningTrackNav'
 import { MarkdownContent } from 'components/ui/MarkdownContent'
 import { Lead } from 'components/ui/Lead'
 import { ArticleGridLayout } from './ArticleGridLayout'
+import { PlatformPicker } from 'components/article/PlatformPicker'

 // Mapping of a "normal" article to it's interactive counterpart
 const interactiveAlternatives: Record<string, { href: string }> = {
@ -35,7 +36,6 @@ export const ArticlePage = () => {
    contributor,
    permissions,
    includesPlatformSpecificContent,
-    defaultPlatform,
    product,
    miniTocItems,
    currentLearningTrack,
@ -87,35 +87,7 @@ export const ArticlePage = () => {
                </div>
              )}

-              {includesPlatformSpecificContent && (
-                <nav
-                  className="UnderlineNav my-3"
-                  data-default-platform={defaultPlatform || undefined}
-                >
-                  <div className="UnderlineNav-body">
-                    {/* eslint-disable-next-line jsx-a11y/anchor-is-valid */}
-                    <a href="#" className="UnderlineNav-item platform-switcher" data-platform="mac">
-                      Mac
-                    </a>
-                    {/* eslint-disable-next-line jsx-a11y/anchor-is-valid */}
-                    <a
-                      href="#"
-                      className="UnderlineNav-item platform-switcher"
-                      data-platform="windows"
-                    >
-                      Windows
-                    </a>
-                    {/* eslint-disable-next-line jsx-a11y/anchor-is-valid */}
-                    <a
-                      href="#"
-                      className="UnderlineNav-item platform-switcher"
-                      data-platform="linux"
-                    >
-                      Linux
-                    </a>
-                  </div>
-                </nav>
-              )}
+              {includesPlatformSpecificContent && <PlatformPicker variant="underlinenav" />}

              {product && (
                <Callout
--- a/components/article/PlatformPicker.tsx
+++ b/components/article/PlatformPicker.tsx
@ -0,0 +1,163 @@
+import { useEffect, useState } from 'react'
+import Cookies from 'js-cookie'
+import { SubNav, TabNav, UnderlineNav } from '@primer/components'
+import { sendEvent, EventType } from 'components/lib/events'
+
+import { useArticleContext } from 'components/context/ArticleContext'
+import parseUserAgent from 'components/lib/user-agent'
+
+const platforms = [
+  { id: 'mac', label: 'Mac' },
+  { id: 'windows', label: 'Windows' },
+  { id: 'linux', label: 'Linux' },
+]
+
+// Imperatively modify article content to show only the selected platform
+// find all platform-specific *block* elements and hide or show as appropriate
+// example: {% mac } block content {% mac %}
+function showPlatformSpecificContent(platform: string) {
+  const markdowns = Array.from(document.querySelectorAll<HTMLElement>('.extended-markdown'))
+  markdowns
+    .filter((el) => platforms.some((platform) => el.classList.contains(platform.id)))
+    .forEach((el) => {
+      el.style.display = el.classList.contains(platform) ? '' : 'none'
+    })
+
+  // find all platform-specific *inline* elements and hide or show as appropriate
+  // example: <span class="platform-mac">inline content</span>
+  const platformEls = Array.from(
+    document.querySelectorAll<HTMLElement>(
+      platforms.map((platform) => `.platform-${platform.id}`).join(', ')
+    )
+  )
+  platformEls.forEach((el) => {
+    el.style.display = el.classList.contains(`platform-${platform}`) ? '' : 'none'
+  })
+}
+
+// uses the order of the supportedPlatforms array to
+// determine the default platform
+const getFallbackPlatform = (detectedPlatforms: Array<string>): string => {
+  const foundPlatform = platforms.find((platform) => detectedPlatforms.includes(platform.id))
+  return foundPlatform?.id || 'linux'
+}
+
+type Props = {
+  variant?: 'subnav' | 'tabnav' | 'underlinenav'
+}
+export const PlatformPicker = ({ variant = 'subnav' }: Props) => {
+  const { defaultPlatform, detectedPlatforms } = useArticleContext()
+  const [currentPlatform, setCurrentPlatform] = useState(defaultPlatform || '')
+
+  // Run on mount for client-side only features
+  useEffect(() => {
+    let userAgent = parseUserAgent().os
+    if (userAgent === 'ios') {
+      userAgent = 'mac'
+    }
+
+    setCurrentPlatform(defaultPlatform || Cookies.get('osPreferred') || userAgent || 'linux')
+  }, [])
+
+  // Make sure we've always selected a platform that exists in the article
+  useEffect(() => {
+    // Only check *after* current platform has been determined
+    if (currentPlatform && !detectedPlatforms.includes(currentPlatform)) {
+      setCurrentPlatform(getFallbackPlatform(detectedPlatforms))
+    }
+  }, [currentPlatform, detectedPlatforms.join(',')])
+
+  const onClickPlatform = (platform: string) => {
+    setCurrentPlatform(platform)
+
+    // imperatively modify the article content
+    showPlatformSpecificContent(platform)
+
+    sendEvent({
+      type: EventType.preference,
+      preference_name: 'os',
+      preference_value: platform,
+    })
+
+    Cookies.set('osPreferred', platform, {
+      sameSite: 'strict',
+      secure: true,
+    })
+  }
+
+  // only show platforms that are in the current article
+  const platformOptions = platforms.filter((platform) => detectedPlatforms.includes(platform.id))
+
+  const sharedContainerProps = {
+    'data-testid': 'platform-picker',
+    'aria-label': 'Platform picker',
+    'data-default-platform': defaultPlatform,
+    className: 'mb-4',
+  }
+
+  if (variant === 'subnav') {
+    return (
+      <SubNav {...sharedContainerProps}>
+        <SubNav.Links>
+          {platformOptions.map((option) => {
+            return (
+              <SubNav.Link
+                key={option.id}
+                data-platform={option.id}
+                as="button"
+                selected={option.id === currentPlatform}
+                onClick={() => {
+                  onClickPlatform(option.id)
+                }}
+              >
+                {option.label}
+              </SubNav.Link>
+            )
+          })}
+        </SubNav.Links>
+      </SubNav>
+    )
+  }
+
+  if (variant === 'underlinenav') {
+    return (
+      <UnderlineNav {...sharedContainerProps}>
+        {platformOptions.map((option) => {
+          return (
+            <UnderlineNav.Link
+              key={option.id}
+              data-platform={option.id}
+              as="button"
+              selected={option.id === currentPlatform}
+              onClick={() => {
+                onClickPlatform(option.id)
+              }}
+            >
+              {option.label}
+            </UnderlineNav.Link>
+          )
+        })}
+      </UnderlineNav>
+    )
+  }
+
+  return (
+    <TabNav {...sharedContainerProps}>
+      {platformOptions.map((option) => {
+        return (
+          <TabNav.Link
+            key={option.id}
+            data-platform={option.id}
+            as="button"
+            selected={option.id === currentPlatform}
+            onClick={() => {
+              onClickPlatform(option.id)
+            }}
+          >
+            {option.label}
+          </TabNav.Link>
+        )
+      })}
+    </TabNav>
+  )
+}
--- a/components/context/ArticleContext.tsx
+++ b/components/context/ArticleContext.tsx
@ -25,6 +25,7 @@ export type ArticleContextT = {
  defaultPlatform?: string
  product?: string
  currentLearningTrack?: LearningTrack
+  detectedPlatforms: Array<string>
 }

 export const ArticleContext = createContext<ArticleContextT | null>(null)
@ -62,5 +63,6 @@ export const getArticleContextFromRequest = (req: any): ArticleContextT => {
    defaultPlatform: page.defaultPlatform || '',
    product: page.product || '',
    currentLearningTrack: req.context.currentLearningTrack,
+    detectedPlatforms: page.detectedPlatforms || [],
  }
 }
--- a/components/lib/display-platform-specific-content.ts
+++ b/components/lib/display-platform-specific-content.ts
@ -1,132 +0,0 @@
-import Cookies from 'js-cookie'
-import parseUserAgent from './user-agent'
-import { sendEvent, EventType } from './events'
-
-const supportedPlatforms = ['mac', 'windows', 'linux']
-const detectedPlatforms = new Set<string>()
-
-// Emphasize content for the visitor's OS (inferred from user agent string)
-
-export default function displayPlatformSpecificContent() {
-  let platform = getDefaultPlatform() || parseUserAgent().os
-
-  // adjust platform names to fit existing mac/windows/linux scheme
-  if (!platform) platform = 'linux'
-  if (platform === 'ios') platform = 'mac'
-
-  const platformsInContent = getDetectedPlatforms()
-  // when the `defaultPlatform` frontmatter isn't set and the article
-  // does not define all platforms in the content, documentation is hidden
-  // for users with the undefined platform. This sets a default
-  // platform for those users to prevent unintentionally hiding content
-  if (!platformsInContent.includes(platform)) {
-    // uses the order of the supportedPlatforms array to
-    // determine the default platform
-    platform = supportedPlatforms.filter((elem) => platformsInContent.includes(elem))[0]
-  }
-
-  showPlatformSpecificContent(platform)
-
-  hideSwitcherLinks(platformsInContent)
-
-  setActiveSwitcherLinks(platform)
-
-  // configure links for switching platform content
-  switcherLinks().forEach((link) => {
-    link.addEventListener('click', (event) => {
-      event.preventDefault()
-      const target = event.target as HTMLElement
-      setActiveSwitcherLinks(target.dataset.platform || '')
-      showPlatformSpecificContent(target.dataset.platform || '')
-
-      Cookies.set('osPreferred', target.dataset.platform || '', {
-        sameSite: 'strict',
-        secure: true,
-      })
-
-      // Send event data
-      sendEvent({
-        type: EventType.preference,
-        preference_name: 'os',
-        preference_value: target.dataset.platform,
-      })
-    })
-  })
-}
-
-function setActiveSwitcherLinks(platform: string) {
-  // (de)activate switcher link appearances
-  switcherLinks().forEach((link) => {
-    link.dataset.platform === platform
-      ? link.classList.add('selected')
-      : link.classList.remove('selected')
-  })
-}
-
-function showPlatformSpecificContent(platform: string) {
-  // find all platform-specific *block* elements and hide or show as appropriate
-  // example: {{ #mac }} block content {{/mac}}
-  const markdowns = Array.from(document.querySelectorAll<HTMLElement>('.extended-markdown'))
-  markdowns
-    .filter((el) => supportedPlatforms.some((platform) => el.classList.contains(platform)))
-    .forEach((el) => {
-      el.style.display = el.classList.contains(platform) ? '' : 'none'
-    })
-
-  // find all platform-specific *inline* elements and hide or show as appropriate
-  // example: <span class="platform-mac">inline content</span>
-  const platforms = Array.from(
-    document.querySelectorAll<HTMLElement>('.platform-mac, .platform-windows, .platform-linux')
-  )
-  platforms.forEach((el) => {
-    el.style.display = el.classList.contains('platform-' + platform) ? '' : 'none'
-  })
-}
-
-// hide links for any platform-specific sections that are not present
-function hideSwitcherLinks(platformsInContent: Array<string>) {
-  const links = Array.from(
-    document.querySelectorAll('a.platform-switcher')
-  ) as Array<HTMLAnchorElement>
-  links.forEach((link) => {
-    if (platformsInContent.includes(link.dataset.platform || '')) return
-    link.style.display = 'none'
-  })
-}
-
-// gets the list of detected platforms in the current article
-function getDetectedPlatforms(): Array<string> {
-  // find all platform-specific *block* elements and hide or show as appropriate
-  // example: {{ #mac }} block content {{/mac}}
-  const allEls = Array.from(document.querySelectorAll('.extended-markdown')) as Array<HTMLElement>
-  allEls
-    .filter((el) => supportedPlatforms.some((platform) => el.classList.contains(platform)))
-    .forEach((el) => detectPlatforms(el))
-
-  // find all platform-specific *inline* elements and hide or show as appropriate
-  // example: <span class="platform-mac">inline content</span>
-  const platformEls = Array.from(
-    document.querySelectorAll<HTMLElement>('.platform-mac, .platform-windows, .platform-linux')
-  )
-  platformEls.forEach((el) => detectPlatforms(el))
-
-  return Array.from(detectedPlatforms)
-}
-
-function detectPlatforms(el: HTMLElement) {
-  el.classList.forEach((elClass) => {
-    const value = elClass.replace(/platform-/, '')
-    if (supportedPlatforms.includes(value)) detectedPlatforms.add(value)
-  })
-}
-
-function getDefaultPlatform() {
-  if (Cookies.get('osPreferred')) return Cookies.get('osPreferred')
-
-  const el = document.querySelector('[data-default-platform]') as HTMLElement
-  if (el) return el.dataset.defaultPlatform
-}
-
-function switcherLinks(): Array<HTMLAnchorElement> {
-  return Array.from(document.querySelectorAll('a.platform-switcher'))
-}
--- a/lib/page.js
+++ b/lib/page.js
@ -256,6 +256,12 @@ class Page {
      html.includes('extended-markdown windows') ||
      html.includes('extended-markdown linux')

+    this.detectedPlatforms = [
+      html.includes('extended-markdown mac') && 'mac',
+      html.includes('extended-markdown windows') && 'windows',
+      html.includes('extended-markdown linux') && 'linux',
+    ].filter(Boolean)
+
    return html
  }

--- a/pages/[versionId]/[productId]/index.tsx
+++ b/pages/[versionId]/[productId]/index.tsx
@ -4,7 +4,6 @@ import { useRouter } from 'next/router'
 // "legacy" javascript needed to maintain existing functionality
 // typically operating on elements **within** an article.
 import copyCode from 'components/lib/copy-code'
-import displayPlatformSpecificContent from 'components/lib/display-platform-specific-content'
 import displayToolSpecificContent from 'components/lib/display-tool-specific-content'
 import localization from 'components/lib/localization'
 import wrapCodeTerms from 'components/lib/wrap-code-terms'
@ -41,7 +40,6 @@ import { useEffect } from 'react'

 function initiateArticleScripts() {
  copyCode()
-  displayPlatformSpecificContent()
  displayToolSpecificContent()
  localization()
  wrapCodeTerms()
--- a/tests/browser/browser.js
+++ b/tests/browser/browser.js
@ -181,7 +181,7 @@ describe('csrf meta', () => {
  })
 })

-describe('platform specific content', () => {
+describe('platform picker', () => {
  // from tests/javascripts/user-agent.js
  const userAgents = [
    {
@ -201,27 +201,27 @@ describe('platform specific content', () => {
    },
  ]
  const linuxUserAgent = userAgents[2]
-  const pageWithSwitcher =
+  const pageWithPlatformPicker =
    'http://localhost:4001/en/github/using-git/configuring-git-to-handle-line-endings'
-  const pageWithoutSwitcher = 'http://localhost:4001/en/github/using-git'
+  const pageWithoutPlatformPicker = 'http://localhost:4001/en/github/using-git'
  const pageWithDefaultPlatform =
    'http://localhost:4001/en/actions/hosting-your-own-runners/configuring-the-self-hosted-runner-application-as-a-service'

-  it('should have a platform switcher', async () => {
-    await page.goto(pageWithSwitcher)
-    const nav = await page.$$('nav.UnderlineNav')
-    const switches = await page.$$('a.platform-switcher')
-    const selectedSwitch = await page.$$('a.platform-switcher.selected')
+  it('should have a platform picker', async () => {
+    await page.goto(pageWithPlatformPicker)
+    const nav = await page.$$('[data-testid=platform-picker]')
+    const switches = await page.$$('[data-testid=platform-picker] button')
+    const selectedSwitch = await page.$$('[data-testid=platform-picker] .selected')
    expect(nav).toHaveLength(1)
    expect(switches.length).toBeGreaterThan(1)
    expect(selectedSwitch).toHaveLength(1)
  })

-  it('should NOT have a platform switcher', async () => {
-    await page.goto(pageWithoutSwitcher)
-    const nav = await page.$$('nav.UnderlineNav')
-    const switches = await page.$$('a.platform-switcher')
-    const selectedSwitch = await page.$$('a.platform-switcher.selected')
+  it('should NOT have a platform picker', async () => {
+    await page.goto(pageWithoutPlatformPicker)
+    const nav = await page.$$('[data-testid=platform-picker]')
+    const switches = await page.$$('[data-testid=platform-picker] button')
+    const selectedSwitch = await page.$$('[data-testid=platform-picker] .selected')
    expect(nav).toHaveLength(0)
    expect(switches).toHaveLength(0)
    expect(selectedSwitch).toHaveLength(0)
@ -230,10 +230,15 @@ describe('platform specific content', () => {
  it('should detect platform from user agent', async () => {
    for (const agent of userAgents) {
      await page.setUserAgent(agent.ua)
-      await page.goto(pageWithSwitcher)
-      const selectedPlatformElement = await page.waitForSelector('a.platform-switcher.selected')
-      const selectedPlatform = await page.evaluate((el) => el.textContent, selectedPlatformElement)
-      expect(selectedPlatform).toBe(agent.name)
+      await page.goto(pageWithPlatformPicker)
+      const selectedPlatformElement = await page.waitForSelector(
+        '[data-testid=platform-picker] .selected'
+      )
+      const selectedPlatform = await page.evaluate(
+        (el) => el.dataset.platform,
+        selectedPlatformElement
+      )
+      expect(selectedPlatform).toBe(agent.name.toLowerCase())
    }
  })

@ -245,7 +250,9 @@ describe('platform specific content', () => {
        '[data-default-platform]',
        (el) => el.dataset.defaultPlatform
      )
-      const selectedPlatformElement = await page.waitForSelector('a.platform-switcher.selected')
+      const selectedPlatformElement = await page.waitForSelector(
+        '[data-testid=platform-picker] .selected'
+      )
      const selectedPlatform = await page.evaluate((el) => el.textContent, selectedPlatformElement)
      expect(defaultPlatform).toBe(linuxUserAgent.id)
      expect(selectedPlatform).toBe(linuxUserAgent.name)
@ -253,17 +260,17 @@ describe('platform specific content', () => {
  })

  it('should show the content for the selected platform only', async () => {
-    await page.goto(pageWithSwitcher)
+    await page.goto(pageWithPlatformPicker)

    const platforms = ['mac', 'windows', 'linux']
    for (const platform of platforms) {
-      await page.click(`.platform-switcher[data-platform="${platform}"]`)
+      await page.click(`[data-testid=platform-picker] [data-platform=${platform}]`)

      // content for selected platform is expected to become visible
      await page.waitForSelector(`.extended-markdown.${platform}`, { visible: true, timeout: 3000 })

      // only a single tab should be selected
-      const selectedSwitch = await page.$$('a.platform-switcher.selected')
+      const selectedSwitch = await page.$$('[data-testid=platform-picker] .selected')
      expect(selectedSwitch).toHaveLength(1)

      // content for NOT selected platforms is expected to become hidden
--- a/tsconfig.json
+++ b/tsconfig.json
@ -1,7 +1,11 @@
 {
  "compilerOptions": {
    "target": "es5",
-    "lib": ["dom", "dom.iterable", "esnext"],
+    "lib": [
+      "dom",
+      "dom.iterable",
+      "esnext"
+    ],
    "allowJs": true,
    "skipLibCheck": true,
    "strict": true,
@ -15,8 +19,15 @@
    "jsx": "preserve",
    "baseUrl": ".",
    "noEmit": true,
-    "allowSyntheticDefaultImports": true
+    "allowSyntheticDefaultImports": true,
+    "incremental": true
  },
-  "exclude": ["node_modules"],
-  "include": ["*.d.ts", "**/*.ts", "**/*.tsx"]
+  "exclude": [
+    "node_modules"
+  ],
+  "include": [
+    "*.d.ts",
+    "**/*.ts",
+    "**/*.tsx"
+  ]
 }