Merge remote-tracking branch 'private/main'

Author: heiskr

.github/workflows/sync-audit-logs.yml

@@ -51,34 +51,38 @@ jobs:
           # Needed for gh
           GITHUB_TOKEN: ${{ secrets.DOCS_BOT_PAT_BASE }}
         run: |
-          # If nothing to commit, exit now. It's fine. No orphans.
-          changes=$(git diff --name-only | wc -l)
-          untracked=$(git status --untracked-files --short | wc -l)
-          filesChanged=$(git diff --name-only)
-          # There will always be at least one file changed:
-          # src/audit-logs/lib/config.json
-          # If the config file is the only file changed, exit.
-          if [[ $changes -eq 1 ]] && [[ $untracked -eq 1 ]] && [[ $filesChanged == *lib/config.json ]]; then
-            echo "There are no changes to commit or untracked files. Exiting..."
+          echo "Creating a new branch if needed..."
+          branchname=audit-logs-schema-update-${{ steps.audit-log-allowlists.outputs.COMMIT_SHA }}
+          remotesha=$(git ls-remote --heads origin $branchname)
+          if [ -n "$remotesha" ]; then
+            # output is not empty, it means the remote branch exists
+            echo "Branch $branchname already exists in 'github/docs-internal'. Exiting..."
             exit 0
           fi
+          git checkout -b $branchname
+          echo "Created a new branch $branchname"
 
+          echo "Preparing commit..."
           git config --global user.name "docs-bot"
           git config --global user.email "77750099+docs-bot@users.noreply.github.com"
+          git add -A .
+          echo "Prepared commit"
 
-          branchname=audit-logs-schema-update-${{ steps.audit-log-allowlists.outputs.COMMIT_SHA }}
-
-          remotesha=$(git ls-remote --heads origin $branchname)
-          if [ -n "$remotesha" ]; then
-            # output is not empty, it means the remote branch exists
-            echo "Branch $branchname already exists in 'github/docs-internal'. Exiting..."
+          echo "Check if there are changes..."
+          if git diff-index --cached --quiet HEAD -- . ':(exclude)src/audit-logs/lib/config.json'
+          then
+            echo "No real changes (only the SHA in config.json moved). Exiting…"
             exit 0
           fi
+          echo "Changes detected, proceeding"
 
-          git checkout -b $branchname
-          git add .
+          echo "Creating commit..."
           git commit -m "Add updated audit log event data"
+          echo "Created commit"
+
+          echo "Pushing commit..."
           git push origin $branchname
+          echo "Pushed commit"
 
           echo "Creating pull request..."
           gh pr create \
@@ -89,16 +93,21 @@ jobs:
             --repo github/docs-internal \
             --label audit-log-pipeline \
             --head=$branchname
+          echo "Created pull request"
 
           # can't approve your own PR, approve with Actions
+          echo "Approving pull request..."
           unset GITHUB_TOKEN
           gh auth login --with-token <<< "${{ secrets.GITHUB_TOKEN }}"
           gh pr review --approve
+          echo "Approved pull request"
 
           # Actions can't merge the PR so back to docs-bot to merge the PR
+          echo "Setting pull request to auto merge..."
           unset GITHUB_TOKEN
           gh auth login --with-token <<< "${{ secrets.DOCS_BOT_PAT_BASE }}"
           gh pr merge --auto
+          echo "Set pull request to auto merge"
 
       - uses: ./.github/actions/slack-alert
         if: ${{ failure() && github.event_name != 'workflow_dispatch' }}

.github/workflows/sync-openapi.yml

@@ -49,6 +49,7 @@ jobs:
           repository: github/models-gateway
           path: models-gateway
           ref: main
+          token: ${{ secrets.DOCS_BOT_PAT_BASE }}
 
       - uses: ./.github/actions/node-npm-setup
 

data/reusables/contributing/content-linter-rules.md

@@ -65,4 +65,4 @@
 | GHD041 | third-party-action-pinning | Code examples that use third-party actions must always pin to a full length commit SHA | error | feature, actions |
 | GHD042 | liquid-tag-whitespace | Liquid tags should start and end with one whitespace. Liquid tag arguments should be separated by only one whitespace. | error | liquid, format |
 | GHD043 | link-quotation | Internal link titles must not be surrounded by quotations | error | links, url |
-| GHD022 | liquid-ifversion-versions | Liquid `ifversion`, `elsif`, and `else` tags should be valid and not contain unsupported versions. | error | liquid, versioning |
+| GHD044 | octicon-aria-labels | Octicons should always have an aria-label attribute even if aria-hidden. | warning | accessibility, octicons |

src/content-linter/lib/linting-rules/index.js

@@ -33,6 +33,7 @@ import { tableLiquidVersioning } from './table-liquid-versioning.js'
 import { thirdPartyActionPinning } from './third-party-action-pinning.js'
 import { liquidTagWhitespace } from './liquid-tag-whitespace.js'
 import { linkQuotation } from './link-quotation.js'
+import { octiconAriaLabels } from './octicon-aria-labels.js'
 import { liquidIfversionVersions } from './liquid-ifversion-versions.js'
 
 const noDefaultAltText = markdownlintGitHub.find((elem) =>
@@ -82,6 +83,6 @@ export const gitHubDocsMarkdownlint = {
     thirdPartyActionPinning,
     liquidTagWhitespace,
     linkQuotation,
-    liquidIfversionVersions,
+    octiconAriaLabels,
   ],
 }

src/content-linter/lib/linting-rules/octicon-aria-labels.js

@@ -0,0 +1,56 @@
+import { TokenKind } from 'liquidjs'
+import { getLiquidTokens, getPositionData } from '../helpers/liquid-utils.js'
+import { addFixErrorDetail } from '../helpers/utils.js'
+/*
+Octicons should always have an aria-label attribute even if aria hidden. For example:
+
+  DO use aria-label
+  {% octicon "alert" aria-label="alert" %}
+  {% octicon "alert" aria-label="alert" aria-hidden="true" %}
+  {% octicon "alert" aria-label="alert" aria-hidden="true" class="foo" %}
+
+  This is necessary for copilot to be able to recognize the svgs correctly when using our API. 
+
+*/
+
+export const octiconAriaLabels = {
+  names: ['GHD044', 'octicon-aria-labels'],
+  description: 'Octicons should always have an aria-label attribute even if aria-hidden.',
+  tags: ['accessibility', 'octicons'],
+  parser: 'markdownit',
+  function: (params, onError) => {
+    const content = params.lines.join('\n')
+    const tokens = getLiquidTokens(content)
+      .filter((token) => token.kind === TokenKind.Tag)
+      .filter((token) => token.name === 'octicon')
+
+    for (const token of tokens) {
+      const { lineNumber, column, length } = getPositionData(token, params.lines)
+
+      const hasAriaLabel = token.args.includes('aria-label=')
+
+      if (!hasAriaLabel) {
+        const range = [column, length]
+
+        const octiconNameMatch = token.args.match(/["']([^"']+)["']/)
+        const octiconName = octiconNameMatch ? octiconNameMatch[1] : 'icon'
+        const originalContent = token.content
+        const fixedContent = originalContent + ` aria-label="${octiconName}"`
+
+        addFixErrorDetail(
+          onError,
+          lineNumber,
+          `octicon should have an aria-label even if aria hidden. Try using 'aria-label=${octiconName}'`,
+          token.content,
+          range,
+          {
+            lineNumber,
+            editColumn: column,
+            deleteCount: length,
+            insertText: `{% ${fixedContent} %}`,
+          },
+        )
+      }
+    }
+  },
+}

src/content-linter/style/github-docs.js

@@ -168,6 +168,12 @@ const githubDocsConfig = {
     'partial-markdown-files': true,
     'yml-files': true,
   },
+  'octicon-aria-labels': {
+    // GHD044
+    severity: 'warning',
+    'partial-markdown-files': true,
+    'yml-files': true,
+  },
 }
 
 export const githubDocsFrontmatterConfig = {

src/content-linter/tests/unit/octicon-aria-labels.js

@@ -0,0 +1,160 @@
+import { describe, expect, test } from 'vitest'
+import { octiconAriaLabels } from '../../lib/linting-rules/octicon-aria-labels.js'
+
+describe('octicon-aria-labels', () => {
+  const rule = octiconAriaLabels
+
+  test('detects octicon without aria-label', () => {
+    const errors = []
+    const onError = (errorInfo) => {
+      errors.push(errorInfo)
+    }
+
+    const content = ['This is a test with an octicon:', '{% octicon "alert" %}', 'Some more text.']
+
+    rule.function({ lines: content }, onError)
+
+    expect(errors.length).toBe(1)
+    expect(errors[0].lineNumber).toBe(2)
+    expect(errors[0].detail).toContain('aria-label=alert')
+    expect(errors[0].fixInfo.insertText).toContain('aria-label="alert"')
+  })
+
+  test('ignores octicons with aria-label', () => {
+    const errors = []
+    const onError = (errorInfo) => {
+      errors.push(errorInfo)
+    }
+
+    const content = [
+      'This is a test with a proper octicon:',
+      '{% octicon "alert" aria-label="alert" %}',
+      'Some more text.',
+    ]
+
+    rule.function({ lines: content }, onError)
+
+    expect(errors.length).toBe(0)
+  })
+
+  test('detects multiple octicons without aria-label', () => {
+    const errors = []
+    const onError = (errorInfo) => {
+      errors.push(errorInfo)
+    }
+
+    const content = [
+      'This is a test with multiple octicons:',
+      '{% octicon "alert" %}',
+      'Some text in between.',
+      '{% octicon "check" %}',
+      'More text.',
+    ]
+
+    rule.function({ lines: content }, onError)
+
+    expect(errors.length).toBe(2)
+    expect(errors[0].lineNumber).toBe(2)
+    expect(errors[0].detail).toContain('aria-label=alert')
+    expect(errors[1].lineNumber).toBe(4)
+    expect(errors[1].detail).toContain('aria-label=check')
+  })
+
+  test('ignores non-octicon liquid tags', () => {
+    const errors = []
+    const onError = (errorInfo) => {
+      errors.push(errorInfo)
+    }
+
+    const content = [
+      'This is a test with non-octicon tags:',
+      '{% data foo.bar %}',
+      '{% ifversion fpt %}',
+      'Some text.',
+      '{% endif %}',
+    ]
+
+    rule.function({ lines: content }, onError)
+
+    expect(errors.length).toBe(0)
+  })
+
+  test('suggests correct fix for octicon with other attributes', () => {
+    const errors = []
+    const onError = (errorInfo) => {
+      errors.push(errorInfo)
+    }
+
+    const content = [
+      'This is a test with an octicon with other attributes:',
+      '{% octicon "plus" aria-hidden="true" class="foo" %}',
+      'Some more text.',
+    ]
+
+    rule.function({ lines: content }, onError)
+
+    expect(errors.length).toBe(1)
+    expect(errors[0].lineNumber).toBe(2)
+    expect(errors[0].fixInfo.insertText).toContain('aria-label="plus"')
+    expect(errors[0].fixInfo.insertText).toContain('aria-hidden="true"')
+    expect(errors[0].fixInfo.insertText).toContain('class="foo"')
+  })
+
+  test('handles octicons with unusual spacing', () => {
+    const errors = []
+    const onError = (errorInfo) => {
+      errors.push(errorInfo)
+    }
+
+    const content = [
+      'This is a test with unusual spacing:',
+      '{%  octicon   "x"    %}',
+      'Some more text.',
+    ]
+
+    rule.function({ lines: content }, onError)
+
+    expect(errors.length).toBe(1)
+    expect(errors[0].lineNumber).toBe(2)
+    expect(errors[0].detail).toContain('aria-label=x')
+  })
+
+  test('handles octicons split across multiple lines', () => {
+    const errors = []
+    const onError = (errorInfo) => {
+      errors.push(errorInfo)
+    }
+
+    const content = [
+      'This is a test with a multi-line octicon:',
+      '{% octicon "chevron-down"',
+      '   class="dropdown-menu-icon"',
+      '%}',
+      'Some more text.',
+    ]
+
+    rule.function({ lines: content }, onError)
+
+    expect(errors.length).toBe(1)
+    expect(errors[0].detail).toContain('aria-label=chevron-down')
+  })
+
+  test('falls back to "icon" when octicon name cannot be determined', () => {
+    const errors = []
+    const onError = (errorInfo) => {
+      errors.push(errorInfo)
+    }
+
+    const content = [
+      'This is a test with a malformed octicon:',
+      '{% octicon variable %}',
+      'Some more text.',
+    ]
+
+    rule.function({ lines: content }, onError)
+
+    expect(errors.length).toBe(1)
+    expect(errors[0].detail).toContain('aria-label=icon')
+    expect(errors[0].fixInfo.insertText).toContain('aria-label="icon"')
+  })
+})

src/frame/lib/page.js

@@ -8,11 +8,11 @@ import { getAlertTitles } from '#src/languages/lib/get-alert-titles.ts'
 import getTocItems from './get-toc-items.js'
 import Permalink from './permalink.js'
 import { renderContent } from '#src/content-render/index.js'
-import processLearningTracks from '#src/learning-track/lib/process-learning-tracks.js'
+import processLearningTracks from '#src/learning-track/lib/process-learning-tracks'
 import { productMap } from '#src/products/lib/all-products.ts'
 import slash from 'slash'
 import readFileContents from './read-file-contents.js'
-import getLinkData from '#src/learning-track/lib/get-link-data.js'
+import getLinkData from '#src/learning-track/lib/get-link-data'
 import getDocumentType from '#src/events/lib/get-document-type.ts'
 import { allTools } from '#src/tools/lib/all-tools.ts'
 import { renderContentWithFallback } from '#src/languages/lib/render-with-fallback.js'

src/landings/middleware/featured-links.ts

@@ -1,8 +1,8 @@
 import type { Response, NextFunction } from 'express'
 
 import type { ExtendedRequest, FeaturedLinkExpanded } from '@/types'
-import getLinkData from '@/learning-track/lib/get-link-data.js'
-import { renderContent } from '@/content-render/index.js'
+import getLinkData from '@/learning-track/lib/get-link-data'
+import { renderContent } from '@/content-render/index'
 
 /**
  * This is the max. number of featured links, by any category, that we
@@ -73,12 +73,20 @@ export default async function featuredLinks(
       if (!(key in req.context.page.featuredLinks))
         throw new Error('featureLinks key not found in Page')
       const pageFeaturedLink = req.context.page.featuredLinks[key]
-      req.context.featuredLinks[key] = (await getLinkData(
-        pageFeaturedLink,
+      // Handle different types of featuredLinks by converting to string array
+      const stringLinks = Array.isArray(pageFeaturedLink)
+        ? pageFeaturedLink.map((item) => (typeof item === 'string' ? item : item.href))
+        : []
+
+      const linkData = await getLinkData(
+        stringLinks,
         req.context,
         { title: true, intro: true, fullTitle: true },
         MAX_FEATURED_LINKS,
-      )) as FeaturedLinkExpanded[] // Remove ones `getLinkData` is TS
+      )
+      // We need to use a type assertion here because the Page interfaces are incompatible
+      // between our local types and the global types, but the actual runtime objects are compatible
+      req.context.featuredLinks[key] = (linkData || []) as unknown as FeaturedLinkExpanded[]
     }
   }