diff --git a/.github/workflows/all-documents.yml b/.github/workflows/all-documents.yml index 1f0101e770ed..0a9d3c0b4ed0 100644 --- a/.github/workflows/all-documents.yml +++ b/.github/workflows/all-documents.yml @@ -1,8 +1,6 @@ name: All documents script -# **What it does**: Verifies that the all-documents script works. -# **Why we have it**: Code quality and sustainability. -# **Who does it impact**: docs-engineering +# Catches all-documents crashes. This workflow has no output assertions. on: pull_request: @@ -38,5 +36,3 @@ jobs: echo "" echo "Look at the first 50 lines of the file..." cat all-documents.json | jq | head -n 50 - - # We're essentially expecting it to not crash and fail. diff --git a/.github/workflows/article-api-docs.yml b/.github/workflows/article-api-docs.yml index 8bb96924ab4d..1243e53a5e25 100644 --- a/.github/workflows/article-api-docs.yml +++ b/.github/workflows/article-api-docs.yml @@ -1,8 +1,6 @@ name: 'Check article-api docs' -# **What it does**: Makes sure changes to the article api are documented. -# **Why we have it**: So what's documented doesn't fall behind -# **Who does it impact**: Docs engineering, CGS team +# Keeps generated article API docs from falling behind middleware changes. on: workflow_dispatch: @@ -10,7 +8,6 @@ on: paths: - 'src/article-api/middleware/article.ts' - 'src/article-api/middleware/pagelist.ts' - # Self-test - .github/workflows/article-api-docs.yml permissions: @@ -34,8 +31,6 @@ jobs: if [ -n "$(git status --porcelain)" ]; then git status git diff - - # Some whitespace for the sake of the message below echo "" echo "" diff --git a/.github/workflows/auto-add-ready-for-doc-review.yml b/.github/workflows/auto-add-ready-for-doc-review.yml index a96829d134cf..a0816f5316a9 100644 --- a/.github/workflows/auto-add-ready-for-doc-review.yml +++ b/.github/workflows/auto-add-ready-for-doc-review.yml @@ -1,8 +1,6 @@ name: Auto-add ready-for-doc-review label -# **What it does**: Automatically adds the "ready-for-doc-review" label to DIY docs PRs that contain content or data changes when they are opened in a non-draft state or converted from draft to ready for review. -# **Why we have it**: To ensure DIY docs PRs are automatically added to the docs-content review board without requiring manual labeling. -# **Who does it impact**: Contributors making content changes and docs-content reviewers. +# Sends DIY content changes to the docs-content review board without manual labeling. on: pull_request: @@ -34,8 +32,7 @@ jobs: github-token: ${{ secrets.DOCS_BOT_PAT_BASE }} script: | try { - // Team is addressed by numeric ID (org github = 9919, team docs = 325922) - // because IDs survive team renames and slugs do not. + // 9919 is the github org and 325922 the docs team; numeric IDs survive renames. await github.request('GET /organizations/{org_id}/team/{team_id}/memberships/{username}', { org_id: 9919, team_id: 325922, diff --git a/.github/workflows/auto-close-dependencies.yml b/.github/workflows/auto-close-dependencies.yml index a073fa3328fa..c0c6a4d5c104 100644 --- a/.github/workflows/auto-close-dependencies.yml +++ b/.github/workflows/auto-close-dependencies.yml @@ -1,10 +1,6 @@ name: Auto Close Open Source Dependency Updates -# **What it does**: -# - close-external: Automatically close dependabot's pull requests in the open-source repository. -# **Why we have it**: -# - close-external: To avoid duplicating updates against the internal repository. -# **Who does it impact**: It helps docs engineering focus on higher value work. +# Closes Dependabot dependency updates in github/docs because the internal repo owns them. on: pull_request: @@ -48,7 +44,7 @@ jobs: run: | gh pr comment "$PR_URL" --body "This dependency update will be handled internally by our engineering team." - # Because we get far too much spam ;_; + # Lock conversations to stop repeated dependency-update comments. - name: Lock conversations uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 env: diff --git a/.github/workflows/benchmark-pages.yml b/.github/workflows/benchmark-pages.yml index a23b3c37784d..aa0e0edf696a 100644 --- a/.github/workflows/benchmark-pages.yml +++ b/.github/workflows/benchmark-pages.yml @@ -1,13 +1,11 @@ name: 'Weekly page benchmark' -# **What it does**: Benchmarks all pages via the article API, flags errors and slow pages -# **Why we have it**: Catch perf regressions and broken pages before users hit them -# **Who does it impact**: Docs engineering +# Catches slow or broken article API pages before users hit them. on: workflow_dispatch: schedule: - - cron: '20 16 * * 1' # Every Monday at 16:20 UTC / 8:20 PST + - cron: '20 16 * * 1' # Mondays at 16:20 UTC. permissions: contents: read diff --git a/.github/workflows/changelog-agent.yml b/.github/workflows/changelog-agent.yml index 71e73de5c3ae..acf2240f0de7 100644 --- a/.github/workflows/changelog-agent.yml +++ b/.github/workflows/changelog-agent.yml @@ -1,11 +1,7 @@ name: Changelog agent — draft entry when a qualified PR merges -# **What it does**: When a PR merges that closes a docs-content issue with a -# parent issue, uses an LLM to draft a changelog entry, opens a PR in -# github/docs-content, and DMs the author in Slack for review. -# **Why we have it**: Automates the changelog drafting process so authors -# don't have to remember to write a changelog entry manually. -# **Who does it impact**: docs-content team members. +# Drafts internal changelog entries for merged PRs that close a docs-content child issue. +# Authors review generated PRs before publication. on: pull_request: @@ -81,7 +77,6 @@ jobs: script: | const author = '${{ steps.resolve_pr.outputs.pr_author }}'; - // Fetch github-to-slack.json from docs-content via API let mapping = {}; try { const { data } = await github.rest.repos.getContent({ @@ -96,7 +91,6 @@ jobs: return; } - // Remove non-user keys (like _comment) const teamMembers = Object.keys(mapping).filter(k => !k.startsWith('_')); if (!teamMembers.includes(author)) { @@ -119,8 +113,7 @@ jobs: script: | const body = process.env.PR_BODY || ''; - // Match closing keywords followed by docs-content issue references. - // Supports: closes github/docs-content#123, fixes https://github.com/github/docs-content/issues/123 + // Finds docs-content issues closed by the source PR body. const patterns = [ /(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?):?\s+github\/docs-content#(\d+)/gi, /(?:close[sd]?|fix(?:e[sd])?|resolve[sd]?):?\s+https:\/\/github\.com\/github\/docs-content\/issues\/(\d+)/gi, @@ -168,7 +161,6 @@ jobs: return; } - // Query for parent issue via GraphQL const query = ` query($nodeId: ID!) { node(id: $nodeId) { @@ -218,7 +210,6 @@ jobs: core.setOutput('parent_assignees', (parent.assignees?.nodes || []).map(a => a.login).join(',')); core.setOutput('parent_repo', parent.repository.nameWithOwner); - // Also store the docs-content issue details core.setOutput('dc_issue_title', issue.title); core.setOutput('dc_issue_body', issue.body || ''); @@ -236,7 +227,6 @@ jobs: const prNumber = parseInt('${{ steps.resolve_pr.outputs.pr_number }}', 10); const prAuthor = '${{ steps.resolve_pr.outputs.pr_author }}'; - // Get approved reviewers (exclude bots and PR author) const { data: reviews } = await github.rest.pulls.listReviews({ owner: context.repo.owner, repo: context.repo.repo, @@ -249,7 +239,6 @@ jobs: .map(r => r.user.login) )]; - // Get changed files (paths only, limit to 50) const { data: files } = await github.rest.pulls.listFiles({ owner: context.repo.owner, repo: context.repo.repo, @@ -297,7 +286,6 @@ jobs: with: github-token: ${{ secrets.DOCS_BOT_PAT_BASE }} script: | - // Fetch changelog-internal.md from docs-content const { data } = await github.rest.repos.getContent({ owner: 'github', repo: 'docs-content', @@ -305,7 +293,6 @@ jobs: }); const changelog = Buffer.from(data.content, 'base64').toString('utf-8'); - // Extract the first 3 entries (each starts with **date**) const lines = changelog.split('\n'); let count = 0; let examples = []; @@ -420,12 +407,8 @@ jobs: uses: actions/ai-inference@2c43c91ae16266ca159d311430343c67a5ffa222 # v3 with: provider: copilot - # Must be an explicit empty string, not omitted. This action defaults - # `model` to "gpt-4.1" and always forwards it as --model, and that slug - # is retired, so omitting the input fails with: - # Error: Model "gpt-4.1" from --model flag is not available. - # An empty string makes the action skip --model entirely and lets the - # Copilot CLI pick its own current default. See actions/ai-inference#271. + # Keep this empty string. Omitting it makes actions/ai-inference pass + # its retired gpt-4.1 default, which fails. model: '' prompt-file: prompt.txt system-prompt-file: system-prompt.txt @@ -471,7 +454,6 @@ jobs: const branchName = `changelog-agent-${{ steps.resolve_pr.outputs.pr_number }}`; const filePath = 'docs-content-docs/docs-content-workflows/changelog-internal.md'; - // Get the current changelog file from docs-content const { data: fileData } = await github.rest.repos.getContent({ owner: 'github', repo: 'docs-content', @@ -480,11 +462,9 @@ jobs: let changelog = Buffer.from(fileData.content, 'base64').toString('utf-8'); - // Build the new entry const entry = `**${process.env.DATE_STR}**\n\n${process.env.DRAFT}\n\n
`; - // Insert after the first H1 heading so leading frontmatter, comments, - // or blank lines do not affect placement. + // Insert after the first H1 heading so frontmatter, comments, or blank lines do not affect placement. const lines = changelog.split('\n'); const headingIndex = lines.findIndex((line) => line.startsWith('# ')); @@ -498,14 +478,12 @@ jobs: : `${beforeAndHeading}\n\n${entry}`; } - // Get the default branch SHA for creating a new branch const { data: ref } = await github.rest.git.getRef({ owner: 'github', repo: 'docs-content', ref: 'heads/main', }); - // Create the branch in docs-content try { await github.rest.git.createRef({ owner: 'github', @@ -521,7 +499,6 @@ jobs: } } - // Fetch the file from the branch (handles both new and existing branches) const { data: branchFileData } = await github.rest.repos.getContent({ owner: 'github', repo: 'docs-content', @@ -529,7 +506,6 @@ jobs: ref: branchName, }); - // Update the changelog file on the new branch await github.rest.repos.createOrUpdateFileContents({ owner: 'github', repo: 'docs-content', @@ -544,7 +520,6 @@ jobs: }, }); - // Build credits for the PR body const reviewers = process.env.APPROVED_REVIEWERS ? process.env.APPROVED_REVIEWERS.split(',').map(r => `@${r}`).join(', ') : 'None'; @@ -586,7 +561,6 @@ jobs: draft: false, }); - // Add labels try { await github.rest.issues.addLabels({ owner: 'github', @@ -598,7 +572,6 @@ jobs: core.warning(`Failed to add labels: ${err.message}`); } - // Request review from PR author try { await github.rest.pulls.requestReviewers({ owner: 'github', @@ -632,7 +605,6 @@ jobs: const author = process.env.PR_AUTHOR; const changelogPrUrl = process.env.CHANGELOG_PR_URL; - // Fetch GitHub-to-Slack mapping from docs-content let slackMapping = {}; try { const { data } = await github.rest.repos.getContent({ @@ -648,7 +620,6 @@ jobs: const slackUserId = slackMapping[author]; - // Build credits summary for the DM const reviewers = process.env.APPROVED_REVIEWERS ? process.env.APPROVED_REVIEWERS.split(',').join(', ') : 'none'; @@ -699,7 +670,6 @@ jobs: core.warning(`No Slack mapping found for GitHub user: ${author}`); } - // Fallback: post a GitHub comment on the source PR core.info('Falling back to GitHub comment notification.'); await github.rest.issues.createComment({ owner: context.repo.owner, @@ -732,7 +702,7 @@ jobs: body: `\nšŸ¤– A changelog draft PR has been automatically created in docs-content: ${changelogPrUrl}`, }); - # Local composite actions below require the repository to be checked out. + # Check out the repository before local composite actions run after a failure. - name: Check out repo if: ${{ failure() && github.event_name != 'workflow_dispatch' }} uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 diff --git a/.github/workflows/check-for-spammy-issues.yml b/.github/workflows/check-for-spammy-issues.yml index 6bd4e68dab42..f5590ce80841 100644 --- a/.github/workflows/check-for-spammy-issues.yml +++ b/.github/workflows/check-for-spammy-issues.yml @@ -1,8 +1,6 @@ name: Check for Spammy Issues -# **What it does**: This action closes low value issues in the open-source repository. -# **Why we have it**: We get lots of spam in the open-source repository. -# **Who does it impact**: Open-source contributors. +# Closes low-value public issues so spam does not stay open in github/docs. on: issues: @@ -39,24 +37,15 @@ jobs: username: context.payload.sender.login, }); - // Do not perform this workflow with GitHub employees. This return - // statement only gets hit if the user is a GitHub employee + // Skip GitHub employees so legitimate internal reports stay open. return } catch (err) { - // An error will be thrown if the user is not a GitHub employee - // If a user is not a GitHub employee, we should check to see if title has at least the minimum required number of words in it and if it does, we can exit the workflow - + // The membership lookup throws for non-employees, so fall through to title checks. if (titleWordCount >= titleWordCountMin && !titleHasUrl && !titleHasDollarSign) { return } } - // - // Assuming the user is not a GitHub employee and the issue title - // does not contain the minimum number of words required, proceed. - // - - // Close the issue and add the invalid label await github.rest.issues.update({ owner: owner, repo: repo, @@ -65,7 +54,6 @@ jobs: state: 'closed' }); - // Comment on the issue await github.rest.issues.createComment({ owner: owner, repo: repo, @@ -73,7 +61,6 @@ jobs: body: `This issue may have been opened accidentally. I'm going to close it now, but feel free to open a new issue with a more descriptive title! Make sure not to include full URLs in your issue titles, and use paths instead.` }); - // Add the issue to the Done column on the triage board try { await github.rest.projects.createCard({ column_id: 11167427, diff --git a/.github/workflows/check-for-spammy-prs.yml b/.github/workflows/check-for-spammy-prs.yml index 6568410ab995..47462242fbea 100644 --- a/.github/workflows/check-for-spammy-prs.yml +++ b/.github/workflows/check-for-spammy-prs.yml @@ -1,8 +1,6 @@ name: Check for Spammy PRs -# **What it does**: This action closes low value pull requests and PRs that do not target main. -# **Why we have it**: We get lots of spam in the open-source repository. -# **Who does it impact**: Open-source contributors. +# Flags low-value public PRs and closes PRs that target branches other than main. on: pull_request_target: diff --git a/.github/workflows/close-bad-repo-sync-prs.yml b/.github/workflows/close-bad-repo-sync-prs.yml index 507b8596f2da..539117b0ba5e 100644 --- a/.github/workflows/close-bad-repo-sync-prs.yml +++ b/.github/workflows/close-bad-repo-sync-prs.yml @@ -1,14 +1,9 @@ name: Close bad repo-sync PRs -# **What it does**: -# Closes and PR from `repo-sync` to `main` that wasn't created by a Hubber. -# **Why we have it**: -# Unfortunately, a lot of PRs in github/docs are created by people who -# shouldn't be creating such PRs. We bot our bots to own it. -# **Who does it impact**: Open-source. +# Closes public PRs from the repo-sync source branch unless the event actor is a GitHub employee. on: - # Necessary in lieu of `pull_request` so that PRs opened from forks can be closed if they try to push to a repo sync branch. + # pull_request_target lets the workflow close forked PRs from the repo-sync source branch. pull_request_target: permissions: @@ -37,15 +32,13 @@ jobs: username: prCreator }) - // If the PR creator is a GitHub employee, stop now + // Skip GitHub employees because the event actor may own legitimate repo-sync work. console.log("PR creator is a GitHub employee") return } catch (err) { - // An error will be thrown if the user is not a GitHub employee. - // That said, we still want to proceed anyway! + // The membership lookup throws for non-employees, so fall through and close the PR. } - // Close the PR and add the invalid label await github.rest.issues.update({ owner, repo, @@ -54,7 +47,6 @@ jobs: state: 'closed' }) - // Comment on the PR await github.rest.issues.createComment({ owner, repo, diff --git a/.github/workflows/close-on-invalid-label.yaml b/.github/workflows/close-on-invalid-label.yaml index 654d97402bdc..c7a0230d7c90 100644 --- a/.github/workflows/close-on-invalid-label.yaml +++ b/.github/workflows/close-on-invalid-label.yaml @@ -1,14 +1,11 @@ name: Close issue/PR on adding invalid label -# **What it does**: This action closes invalid pull requests in the open-source repository. -# **Why we have it**: We get lots of spam in the open-source repository. -# **Who does it impact**: Open-source contributors. +# Closes public issues and PRs when the invalid label is added. on: issues: types: [labeled] - # Needed in lieu of `pull_request` so that PRs from a fork can be - # closed when marked as invalid. + # pull_request_target lets the workflow close forked PRs after the invalid label is added. pull_request_target: types: [labeled] diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml index 28a2c2fb23ff..5c31ec7b3c0a 100644 --- a/.github/workflows/codeql.yml +++ b/.github/workflows/codeql.yml @@ -1,15 +1,12 @@ name: CodeQL analysis -# **What it does**: This runs CodeQL on our repository. -# **Why we have it**: Security scanning. -# **Who does it impact**: Docs engineering. +# Runs CodeQL security scanning for the internal and public docs repositories. on: pull_request: branches: - main - # This is so that when CodeQL runs on a pull request, it can compare - # against the state of the base branch. + # Push runs give pull request scans a base branch for comparison. push: branches: - main @@ -19,7 +16,7 @@ permissions: contents: read security-events: write -# This allows a subsequently queued workflow run to interrupt previous runs +# Newer runs cancel queued or running scans for the same ref. concurrency: group: '${{ github.workflow }} @ ${{ github.event.pull_request.head.label || github.head_ref || github.ref }}' cancel-in-progress: true @@ -33,7 +30,7 @@ jobs: - uses: github/codeql-action/init@e296a935590eb16afc0c0108289f68c87e2a89a5 # v4.30.7 with: - languages: javascript # comma separated list of values from {go, python, javascript, java, cpp, csharp, ruby} + languages: javascript - uses: github/codeql-action/analyze@e296a935590eb16afc0c0108289f68c87e2a89a5 # v4.30.7 continue-on-error: true diff --git a/.github/workflows/comment-release-note-info.yml b/.github/workflows/comment-release-note-info.yml index 8aabdb277200..f7ebce2c1f3a 100644 --- a/.github/workflows/comment-release-note-info.yml +++ b/.github/workflows/comment-release-note-info.yml @@ -1,7 +1,7 @@ -# This workflow provides information when a contributor edits a release note file - name: Comment on release note changes +# Prompts contributors to request technical review for release note changes. + on: pull_request: types: @@ -16,8 +16,7 @@ permissions: jobs: comment: - # Do not add this comment on PRs created by the bot during the standard patch release process - # or in the github/docs repository + # Skip release-controller patch PRs and public-repo PRs. if: github.event.pull_request.user.login != 'release-controller[bot]' && github.repository == 'github/docs-internal' runs-on: ubuntu-latest steps: diff --git a/.github/workflows/confirm-internal-staff-work-in-docs.yml b/.github/workflows/confirm-internal-staff-work-in-docs.yml index f655b4655ba9..a7120ffeab1f 100644 --- a/.github/workflows/confirm-internal-staff-work-in-docs.yml +++ b/.github/workflows/confirm-internal-staff-work-in-docs.yml @@ -1,15 +1,13 @@ name: Confirm internal staff meant to post in public -# **What it does**: If a GitHub staff makes an issue/pull request in the open-source repo, creates an issue in the internal one to verify intent. -# **Why we have it**: We don't want GitHub staff accidentally making issues/pull requests in the wrong repository. -# **Who does it impact**: GitHub staff. +# Asks GitHub staff to confirm public issues and PRs belong in github/docs. on: issues: types: - opened - transferred - # Required in lieu of `pull_request` so that this workflow can query users in org to determine membership. + # pull_request_target lets the workflow check org membership for forked PR authors. pull_request_target: types: - opened @@ -30,7 +28,7 @@ jobs: with: github-token: ${{ secrets.DOCS_BOT_PAT_BASE }} script: | - // Only perform this action with GitHub employees + // Only GitHub employees need confirmation before public posts stay public. try { await github.rest.teams.getMembershipForUserInOrg({ org: 'github', @@ -38,33 +36,25 @@ jobs: username: context.payload.sender.login, }); } catch(err) { - // An error will be thrown if the user is not a GitHub employee - // If a user is not a GitHub employee, we should stop here and - // Not send a notification + // The employee membership lookup throws for non-employees, so skip confirmation. return } - // Don't perform this action with Docs team members + // Docs team members can intentionally work in github/docs. try { - // Team is addressed by numeric ID (org github = 9919, team docs = 325922) - // because IDs survive team renames and slugs do not. + // 9919 is the github org and 325922 the docs team; numeric IDs survive renames. await github.request('GET /organizations/{org_id}/team/{team_id}/memberships/{username}', { org_id: 9919, team_id: 325922, username: context.payload.sender.login, }); - // If the user is a Docs team member, we should stop here and not send - // a notification return } catch(err) { - // An error will be thrown if the user is not a Docs team member - // If a user is not a Docs team member we should continue and send - // the notification + // The docs team membership lookup throws for non-members, who need an internal confirmation issue. } const issueNo = context.number || context.issue.number - // Create an issue in our private repo await github.rest.issues.create({ owner: 'github', repo: process.env.TEAM_CONTENT_REPO, diff --git a/.github/workflows/content-lint-markdown.yml b/.github/workflows/content-lint-markdown.yml index adb16aa08720..dd55f7ecf1f4 100644 --- a/.github/workflows/content-lint-markdown.yml +++ b/.github/workflows/content-lint-markdown.yml @@ -1,8 +1,6 @@ name: 'Content Lint Markdown' -# **What it does**: Lints our content markdown to ensure the content matches the specified styleguide. -# **Why we have it**: We want some level of consistency to our content markdown files. -# **Who does it impact**: Docs content writers. +# Runs content linting on changed content and data files so style issues stay visible. on: pull_request: @@ -25,7 +23,7 @@ jobs: - name: Check out repo uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 with: - # Fetch 2 commits so tj-actions/changed-files can diff without extra API calls + # Fetch two commits so changed-files can diff without extra API calls. fetch-depth: 2 - name: Set up Node and dependencies @@ -43,9 +41,8 @@ jobs: if: steps.changed_files.outputs.any_modified == 'true' env: CHANGED_FILES: ${{ steps.changed_files.outputs.all_modified_files }} - # If there are errors, using `--print-annotations` will make it - # so it does *not* exit non-zero. - # This is so that all warnings and errors are printed. + # Print annotations first because that command exits zero when it reports errors. + # The next step fails the workflow with errors-only. run: npm run lint-content -- --print-annotations --paths $CHANGED_FILES - name: Run content linter if changed content/data files @@ -53,3 +50,21 @@ jobs: env: CHANGED_FILES: ${{ steps.changed_files.outputs.all_modified_files }} run: npm run lint-content -- --errors-only --paths $CHANGED_FILES + + # Journey landing pages reference other articles by path in their frontmatter. + # Moving or deleting one of those articles breaks the reference without touching + # the landing page, so it never appears in the diff and GHD059 never runs on it. + # The weekly full sweep catches it, but up to seven days late, and the article + # loses its next/previous journey navigation the whole time. + # + # Check the landing pages on every run instead. Scoped to that one rule so a PR + # can't be blocked by unrelated findings on pages it didn't touch, and left + # ungated because deleting an article is one of the ways this breaks. + - name: Check journey landing page guide paths + run: | + JOURNEY_PAGES=$(grep -rlE "^layout: *['\"]?journey-landing['\"]? *$" content/ || true) + if [ -z "$JOURNEY_PAGES" ]; then + echo "No journey landing pages found, nothing to check." + exit 0 + fi + npm run lint-content -- --rules journey-tracks-guide-path-exists --paths $JOURNEY_PAGES diff --git a/.github/workflows/content-linter-rules-docs.yml b/.github/workflows/content-linter-rules-docs.yml index da5de0d478d7..2f6090d9b87d 100644 --- a/.github/workflows/content-linter-rules-docs.yml +++ b/.github/workflows/content-linter-rules-docs.yml @@ -1,19 +1,16 @@ name: 'Check content-linter rules docs' -# **What it does**: Makes sure the content-linter-rules.md is up-to-date. -# **Why we have it**: So what's automated doesn't fall behind -# **Who does it impact**: Docs content. +# Keeps generated content-linter rule docs in sync with code and markdownlint changes. on: workflow_dispatch: pull_request: paths: - 'src/content-linter/**' - # In case imported markdownlint rules are updated + # Markdownlint dependency changes can alter generated rule docs. - package-lock.json - # In case manual changes are made to the content-linter-rules.md file + # Manual edits to generated docs must fail this check. - data/reusables/contributing/content-linter-rules.md - # Self-test - .github/workflows/content-linter-rules-docs.yml permissions: @@ -37,8 +34,6 @@ jobs: if [ -n "$(git status --porcelain)" ]; then git status git diff - - # Some whitespace for the sake of the message below echo "" echo "" diff --git a/.github/workflows/content-pipelines.yml b/.github/workflows/content-pipelines.yml index 9ddfc5f3045f..312d4b26e581 100644 --- a/.github/workflows/content-pipelines.yml +++ b/.github/workflows/content-pipelines.yml @@ -1,21 +1,10 @@ name: 'Content pipelines: Update content' -# **What it does**: On a schedule, runs the content pipeline update script for each -# configured entry. The script clones each source repo, detects changes, and -# runs a Copilot agent to update content articles. The workflow handles -# branching, committing, and opening PRs. -# **Why we have it**: Keeps reference documentation in sync with upstream source -# docs without storing copies of those source docs in this repository. -# **Who does it impact**: Docs content writers, docs engineering. -# -# To add a new entry, add it to src/content-pipelines/config.yml and to the matrix -# `include` list below (only `id` is needed). The update logic lives in -# src/content-pipelines/scripts/update.ts, which reads config.yml for all other -# values. Run locally: npx tsx src/content-pipelines/scripts/update.ts --help +# Updates reference docs from upstream source docs without storing source copies here. on: schedule: - - cron: '20 16 * * 1-5' # Mon-Fri at 16:20 UTC + - cron: '20 16 * * 1-5' # Weekdays at 16:20 UTC. workflow_dispatch: permissions: @@ -33,12 +22,10 @@ jobs: fail-fast: false matrix: include: - # Each entry only needs `id`. Everything else (source-repo, - # source-path, target-articles, etc.) is read from - # src/content-pipelines/config.yml by the update script. + # Add each new pipeline to src/content-pipelines/config.yml and this matrix. + # src/content-pipelines/scripts/update.ts reads config.yml for source and target values. - id: copilot-cli - id: gh-stack - # - id: mcp-server steps: - name: Checkout docs-internal diff --git a/.github/workflows/copilot-code-review.yml b/.github/workflows/copilot-code-review.yml index b61a8bebbf92..dc8f949a3a93 100644 --- a/.github/workflows/copilot-code-review.yml +++ b/.github/workflows/copilot-code-review.yml @@ -1,10 +1,8 @@ -# Copilot Code Review setup steps -# -# Code Review cannot access the private early-access repository, so it uses -# this secret-free setup instead of the cloud agent setup workflow. - name: 'Copilot Code Review Setup Steps' +# Code Review lacks early-access repo access, so this uses a secret-free setup +# instead of copilot-setup-steps.yml. + on: workflow_dispatch: diff --git a/.github/workflows/copilot-setup-steps.yml b/.github/workflows/copilot-setup-steps.yml index 417a195a078e..257ba362898d 100644 --- a/.github/workflows/copilot-setup-steps.yml +++ b/.github/workflows/copilot-setup-steps.yml @@ -1,20 +1,8 @@ -# Copilot cloud agent setup steps -# -# This is a special-name workflow recognized by Copilot cloud agent. -# When a cloud agent session starts (via GitHub issue assignment or the -# Copilot UI), these steps run first to bootstrap the development -# environment before the agent begins working. -# -# The workflow_dispatch trigger allows manual testing of the setup steps. -# This is NOT a regular CI workflow — it does not run on push or PR events. -# -# See also: -# .github/copilot-instructions.md — always-on agent instructions -# .github/instructions/ — contextual instruction files -# .github/prompts/ — on-demand prompt files (e.g. /code-review) - name: 'Copilot Setup Steps' +# Copilot cloud agents run this special-name workflow before work; workflow_dispatch tests it without push or PR runs. +# Related agent instructions live in .github/instructions/. + on: workflow_dispatch: @@ -32,12 +20,12 @@ jobs: uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 - uses: ./.github/actions/node-npm-setup - # Search and language test suites require a running Elasticsearch instance. + # Search and language test suites require local Elasticsearch. - uses: ./.github/actions/setup-elasticsearch with: token: ${{ secrets.DOCS_BOT_PAT_BASE }} - # docs-internal has early-access content that must be fetched separately. + # Fetch early-access content separately because docs-internal does not include it. - uses: ./.github/actions/get-docs-early-access if: ${{ github.repository == 'github/docs-internal' }} with: @@ -47,12 +35,7 @@ jobs: - name: Build run: npm run build - # Populate Elasticsearch with fixture data so search/language tests work. - # ELASTICSEARCH_URL is set inline in the run command because the - # Copilot/GHAS agent runtime executes these setup steps with its own - # injected environment and does not apply the workflow's `env:` blocks - # (job-level or step-level). The inline assignment is part of the run - # command, which the agent runs verbatim, so it is honored in both the - # agent context and normal workflow_dispatch runs. + # Set ELASTICSEARCH_URL inline because agent runtimes ignore workflow env blocks. + # Command-level env works in agent sessions and manual test runs. - name: Index fixtures into the local Elasticsearch run: ELASTICSEARCH_URL=http://localhost:9200/ npm run index-test-fixtures diff --git a/.github/workflows/copy-api-issue-to-internal.yml b/.github/workflows/copy-api-issue-to-internal.yml index 1c24f6dbc800..439a4caf4a61 100644 --- a/.github/workflows/copy-api-issue-to-internal.yml +++ b/.github/workflows/copy-api-issue-to-internal.yml @@ -1,8 +1,7 @@ name: Copy to API/events issue to docs-content -# **What it does**: Copies an issue in the open source repo to the docs-content repo, comments on and closes the original issue -# **Why we have it**: OpenAPI/GraphQL schema updates cannot be made in the open source repo. Instead, we copy the issue to an internal issue (we do not transfer so that the issue does not disappear for the contributor) and close the original issue. -# **Who does it impact**: Open source and docs-content maintainers +# Copies public API schema issues to docs-content because updates happen internally. +# It does not transfer them, so the issue stays visible to the contributor. on: issues: @@ -26,8 +25,7 @@ jobs: result-encoding: string script: | const triggerer_login = context.payload.sender.login - // Team is addressed by numeric ID (org github = 9919, team docs = 325922) - // because IDs survive team renames and slugs do not. + // 9919 is the github org and 325922 the docs team; numeric IDs survive renames. const teamMembers = await github.request( `/organizations/9919/team/325922/members?per_page=100` ) diff --git a/.github/workflows/count-translation-corruptions.yml b/.github/workflows/count-translation-corruptions.yml index beb046358e4c..6dd21f7af264 100644 --- a/.github/workflows/count-translation-corruptions.yml +++ b/.github/workflows/count-translation-corruptions.yml @@ -1,8 +1,6 @@ name: Count translation corruptions -# **What it does**: Generates a summary of Liquid corruptions per language. -# **Why we have it**: For insights into the state of translations and things we can do to fix them -# **Who does it impact**: Engineering +# Counts Liquid corruptions across translations before correction logic changes ship. on: workflow_dispatch: @@ -26,13 +24,9 @@ jobs: - name: Checkout English repo uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 with: - # Using a PAT is necessary so that the new commit will trigger the - # CI in the PR. (Events from GITHUB_TOKEN don't trigger new workflows.) token: ${{ secrets.DOCS_BOT_PAT_BASE }} - # It's important because translations are often a bit behind. - # So if a translation is a bit behind, it might still be referencing - # an asset even though none of the English content does. + # Clone translations so the count script can render localized pages for corruption scans. - name: Clone all translations uses: ./.github/actions/clone-translations with: diff --git a/.github/workflows/create-changelog-pr.yml b/.github/workflows/create-changelog-pr.yml index bbffdd387582..2d8ade777565 100644 --- a/.github/workflows/create-changelog-pr.yml +++ b/.github/workflows/create-changelog-pr.yml @@ -1,8 +1,6 @@ name: Create a PR to add an entry to the CHANGELOG.md file in this repo -# **What it does**: If a member of the github org posts a changelog comment, it creates a PR to update the CHANGELOG.md file. -# **Why we have it**: This surfaces docs changelog details publicly. -# **Who does it impact**: GitHub users and staff. +# Surfaces docs changelog details publicly from authorized github-org comments. on: issue_comment: @@ -56,7 +54,6 @@ jobs: env: COMMENT_BODY: ${{ github.event.comment.body }} run: | - # Get the first line of the comment and trim the leading/trailing whitespace: FIRST_LINE=$(printf "%s\n" "$COMMENT_BODY" | head -n1 | sed 's/^[[:space:]]*//;s/[[:space:]]*$//') if [[ "$FIRST_LINE" != '## Changelog summary' ]]; then echo "FIRST_LINE=|$FIRST_LINE|" @@ -92,11 +89,7 @@ jobs: echo "BRANCH=$BRANCH" >> $GITHUB_ENV git checkout -b "$BRANCH" - # Insert new changelog entry after the first heading, as follows: - # Print the first line of the existing CHANGELOG.md file into a `tmp` file, followed by an empty line. - # Then, print the contents of `changelog_entry.txt` into the `tmp` file. - # Then, print the rest of the existing CHANGELOG.md file into the `tmp` file. - # Finally, replace the existing CHANGELOG.md file with the `tmp` file. + # Insert after the first line because the changelog starts with its H1. awk 'NR==1{print; print ""; while ((getline line < "changelog_entry.txt") > 0) print line; next}1' CHANGELOG.md > tmp && mv tmp CHANGELOG.md git add CHANGELOG.md @@ -126,7 +119,6 @@ jobs: if: env.CONTINUE_WORKFLOW == 'true' uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 env: - # Get the number of the PR that was just created: PULL_REQUEST_NUMBER: ${{ steps.create_pull_request.outputs.pull-request-number }} with: github-token: ${{ secrets.DOCS_BOT_PAT_BASE }} @@ -142,7 +134,6 @@ jobs: if: env.CONTINUE_WORKFLOW == 'true' uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 env: - # Reuse the PR number captured earlier PULL_REQUEST_NUMBER: ${{ steps.create_pull_request.outputs.pull-request-number }} with: github-token: ${{ secrets.DOCS_BOT_PAT_BASE }} diff --git a/.github/workflows/delete-orphan-translation-files.yml b/.github/workflows/delete-orphan-translation-files.yml index c827f9848abc..b78884b0e1b6 100644 --- a/.github/workflows/delete-orphan-translation-files.yml +++ b/.github/workflows/delete-orphan-translation-files.yml @@ -1,20 +1,11 @@ name: Delete orphan translation files -# **What it does**: -# Compares content & data files left in each translation that aren't -# in docs-internal. Then creates a PR to delete these files. -# **Why we have it**: -# When Juno dumps to each translation repo it can not account for the -# fact that files in docs-internal get moved or deleted. So the -# sum total of files constantly grows. -# This leads to excess files in each translation repo that are not -# ever used but has to be put into every production build. -# **Who does it impact**: Docs engineering +# Deletes orphan translation files because Juno cannot remove docs-internal moves or deletes. on: workflow_dispatch: schedule: - - cron: '20 16 * * 1' # Run every Monday at 16:20 UTC / 8:20 PST + - cron: '20 16 * * 1' # Mondays at 16:20 UTC. permissions: contents: write @@ -84,13 +75,15 @@ jobs: git config --global user.name "docs-bot" git config --global user.email "77750099+docs-bot@users.noreply.github.com" + # Prefer auto-merge so required checks can finish before merge. + # If GitHub reports missing branch rules, unstable, or not mergeable, fall back to direct merge. - name: Git commit and push, create and merge PR working-directory: ${{ matrix.language_dir }} env: - # Needed for gh + # GH_TOKEN gives gh docs-bot permissions for translation repos. GH_TOKEN: ${{ secrets.DOCS_BOT_PAT_BASE }} run: | - # If nothing to commit, exit now. It's fine. No orphans. + # Exit when the deletion script found no orphan files. changes=$(git diff --name-only | wc -l) untracked=$(git status --untracked-files --short | wc -l) if [[ $changes -eq 0 ]] && [[ $untracked -eq 0 ]]; then @@ -98,7 +91,6 @@ jobs: exit 0 fi - # Create a general retry function that retries and sleeps retry_command() { local max_attempts=3 local attempt=1 @@ -107,7 +99,7 @@ jobs: echo "Attempt $attempt: $@" "$@" && return 0 ((attempt++)) - sleep 3 # You can adjust the sleep duration as needed + sleep 3 done echo "Max attempts reached. Command failed after $max_attempts attempts." @@ -122,7 +114,6 @@ jobs: git commit -a -m "Delete orphan files ($current_daystamp)" git push origin "$branch_name" - # Create PR echo "Creating pull request..." gh pr create \ --title "Delete orphan files ($current_daystamp)" \ @@ -132,14 +123,6 @@ jobs: --label "workflow-generated" \ --head=$branch_name echo "Merge created PR..." - # Prefer enabling auto-merge so the PR waits for any required - # checks before merging. If auto-merge can't be enabled — usually - # because all required checks completed before this step ran and - # the PR is already immediately mergeable — fall back to a direct - # merge. GitHub returns one of these misleading errors in that - # case: "Branch does not have required protected branch rules", - # "Pull request is in unstable status", or "Pull request is not - # in a mergeable state". auto_merge_err=$(mktemp) trap 'rm -f "$auto_merge_err"' EXIT if retry_command gh pr merge --merge --auto --delete-branch "$branch_name" 2>"$auto_merge_err"; then diff --git a/.github/workflows/docs-review-collect.yml b/.github/workflows/docs-review-collect.yml index d34402f0295a..0aba95d7e4c5 100644 --- a/.github/workflows/docs-review-collect.yml +++ b/.github/workflows/docs-review-collect.yml @@ -1,13 +1,11 @@ name: Add docs-reviewers request to the docs-content review board -# **What it does**: Adds PRs in github/github and github/audit-log-allowlists that requested a review from docs-reviewers to the docs-content review board -# **Why we have it**: To catch docs-reviewers requests in github/audit-log-allowlists -# **Who does it impact**: docs-content maintainers +# Catches audit-log-allowlists docs-reviewers requests that the regular board misses. on: workflow_dispatch: schedule: - - cron: '20 16 * * 1-5' # Run Mon-Fri at 16:20 UTC / 8:20 PST + - cron: '20 16 * * 1-5' # Weekdays at 16:20 UTC. permissions: contents: read diff --git a/.github/workflows/dont-delete-assets.yml b/.github/workflows/dont-delete-assets.yml index 3c3e73d4cacf..3d10e2c54fd1 100644 --- a/.github/workflows/dont-delete-assets.yml +++ b/.github/workflows/dont-delete-assets.yml @@ -1,14 +1,7 @@ name: Don't delete assets -# **What it does**: -# If the PR (against main) involves deletion of assets, if any of -# them are deletions or renames, post a comment, and ultimately -# fail the check. -# **Why we have it**: -# If you delete the reference to an image, the English content is fine -# because it no longer tries to serve an image that doesn't exist. -# But this is not the case for translations. -# **Who does it impact**: Docs content. +# Fails asset-deletion PRs because translations still reference deleted English assets. +# The comment script exempts generated copilot-sdk assets that sync pipelines recreate. on: workflow_dispatch: @@ -25,7 +18,7 @@ permissions: jobs: dont-delete-assets: - # It's 'docs-bot' that creates those PR from "Delete orphaned assets" + # The Orphaned files check workflow opens docs-bot PRs that delete assets on purpose. if: github.event.pull_request.user.login != 'docs-bot' && (github.repository == 'github/docs-internal' || github.repository == 'github/docs') runs-on: ubuntu-latest steps: diff --git a/.github/workflows/dont-delete-features.yml b/.github/workflows/dont-delete-features.yml index 66c5995c0f3f..172ba48603e8 100644 --- a/.github/workflows/dont-delete-features.yml +++ b/.github/workflows/dont-delete-features.yml @@ -1,14 +1,6 @@ name: Don't delete features -# **What it does**: -# If the PR (against main) involves deletion of features, if any of -# them are deletions or renames, post a comment, and ultimately -# fail the check. -# **Why we have it**: -# If you delete the reference to an image, the English content is fine -# because it no longer tries to use the feature that doesn't exist. -# But this is not the case for translations. -# **Who does it impact**: Docs content. +# Fails feature-deletion PRs because translations still use deleted English features. on: workflow_dispatch: @@ -60,6 +52,6 @@ jobs: - name: Ultimately fail the workflow for attention if: ${{ steps.comment.outputs.markdown != '' }} run: | - echo "More than 1 feature was deleted as part of this PR." - echo "See posted PR commented about how to get them back." + echo "This PR deletes or renames one or more feature files." + echo "See the PR comment for how to get them back." exit 1 diff --git a/.github/workflows/reviewers-docs-engineering.yml b/.github/workflows/reviewers-docs-engineering.yml index 1bd5ab675ba3..03ba53d2ee49 100644 --- a/.github/workflows/reviewers-docs-engineering.yml +++ b/.github/workflows/reviewers-docs-engineering.yml @@ -60,15 +60,25 @@ jobs: env: GH_TOKEN: ${{ secrets.DOCS_BOT_PAT_BASE }} PR_AUTHOR: ${{ github.event.pull_request.user.login }} + PR_NUMBER: ${{ github.event.pull_request.number }} + CHANGED_FILES: ${{ github.event.pull_request.changed_files }} + REPO: ${{ github.repository }} run: | if [ "$PR_AUTHOR" = "dependabot[bot]" ]; then echo "Author is Dependabot; skipping lockfile churn detection." echo "lockfile_only=false" >> "$GITHUB_OUTPUT" exit 0 fi - changed=$(gh pr diff "$PR" --name-only) + # `gh pr diff` fails above 300 files, and `gh pr view --json files` stops at 100. + changed=$(gh api --paginate "repos/$REPO/pulls/$PR_NUMBER/files" --jq '.[].filename') echo "Changed files:" echo "$changed" + # The files API returns at most 3,000 files. If the list is incomplete, request review anyway. + if [ "$(echo "$changed" | grep -c .)" -lt "$CHANGED_FILES" ]; then + echo "Listed fewer files than the PR changes; skipping lockfile churn detection." + echo "lockfile_only=false" >> "$GITHUB_OUTPUT" + exit 0 + fi lockfile=$(echo "$changed" | grep -c '^package-lock\.json$' || true) other_eng=$(echo "$changed" | grep -cE '(\.tsx?$|\.scss$|^src/|^package\.json$|^\.github/|^config/|^\.devcontainer/|Dockerfile)' || true) if [ "$lockfile" -gt 0 ] && [ "$other_eng" -eq 0 ]; then diff --git a/content/site-policy/acceptable-use-policies/github-appeal-and-reinstatement.md b/content/site-policy/acceptable-use-policies/github-appeal-and-reinstatement.md index d09806ebfc53..676572e6618e 100644 --- a/content/site-policy/acceptable-use-policies/github-appeal-and-reinstatement.md +++ b/content/site-policy/acceptable-use-policies/github-appeal-and-reinstatement.md @@ -31,6 +31,8 @@ If you'd like to seek Reinstatement or wish to Appeal an enforcement action on G * [GitHub Appeal and Reinstatement form](https://support.github.com/contact/reinstatement) * [npm Appeal and Reinstatement form](https://support.github.com/support/contact/product-selection/reinstatement-requests/npm-reinstatement-request) +If you are unable to sign in to your GitHub account, see [Unable to sign in](https://support.github.com/contact/cannot_sign_in) to contact GitHub Support. After you verify your email, you can access the appeal and reinstatement forms. + On GitHub, you may seek Reinstatement or Appeal a moderation decision for up to six months following the decision. GitHub may, in its discretion, refuse to consider any requests submitted more than six months after the decision. GitHub staff will review the information provided in the form to determine whether there is sufficient information to warrant Reinstatement or granting of an Appeal. diff --git a/package-lock.json b/package-lock.json index 3ed24cad3e95..c1adc8c5087b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1372,9 +1372,9 @@ } }, "node_modules/@grpc/grpc-js": { - "version": "1.14.4", - "resolved": "https://registry.npmjs.org/@grpc/grpc-js/-/grpc-js-1.14.4.tgz", - "integrity": "sha512-k9Dj3DV/itK9D06Y8f190Qgop7/Ui+D0njFV3LHMPwPT75DpXLQohE9Wmz0QElrJnzsjB7KPWiKJbOl7IPDArQ==", + "version": "1.14.5", + "resolved": "https://registry.npmjs.org/@grpc/grpc-js/-/grpc-js-1.14.5.tgz", + "integrity": "sha512-7VZM+SVdEcUUqSQeNI3zM8Qs/BhQKZndPo2h5VkYkAM8Iz0wJIa8mKV5ekQGqG8UUsnkQ0NMxIxwkIHYvj0qOw==", "license": "Apache-2.0", "dependencies": { "@grpc/proto-loader": "^0.8.0", diff --git a/package.json b/package.json index b176ae0f75f9..6bc63e168fe3 100644 --- a/package.json +++ b/package.json @@ -98,6 +98,7 @@ "symlink-from-local-repo": "tsx src/early-access/scripts/symlink-from-local-repo.ts", "sync-audit-log": "tsx src/audit-logs/scripts/sync.ts", "rebuild-audit-log-dedup": "tsx src/audit-logs/scripts/rebuild-dedup.ts", + "rebuild-github-apps-dedup": "tsx src/github-apps/scripts/rebuild-dedup.ts", "sync-codeql-cli": "tsx src/codeql-cli/scripts/sync.ts", "sync-graphql": "tsx src/graphql/scripts/sync.ts", "sync-rest": "tsx src/rest/scripts/update-files.ts", diff --git a/src/audit-logs/lib/deduplicate.ts b/src/audit-logs/lib/deduplicate.ts index c5ce3370b9c9..d4d349f43eb7 100644 --- a/src/audit-logs/lib/deduplicate.ts +++ b/src/audit-logs/lib/deduplicate.ts @@ -1,4 +1,4 @@ -import { existsSync } from 'fs' +import { existsSync, readdirSync, readFileSync, statSync } from 'fs' import { mkdir, writeFile } from 'fs/promises' import path from 'path' @@ -83,3 +83,44 @@ export async function writeDeduplicatedAuditLogData( `āœ… Deduplicated audit log data: ${totalEntries} total → ${uniqueEntries} unique entries (${dedupRate}% dedup), ${uniqueFields} unique field lists`, ) } + +// Rebuilds the deduplicated files from per-version JSON already on disk. +const NON_VERSION_DIRS = new Set(['shared']) + +function loadAuditLogDataFromDisk(): VersionedAuditLogData { + const auditLogData: VersionedAuditLogData = {} + + for (const version of readdirSync(AUDIT_LOG_DATA_DIR)) { + if (NON_VERSION_DIRS.has(version)) continue + const versionDir = path.join(AUDIT_LOG_DATA_DIR, version) + if (!statSync(versionDir).isDirectory()) continue + + const pages: Record = {} + for (const file of readdirSync(versionDir)) { + if (!file.endsWith('.json')) continue + const page = path.basename(file, '.json') + pages[page] = JSON.parse(readFileSync(path.join(versionDir, file), 'utf8')) + } + + if (Object.keys(pages).length > 0) { + auditLogData[version] = pages + } + } + + return auditLogData +} + +export async function rebuildAuditLogDedup() { + if (!existsSync(AUDIT_LOG_DATA_DIR)) { + throw new Error(`Audit log data directory not found: ${AUDIT_LOG_DATA_DIR}`) + } + + const auditLogData = loadAuditLogDataFromDisk() + const versionCount = Object.keys(auditLogData).length + if (versionCount === 0) { + throw new Error(`No per-version audit log data found in ${AUDIT_LOG_DATA_DIR}`) + } + + console.log(`\nā–¶ļø Rebuilding deduplicated format from ${versionCount} versions on disk...`) + await writeDeduplicatedAuditLogData(auditLogData) +} diff --git a/src/audit-logs/scripts/rebuild-dedup.ts b/src/audit-logs/scripts/rebuild-dedup.ts index 618e183810e8..115f609213d0 100644 --- a/src/audit-logs/scripts/rebuild-dedup.ts +++ b/src/audit-logs/scripts/rebuild-dedup.ts @@ -3,52 +3,6 @@ // It does not fetch github/audit-log-allowlists and does not need GITHUB_TOKEN. // Use it when src/audit-logs/data/shared or version-index.json is stale. // Run with npm run rebuild-audit-log-dedup. -import { existsSync, readdirSync, readFileSync, statSync } from 'fs' -import path from 'path' +import { rebuildAuditLogDedup } from '@/audit-logs/lib/deduplicate' -import { writeDeduplicatedAuditLogData } from '../lib/deduplicate' -import type { AuditLogEventT, VersionedAuditLogData } from '../types' - -const AUDIT_LOG_DATA_DIR = 'src/audit-logs/data' - -const NON_VERSION_DIRS = new Set(['shared']) - -function loadAuditLogDataFromDisk(): VersionedAuditLogData { - const auditLogData: VersionedAuditLogData = {} - - for (const version of readdirSync(AUDIT_LOG_DATA_DIR)) { - if (NON_VERSION_DIRS.has(version)) continue - const versionDir = path.join(AUDIT_LOG_DATA_DIR, version) - if (!statSync(versionDir).isDirectory()) continue - - const pages: Record = {} - for (const file of readdirSync(versionDir)) { - if (!file.endsWith('.json')) continue - const page = path.basename(file, '.json') - pages[page] = JSON.parse(readFileSync(path.join(versionDir, file), 'utf8')) - } - - if (Object.keys(pages).length > 0) { - auditLogData[version] = pages - } - } - - return auditLogData -} - -async function main() { - if (!existsSync(AUDIT_LOG_DATA_DIR)) { - throw new Error(`Audit log data directory not found: ${AUDIT_LOG_DATA_DIR}`) - } - - const auditLogData = loadAuditLogDataFromDisk() - const versionCount = Object.keys(auditLogData).length - if (versionCount === 0) { - throw new Error(`No per-version audit log data found in ${AUDIT_LOG_DATA_DIR}`) - } - - console.log(`\nā–¶ļø Rebuilding deduplicated format from ${versionCount} versions on disk...`) - await writeDeduplicatedAuditLogData(auditLogData) -} - -main() +await rebuildAuditLogDedup() diff --git a/src/content-render/unified/wrap-procedural-images.ts b/src/content-render/unified/wrap-procedural-images.ts index 938e6d184ff6..21289729dc13 100644 --- a/src/content-render/unified/wrap-procedural-images.ts +++ b/src/content-render/unified/wrap-procedural-images.ts @@ -18,15 +18,15 @@ function insideOlLi(ancestors: Parent[]): boolean { return false } -// When a writer leaves a blank line before a list image, Markdown wraps it in a paragraph. -// The visitor skips that branch because the paragraph already adds spacing, and div wrappers -// inside paragraphs cause hydration mismatches. +// When a writer leaves a blank line before a list image, Markdown wraps it in a paragraph, +// possibly with a link in between. The visitor skips that branch because the paragraph +// already adds spacing, and div wrappers inside paragraphs cause hydration mismatches. function visitor(node: Element, ancestors: Parent[]): void { if (!insideOlLi(ancestors)) return const parent = ancestors.at(-1) if (!parent || !parent.children) return - if ((parent as Element).tagName === 'p') return + if (ancestors.some((ancestor) => (ancestor as Element).tagName === 'p')) return const shallowClone: Element = Object.assign({}, node) shallowClone.tagName = 'div' diff --git a/src/fixtures/fixtures/content/get-started/images/emoji-and-decorative-images.md b/src/fixtures/fixtures/content/get-started/images/emoji-and-decorative-images.md new file mode 100644 index 000000000000..73a2c0067932 --- /dev/null +++ b/src/fixtures/fixtures/content/get-started/images/emoji-and-decorative-images.md @@ -0,0 +1,17 @@ +--- +title: Emoji and decorative images +intro: 'An intro with an emoji :strawberry: and an image Intro image' +versions: + fpt: '*' + ghes: '*' + ghec: '*' +contentType: how-tos +--- + +## An emoji image + +Typing `:strawberry:` renders the emoji :strawberry: inline. + +## A decorative image + + diff --git a/src/fixtures/fixtures/content/get-started/images/images-in-lists.md b/src/fixtures/fixtures/content/get-started/images/images-in-lists.md index 237ad23da21c..897001107654 100644 --- a/src/fixtures/fixtures/content/get-started/images/images-in-lists.md +++ b/src/fixtures/fixtures/content/get-started/images/images-in-lists.md @@ -14,3 +14,10 @@ contentType: how-tos 1. French press is also great ![test image](/assets/images/_fixtures/electrocat.png) 3. Drip coffee is not so great. + +## A numbered list with a linked image + +1. Pour-over takes patience. +1. The linked image renders like this: + + [![Linked test image](/assets/images/_fixtures/electrocat.png)](https://github.com) diff --git a/src/fixtures/fixtures/content/get-started/images/index.md b/src/fixtures/fixtures/content/get-started/images/index.md index ef16588d1a04..a6b7262d8d46 100644 --- a/src/fixtures/fixtures/content/get-started/images/index.md +++ b/src/fixtures/fixtures/content/get-started/images/index.md @@ -10,4 +10,5 @@ children: - /images-in-lists - /link-to-image - /retina-image + - /emoji-and-decorative-images --- diff --git a/src/fixtures/tests/images.ts b/src/fixtures/tests/images.ts index b9aee2601786..796d79eb2104 100644 --- a/src/fixtures/tests/images.ts +++ b/src/fixtures/tests/images.ts @@ -37,6 +37,7 @@ describe('render Markdown image tags', () => { expect(src).toMatch(/^\/assets\/cb-\w+\/images\/_fixtures\/screenshot\.png$/) const alt = imgs.attr('alt') expect(alt).toBe('This is the alt text') + expect(imgs.attr('class')).toMatch(/Image/) const res = await get(srcset!.split(' ')[0], { responseType: 'buffer' }) expect(res.statusCode).toBe(200) @@ -69,6 +70,16 @@ describe('render Markdown image tags', () => { expect(imageSpan.length).toBe(1) }) + // A div inside a paragraph is invalid HTML and breaks hydration. + test('linked image in a list paragraph has no wrapper', async () => { + const $: CheerioAPI = await getDOM('/get-started/images/images-in-lists') + + const link = $('#article-contents ol > li > p > a[href="https://github.com"]') + expect(link.length).toBe(1) + expect($('div', link).length).toBe(0) + expect($('img', link).attr('alt')).toBe('Linked test image') + }) + test("links directly to images aren't rewritten", async () => { const $: CheerioAPI = await getDOM('/get-started/images/link-to-image') // The fixture has one article link; header links are out of scope. @@ -79,4 +90,34 @@ describe('render Markdown image tags', () => { const res = await head(links.attr('href')!) expect(res.statusCode).toBe(200) }) + + test('emoji images stay plain img elements', async () => { + const $: CheerioAPI = await getDOM('/get-started/images/emoji-and-decorative-images') + const emoji = $( + '#article-contents img[src^="https://github.githubassets.com/images/icons/emoji"]', + ) + expect(emoji.length).toBe(1) + expect(emoji.attr('alt')).toBe(':strawberry:') + expect(emoji.attr('class')).toBeUndefined() + }) + + test('images without alt text render an empty alt', async () => { + const $: CheerioAPI = await getDOM('/get-started/images/emoji-and-decorative-images') + const imgs = $('#article-contents img[src*="/images/_fixtures/screenshot.png"]') + expect(imgs.length).toBe(1) + expect(imgs.attr('alt')).toBe('') + expect(imgs.attr('class')).toMatch(/Image/) + }) + + test('images in RenderedHTML intros use the Brand Image component', async () => { + const $: CheerioAPI = await getDOM('/get-started/images/emoji-and-decorative-images') + const lead = $('[data-container="lead"]') + const image = $('img[src*="/images/_fixtures/electrocat.png"]', lead) + expect(image.length).toBe(1) + expect(image.attr('alt')).toBe('Intro image') + expect(image.attr('class')).toMatch(/Image/) + const emoji = $('img[src^="https://github.githubassets.com/images/icons/emoji"]', lead) + expect(emoji.length).toBe(1) + expect(emoji.attr('class')).toBeUndefined() + }) }) diff --git a/src/frame/components/article/ViewMarkdownButton.module.scss b/src/frame/components/article/ViewMarkdownButton.module.scss index 46dbbabec6fc..1c253f3c58c1 100644 --- a/src/frame/components/article/ViewMarkdownButton.module.scss +++ b/src/frame/components/article/ViewMarkdownButton.module.scss @@ -23,7 +23,11 @@ // @primer/react ActionMenu.Button, so several rules here make them agree. Where // a doubled class or !important appears, it beats a library rule; the specificity // that forced it is noted. -.button { +// +// The doubled classes beat @primer/react ButtonBase rules, which have one-class +// specificity. Without them the winner depends on stylesheet load order, and a +// dev hot reload or chunk reorder brings back the chevron's border and radius. +.button.button { display: inline-flex; align-items: center; justify-content: center; @@ -75,7 +79,7 @@ // The pill's left end is rounded outside and square where it meets the chevron. // The radius token is 624.9375rem rather than 9999px, and nothing clips it. -.copyButton { +.copyButton.copyButton { border-radius: var(--brand-borderRadius-full, 624.9375rem) 0 0 var(--brand-borderRadius-full, 624.9375rem); // Less padding on the seam side so the label sits closer to the chevron than @@ -92,7 +96,7 @@ // The pill's right end: square at the seam, rounded outside. Width matches the // height for an even chevron target. -.dropdownButton { +.dropdownButton.dropdownButton { border-radius: 0 var(--brand-borderRadius-full, 624.9375rem) var(--brand-borderRadius-full, 624.9375rem) 0; width: 28px; diff --git a/src/frame/components/hooks/useHasAccount.ts b/src/frame/components/hooks/useHasAccount.ts index 589eaf4997b1..047e78748a0c 100644 --- a/src/frame/components/hooks/useHasAccount.ts +++ b/src/frame/components/hooks/useHasAccount.ts @@ -2,15 +2,9 @@ import { useState, useEffect } from 'react' import Cookies from '@/frame/components/lib/cookies' import { COLOR_MODE_COOKIE_NAME, PREFERRED_COLOR_MODE_COOKIE_NAME } from '@/frame/lib/constants' -// Measure if the user has a github.com account and signed in during this session. -// The github.com sends the color_mode cookie every request when you sign in, -// but does not delete the color_mode cookie on sign out. -// You do not need to change your color mode settings to get this cookie, -// this applies to every user regardless of if they changed this setting. -// To test this, try a private browser tab. -// We are using the color_mode cookie because it is not HttpOnly. -// For users that haven't changed their session cookies recently, -// we also can check for the browser-set `preferred_color_mode` cookie. +// github.com sends color_mode on every signed-in request and leaves it on sign-out. +// Every account gets that client-readable cookie, regardless of color-mode settings. +// preferred_color_mode covers sessions that still use the browser-set cookie. export function useHasAccount() { const [hasAccount, setHasAccount] = useState(null) diff --git a/src/frame/components/hooks/useQueryParam.ts b/src/frame/components/hooks/useQueryParam.ts index dffcdf121ece..6a7fdac5d15f 100644 --- a/src/frame/components/hooks/useQueryParam.ts +++ b/src/frame/components/hooks/useQueryParam.ts @@ -1,5 +1,4 @@ -// A generic hook for getting and setting a query parameter without reloading the page -// The `queryParam` variable returned from this method are stateful and will be set to the query param on page load +// Shallow URL updates keep query controls in sync without reloading the page. import { useRouter } from 'next/router' import { useState, useEffect } from 'react' @@ -11,7 +10,6 @@ type UseQueryParamReturn = { setQueryParam: (value: T) => void } -// Overloads so we can use this for a boolean or string query param export function useQueryParam(queryParamKey: string, isBoolean: true): UseQueryParamReturn export function useQueryParam(queryParamKey: string, isBoolean?: false): UseQueryParamReturn export function useQueryParam( @@ -24,7 +22,7 @@ export function useQueryParam( const [debug, setDebug] = useState(false) const queryParam: string | boolean = isBoolean ? queryParamString === 'true' : queryParamString - // Only set the initial query param values on page load, the rest of the time we use React state + // Read URL values only on route changes, so React state owns later edits. useEffect(() => { let initialQueryParam = '' const paramValue = router.query[queryParamKey] diff --git a/src/frame/components/lib/cookies.ts b/src/frame/components/lib/cookies.ts index 7916467b3c39..170acac51953 100644 --- a/src/frame/components/lib/cookies.ts +++ b/src/frame/components/lib/cookies.ts @@ -1,7 +1,6 @@ import Cookies from 'js-cookie' -// This library only works client side, -// so on the server side we return a mock. +// js-cookie reads document, so server rendering gets a no-op mock. export default typeof document === 'undefined' ? { get: () => undefined, diff --git a/src/frame/components/lib/prefetch.ts b/src/frame/components/lib/prefetch.ts index 7213b3cacca0..ee03f4fccef3 100644 --- a/src/frame/components/lib/prefetch.ts +++ b/src/frame/components/lib/prefetch.ts @@ -3,24 +3,20 @@ import type { useRouter } from 'next/router' type Router = ReturnType -// Session-lived de-dupe set. Module scope (not a per-component useRef) so it -// survives the sidebar's per-navigation remount. Otherwise the cache would -// reset every nav and "de-duped per href" would only hold within a single page. -// Bounded by the number of distinct internal hrefs the user hovers/focuses. +// Keep one de-dupe set for the browser session so sidebar remounts do not refetch. +// The set can only grow to the distinct internal hrefs the user hovers or focuses. const prefetchedHrefs = new Set() -// Brand NavList/Breadcrumbs items render plain s navigated via router.push, -// so Next.js doesn't prefetch their destinations the way next/link would. This -// hook warms a route on hover/focus. +// Brand NavList and Breadcrumbs render plain anchors navigated through router.push, +// so Next.js will not prefetch them like next/link. This hook warms routes on +// hover and focus. // -// Scope of the benefit here is small on purpose: every article is a -// getServerSideProps route, and router.prefetch only fetches the JS bundle for -// those (not the getServerSideProps data), while page data is already served fast -// from the Fastly edge. All articles also share one [...restPage] bundle, so after -// the first navigation there's usually nothing left to fetch. It still helps the -// first cold visit, and would fetch data too if a route ever moves to -// getStaticProps. router.prefetch is production-only (no prefetch in dev); we -// de-dupe per href so re-entering a link doesn't re-issue. +// The benefit stays small: articles use getServerSideProps, so router.prefetch +// fetches only the JS bundle, not data. Fastly already serves page data quickly, +// and articles share one [...restPage] bundle, so prefetch usually only helps the +// first cold visit. If a route moves to getStaticProps, this also fetches data. +// router.prefetch runs only in production. prefetchedHrefs keeps each href to one +// successful request; failed hrefs leave the set so a later hover can retry. export function usePrefetchOnInteraction() { const prefetch = useCallback(async (router: Router, href: string) => { if ( @@ -32,12 +28,10 @@ export function usePrefetchOnInteraction() { } prefetchedHrefs.add(href) try { - // hrefs already include the locale prefix (e.g. /en/...), so disable Next.js - // locale handling to match the router.push calls at the click sites. + // hrefs already include the locale prefix, so locale: false matches click-site router.push. await router.prefetch(href, undefined, { locale: false }) } catch { - // A transient chunk/network failure shouldn't permanently suppress retries; - // un-mark so a later hover/focus can try again (mirrors next/link). + // Delete failed hrefs so transient chunk or network errors can retry like next/link. prefetchedHrefs.delete(href) } }, []) diff --git a/src/frame/components/lib/toggle-annotations.ts b/src/frame/components/lib/toggle-annotations.ts index 628391ec7c1e..04a032231c8e 100644 --- a/src/frame/components/lib/toggle-annotations.ts +++ b/src/frame/components/lib/toggle-annotations.ts @@ -8,7 +8,6 @@ enum annotationMode { Inline = 'inline', } -// Returns the mode if it is 'beside' or 'inline', otherwise falls back to Beside. function validateMode(mode?: string) { if (mode === annotationMode.Beside || mode === annotationMode.Inline) return mode else return annotationMode.Beside @@ -39,9 +38,6 @@ export default function toggleAnnotation() { } } -// Sets aria-current on every button whose value matches the validated mode, and -// clears it from the rest. Missing or invalid modes validate to Beside. Throws if -// no button matches. function setActive(annotationButtons: Array, targetMode?: string) { const activeElements: Array = [] targetMode = validateMode(targetMode) diff --git a/src/frame/components/page-header/ActionMenuTrigger.tsx b/src/frame/components/page-header/ActionMenuTrigger.tsx index bcf7db25490b..5177684b0a09 100644 --- a/src/frame/components/page-header/ActionMenuTrigger.tsx +++ b/src/frame/components/page-header/ActionMenuTrigger.tsx @@ -1,13 +1,11 @@ import type { ComponentProps, ComponentType, ReactNode } from 'react' import { ActionMenu } from '@primer/react-brand' -// Brand's ActionMenu.Button hardcodes `trailingVisual={}` and its -// props type does not declare `trailingVisual`. It spreads rest props *after* that -// default, so a caller-supplied icon still wins at runtime, and Docs 2026 specifies a -// filled triangle caret rather than a chevron. This cast is the single deliberate -// divergence from the stock component's typed API, and it is shared by both header -// pickers so there is only one line to fix. If a future @primer/react-brand release -// destructures `trailingVisual` out of its rest props, this is the line that stops +// Brand's ActionMenu.Button hardcodes trailingVisual={}, and its +// props omit trailingVisual. It spreads rest props after that default, so caller +// icons still win at runtime. Docs 2026 needs a filled triangle caret, not a +// chevron. This cast is the shared typed API divergence for both header pickers. If +// @primer/react-brand destructures trailingVisual out of rest props, this line stops // working and the caret silently reverts to a chevron. type WithTrailingVisual = { trailingVisual?: ReactNode } diff --git a/src/frame/components/page-header/Breadcrumbs.tsx b/src/frame/components/page-header/Breadcrumbs.tsx index cf6ac585308d..e0f7f87c533f 100644 --- a/src/frame/components/page-header/Breadcrumbs.tsx +++ b/src/frame/components/page-header/Breadcrumbs.tsx @@ -10,10 +10,8 @@ import { usePrefetchOnInteraction } from '@/frame/components/lib/prefetch' type Props = { inHeader?: boolean - // Placement variant. Defaults derived from `inHeader` for back-compat: - // - 'in-article' (default): sits above the article; hides the last (current) crumb. - // - 'header': the mobile subnav row; shows all crumbs. - // - 'bar': the Docs 2026 secondary bar; shows all crumbs and a leading Home crumb. + // Defaults preserve inHeader callers: in-article hides the current crumb, header + // shows all crumbs, and bar shows all crumbs plus Home. variant?: 'in-article' | 'header' | 'bar' } @@ -29,15 +27,12 @@ export const Breadcrumbs = ({ inHeader, variant }: Props) => { const { t } = useTranslation('header') const prefetchHref = usePrefetchOnInteraction() - // Warm a breadcrumb destination on hover/focus. BrandBreadcrumbs.Item renders a - // plain navigated via router.push, so Next.js won't prefetch it otherwise. + // BrandBreadcrumbs.Item renders a plain anchor, so warm hrefs Next.js will not prefetch. const prefetch = useCallback((href: string) => prefetchHref(router, href), [router, prefetchHref]) const placement = variant ?? (inHeader ? 'header' : 'in-article') - // Only the in-article placement hides the current (last) crumb; the header and - // secondary-bar placements show the full trail. + // In-article crumbs hide the current page, while header and bar show the full trail. const hideLastCrumb = placement === 'in-article' - // The secondary bar leads with a Home crumb (replacing the old "← Home" rail link). const showHomeCrumb = placement === 'bar' const testId = placement === 'bar' @@ -50,10 +45,7 @@ export const Breadcrumbs = ({ inHeader, variant }: Props) => { currentVersion === DEFAULT_VERSION ? '' : `/${currentVersion}` }` - // BrandBreadcrumbs.Item renders a plain , so intercept clicks to restore - // next/link-style client-side navigation. Modifier/middle clicks fall through - // to the browser so open-in-new-tab still works, and the keeps the - // links crawlable for SSR. + // Restore next/link navigation; modifier, middle, and external clicks keep browser behavior. const handleClick = (event: MouseEvent, href: string) => { if ( event.defaultPrevented || @@ -67,8 +59,7 @@ export const Breadcrumbs = ({ inHeader, variant }: Props) => { return } event.preventDefault() - // hrefs already include the locale prefix (e.g. /en/...), so disable - // Next.js locale handling to avoid double-prefixing. + // hrefs already include the locale prefix, so locale: false prevents double-prefixing. router.push(href, undefined, { locale: false }) } @@ -97,9 +88,7 @@ export const Breadcrumbs = ({ inHeader, variant }: Props) => { ) } - // The last crumb is the page being viewed, so it isn't a link to - // itself: brand's `selected` renders it as static text carrying - // aria-current="page" (and pointer-events: none) instead of an . + // Brand selected renders the current page as aria-current static text, not a self-link. const isCurrent = i === arr.length - 1 return ( { onFocus: () => prefetch(breadcrumb.href!), })} className={cx( - // Show the last breadcrumb if it's in the header/bar, but not if it's in the article. - // If there's only 1 breadcrumb, show it. + // Header and bar show current crumb; in-article hides it unless it stands alone. hideLastCrumb && isCurrent && arr.length !== 1 && 'd-none', )} > diff --git a/src/frame/components/page-header/BreadcrumbsScroller.module.scss b/src/frame/components/page-header/BreadcrumbsScroller.module.scss index 1b67a41244a2..e2b62c3c8743 100644 --- a/src/frame/components/page-header/BreadcrumbsScroller.module.scss +++ b/src/frame/components/page-header/BreadcrumbsScroller.module.scss @@ -1,7 +1,5 @@ -// Horizontal scroll wrapper for the secondary-bar breadcrumbs. When the trail -// overflows, the scroll area is anchored to the right (current page visible) -// and a left chevron reveals ancestor crumbs. A left-edge fade signals that -// there is more content scrolled off to the left. +// Overflowing secondary-bar trails anchor right so the current page stays visible. +// A left chevron and fade reveal ancestor crumbs. .scroller { display: flex; @@ -12,12 +10,9 @@ position: relative; } -// The chevrons overlay the ends of the scroll area (absolutely positioned) -// rather than sitting in the flex flow. This keeps the scroll area full-width -// and constant, so its scrollable range never shifts when a chevron toggles, -// while a hidden chevron leaves NO reserved gap at that edge. Each chevron -// carries the secondary bar's own background so a crumb scrolling underneath is -// masked rather than showing through the icon. +// Chevrons overlay the scroll area instead of sitting in flex flow, so the scroll +// range stays stable when visibility changes. Hidden chevrons reserve no gap. +// Their secondary-bar background masks crumbs as they pass underneath. .leftChevron { position: absolute; left: 0; @@ -25,8 +20,7 @@ transform: translateY(-50%); z-index: 1; color: var(--brand-color-text-muted, #58635b); - // Must stay whatever opaque token DocsSecondaryBar paints on `.bar`, or a crumb - // scrolling underneath shows through. + // Match DocsSecondaryBar .bar's opaque canvas, or passing crumbs show through. background-color: var(--brand-color-canvas-default); } @@ -37,14 +31,13 @@ transform: translateY(-50%); z-index: 1; color: var(--brand-color-text-muted, #58635b); - // Must match `.bar` — see .leftChevron. + // Keep this equal to .bar; see .leftChevron. background-color: var(--brand-color-canvas-default); } -// On hover, keep the solid canvas background (Primer's invisible IconButton -// otherwise swaps in a translucent tint, letting crumbs bleed through the -// button) and instead signal interactivity by bolding the chevron: a -// currentColor stroke thickens the fill-based octicon glyph. +// Keep the solid canvas on hover because Primer's invisible IconButton uses a +// translucent tint that lets crumbs bleed through. A currentColor stroke thickens +// the fill-based octicon glyph for hover feedback. .leftChevron:hover, .rightChevron:hover { background-color: var(--brand-color-canvas-default) !important; @@ -56,9 +49,8 @@ } } -// Hidden chevrons are removed from view and interaction (and from the tab -// order / a11y tree in the component). Because they're absolutely positioned -// they already occupy no layout space, so the scroll area is unaffected. +// Hidden chevrons leave view, interaction, tab order, and the accessibility tree. +// Absolute positioning already keeps them out of layout. .chevronHidden { visibility: hidden; pointer-events: none; @@ -69,12 +61,9 @@ min-width: 0; overflow-x: auto; overflow-y: hidden; - // Constant 16px on both ends: keeps a crumb off the border at the scroll - // extremes, and mid-scroll the chevron overlays the end of the trail (its - // solid background masks whatever passes underneath). Keeping this padding - // fixed, never toggled with the chevrons, means the scroll content's width - // never changes, so toggling a chevron can't reflow the trail or nudge the - // scroll position (no hop, no mount bounce, no click-lands-short). + // Fixed 16px end padding keeps crumbs clear at scroll extremes and gives + // overlaid chevrons room to mask mid-scroll crumbs. Never toggle it with + // chevron visibility, or content width changes and the trail position jumps. padding: 0 16px; scrollbar-width: none; // Firefox: hide the horizontal scrollbar -ms-overflow-style: none; @@ -83,12 +72,11 @@ display: none; // Chrome/Safari } - // Make the brand Breadcrumbs