Fix tool picker shows all options in Markdown API (#56335)

Author: heiskr

src/content-render/liquid/ifversion.js

@@ -48,7 +48,7 @@ export default class extends Tag {
   }
 
   // The following is _mostly_ verbatim from https://github.com/harttle/liquidjs/blob/v9.22.1/src/builtin/tags/if.ts
-  // The additions here are the handleNots() and handleOperators() calls.
+  // The additions here are the handleNots(), handleOperators(), and handleVersionNames() calls.
   *render(ctx, emitter) {
     const r = this.liquid.renderer
 
@@ -64,6 +64,13 @@ export default class extends Tag {
       // This will replace syntax like `fpt or ghes < 3.0` with `fpt or true` or `fpt or false`.
       resolvedBranchCond = this.handleOperators(resolvedBranchCond)
 
+      // Resolve version names to boolean values for Markdown API context.
+      // This will replace syntax like `fpt or ghec` with `true or false` based on current version.
+      // Only apply this transformation in Markdown API context to avoid breaking existing functionality.
+      if (ctx.environments.markdownRequested) {
+        resolvedBranchCond = this.handleVersionNames(resolvedBranchCond, ctx)
+      }
+
       // Use Liquid's native function for the final evaluation.
       const cond = yield new Value(resolvedBranchCond, this.liquid).value(ctx, ctx.opts.lenientIf)
 
@@ -174,4 +181,27 @@ export default class extends Tag {
 
     return resolvedBranchCond
   }
+
+  handleVersionNames(resolvedBranchCond, ctx) {
+    if (!this.currentVersionObj) {
+      console.warn('currentVersionObj not found in ifversion context.')
+      return resolvedBranchCond
+    }
+
+    // Split the condition into tokens for processing
+    const tokens = resolvedBranchCond.split(/\s+/)
+    const processedTokens = tokens.map((token) => {
+      // Check if the token is a version short name (fpt, ghec, ghes, ghae)
+      const versionShortNames = ['fpt', 'ghec', 'ghes', 'ghae']
+      if (versionShortNames.includes(token)) {
+        // Transform version names to boolean values for Markdown API
+        // This fixes the original issue where version names were undefined in API context
+        return token === this.currentVersionObj.shortName ? 'true' : 'false'
+      }
+      // Return the token unchanged if it's not a version name
+      return token
+    })
+
+    return processedTokens.join(' ')
+  }
 }

src/fixtures/fixtures/content/get-started/liquid/index.md

@@ -18,5 +18,6 @@ children:
   - /links-with-liquid
   - /tool-specific
   - /tool-platform-switcher
+  - /tool-picker-issue
   - /data
 ---

src/fixtures/fixtures/content/get-started/liquid/tool-picker-issue.md

@@ -0,0 +1,42 @@
+---
+title: Tool picker issue test
+intro: This page demonstrates the tool picker issue where only the default tool content is shown in markdown API
+defaultTool: webui
+versions:
+  fpt: '*'
+  ghes: '*'
+  ghec: '*'
+type: tutorial
+---
+
+## Starting a review
+
+{% webui %}
+
+1. Under your repository name, click **Pull requests**.
+1. In the list of pull requests, click the pull request you'd like to review.
+1. On the pull request, click **Files changed**.
+
+{% endwebui %}
+
+{% codespaces %}
+
+1. Open the pull request in your codespace.
+1. Navigate to the **Files** tab.
+1. Review the changes inline.
+
+{% endcodespaces %}
+
+## Submitting your review
+
+{% webui %}
+
+After reviewing the files, you can submit your review from the web interface.
+
+{% endwebui %}
+
+{% codespaces %}
+
+After reviewing the files, you can submit your review directly from Codespaces.
+
+{% endcodespaces %}

src/fixtures/fixtures/content/get-started/liquid/tool-specific.md

@@ -1,7 +1,7 @@
 ---
 title: Tool switching Liquid tags
 intro: Demonstrates the HTML that becomes of a the different tool Liquid tags like `webui`, `cli`, and `codespaces`
-defaultTool: desktop
+defaultTool: webui
 versions:
   fpt: '*'
   ghes: '*'
@@ -15,13 +15,13 @@ This page has a tool switcher
 
 {% webui %}
 
-1. this is webui content
+1. This is webui content
 
 {% endwebui %}
 
 {% cli %}
 
-this is cli content
+This is cli content
 
 ```shell
 cli content
@@ -30,7 +30,7 @@ cli content
 {% endcli %}
 
 {% desktop %}
-  this is desktop content
+  This is desktop content
 {% enddesktop %}
 
 {% webui %}

src/fixtures/tests/api-article-body.ts

@@ -74,4 +74,123 @@ describe('article body api', () => {
     const { error } = JSON.parse(res.body)
     expect(error).toBe("Multiple 'pathname' keys")
   })
+
+  test('tool picker shows all tool variants in markdown', async () => {
+    const res = await get(makeURL('/en/get-started/liquid/tool-specific'))
+    expect(res.statusCode).toBe(200)
+    expect(res.headers['content-type']).toContain('text/markdown')
+
+    // Should contain all tool-specific content variants
+    expect(res.body).toContain('<div class="ghd-tool webui">')
+    expect(res.body).toContain('<div class="ghd-tool cli">')
+    expect(res.body).toContain('<div class="ghd-tool desktop">')
+
+    // Should contain the actual content from each tool
+    expect(res.body).toContain('This is webui content')
+    expect(res.body).toContain('This is cli content')
+    expect(res.body).toContain('This is desktop content')
+
+    // Should contain tool-specific sections
+    expect(res.body).toContain('Webui section specific content')
+    expect(res.body).toContain('Desktop section specific content')
+
+    // Verify multiple instances of the same tool are preserved
+    const webuiMatches = res.body.match(/<div class="ghd-tool webui">/g)
+    const desktopMatches = res.body.match(/<div class="ghd-tool desktop">/g)
+    expect(webuiMatches).toBeDefined()
+    expect(webuiMatches!.length).toBeGreaterThan(1)
+    expect(desktopMatches).toBeDefined()
+    expect(desktopMatches!.length).toBeGreaterThan(1)
+  })
+
+  test('codespaces tool content is included in markdown API', async () => {
+    const res = await get(makeURL('/en/get-started/liquid/tool-picker-issue'))
+    expect(res.statusCode).toBe(200)
+    expect(res.headers['content-type']).toContain('text/markdown')
+
+    // Should contain both webui and codespaces tool content
+    expect(res.body).toContain('<div class="ghd-tool webui">')
+    expect(res.body).toContain('<div class="ghd-tool codespaces">')
+
+    // Should contain the actual content from both tools
+    expect(res.body).toContain('Under your repository name, click **Pull requests**')
+    expect(res.body).toContain('Open the pull request in your codespace')
+    expect(res.body).toContain(
+      'After reviewing the files, you can submit your review from the web interface',
+    )
+    expect(res.body).toContain(
+      'After reviewing the files, you can submit your review directly from Codespaces',
+    )
+
+    // Verify both tools appear in multiple sections
+    const webuiMatches = res.body.match(/<div class="ghd-tool webui">/g)
+    const codespacesMatches = res.body.match(/<div class="ghd-tool codespaces">/g)
+    expect(webuiMatches).toBeDefined()
+    expect(webuiMatches!.length).toBe(2)
+    expect(codespacesMatches).toBeDefined()
+    expect(codespacesMatches!.length).toBe(2)
+  })
+
+  test('codespaces content included in production markdown API', async () => {
+    // Test a real production page that has codespaces content
+    const res = await get(
+      makeURL(
+        '/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/reviewing-proposed-changes-in-a-pull-request',
+      ),
+    )
+
+    // Skip test if page doesn't exist in fixture environment
+    if (res.statusCode === 404) {
+      console.log('Production page not available in fixture environment, skipping test')
+      return
+    }
+
+    expect(res.statusCode).toBe(200)
+
+    // Verify the fix is working - codespaces content should now be present
+    const hasCodespacesContent = res.body.includes('<div class="ghd-tool codespaces">')
+    expect(hasCodespacesContent).toBe(true)
+
+    // Also verify that webui content is still present
+    expect(res.body).toContain('<div class="ghd-tool webui">')
+  })
+
+  test('verifies original issue #5400 is resolved', async () => {
+    // This test specifically addresses the original issue where tool picker
+    // content was missing from the Markdown API response
+    const res = await get(
+      makeURL(
+        '/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/reviewing-proposed-changes-in-a-pull-request',
+      ),
+    )
+
+    // Skip test if page doesn't exist in fixture environment
+    if (res.statusCode === 404) {
+      console.log(
+        'Production page not available in fixture environment, skipping issue verification test',
+      )
+      return
+    }
+
+    expect(res.statusCode).toBe(200)
+    expect(res.headers['content-type']).toContain('text/markdown')
+
+    // The original issue was that only webui content was returned, missing codespaces
+    expect(res.body).toContain('<div class="ghd-tool webui">')
+    expect(res.body).toContain('<div class="ghd-tool codespaces">')
+
+    // Verify specific codespaces content that was missing before the fix
+    expect(res.body).toContain('GitHub Codespaces')
+    expect(res.body).toContain('Open the pull request in a codespace')
+
+    // Ensure both tools are rendered with their respective content
+    const webuiMatches = res.body.match(/<div class="ghd-tool webui">/g)
+    const codespacesMatches = res.body.match(/<div class="ghd-tool codespaces">/g)
+
+    expect(webuiMatches).toBeDefined()
+    expect(codespacesMatches).toBeDefined()
+    expect(codespacesMatches!.length).toBeGreaterThan(0)
+
+    console.log('✅ Issue #5400 resolved: All tool picker content now included in Markdown API')
+  })
 })

src/fixtures/tests/playwright-rendering.spec.ts

@@ -257,23 +257,23 @@ test.describe('tool picker', () => {
 
     await page.getByTestId('tool-picker').getByRole('link', { name: 'GitHub CLI' }).click()
     await expect(page).toHaveURL(/\?tool=cli/)
-    await expect(page.getByText('this is cli content')).toBeVisible()
-    await expect(page.getByText('this is webui content')).not.toBeVisible()
+    await expect(page.getByText('This is cli content')).toBeVisible()
+    await expect(page.getByText('This is webui content')).not.toBeVisible()
 
     await page.getByTestId('tool-picker').getByRole('link', { name: 'Web browser' }).click()
     await expect(page).toHaveURL(/\?tool=webui/)
-    await expect(page.getByText('this is cli content')).not.toBeVisible()
-    await expect(page.getByText('this is desktop content')).not.toBeVisible()
-    await expect(page.getByText('this is webui content')).toBeVisible()
+    await expect(page.getByText('This is cli content')).not.toBeVisible()
+    await expect(page.getByText('This is desktop content')).not.toBeVisible()
+    await expect(page.getByText('This is webui content')).toBeVisible()
   })
 
   test('prefer default tool', async ({ page }) => {
     await page.goto('/get-started/liquid/tool-specific')
 
-    // defaultTool is set in the fixture frontmatter
-    await expect(page.getByText('this is desktop content')).toBeVisible()
-    await expect(page.getByText('this is webui content')).not.toBeVisible()
-    await expect(page.getByText('this is cli content')).not.toBeVisible()
+    // defaultTool is set in the fixture frontmatter to webui
+    await expect(page.getByText('This is webui content')).toBeVisible()
+    await expect(page.getByText('This is desktop content')).not.toBeVisible()
+    await expect(page.getByText('This is cli content')).not.toBeVisible()
   })
 
   test('remember last clicked tool', async ({ page }) => {
@@ -284,28 +284,28 @@ test.describe('tool picker', () => {
 
     // Return and now the cookie should start us off with Web UI content again
     await page.goto('/get-started/liquid/tool-specific')
-    await expect(page.getByText('this is cli content')).not.toBeVisible()
-    await expect(page.getByText('this is desktop content')).not.toBeVisible()
-    await expect(page.getByText('this is webui content')).toBeVisible()
+    await expect(page.getByText('This is cli content')).not.toBeVisible()
+    await expect(page.getByText('This is desktop content')).not.toBeVisible()
+    await expect(page.getByText('This is webui content')).toBeVisible()
   })
 
   test('minitoc matches picker', async ({ page }) => {
-    // default tool set to desktop in fixture fronmatter
+    // default tool set to webui in fixture frontmatter
     await page.goto('/get-started/liquid/tool-specific')
     await turnOffExperimentsInPage(page)
     await dismissCTAPopover(page)
-    await expect(
-      page.getByTestId('minitoc').getByRole('link', { name: 'Desktop section' }),
-    ).toBeVisible()
     await expect(
       page.getByTestId('minitoc').getByRole('link', { name: 'Webui section' }),
-    ).not.toBeVisible()
-    await page.getByTestId('tool-picker').getByRole('link', { name: 'Web browser' }).click()
+    ).toBeVisible()
     await expect(
       page.getByTestId('minitoc').getByRole('link', { name: 'Desktop section' }),
     ).not.toBeVisible()
+    await page.getByTestId('tool-picker').getByRole('link', { name: 'Desktop' }).click()
     await expect(
       page.getByTestId('minitoc').getByRole('link', { name: 'Webui section' }),
+    ).not.toBeVisible()
+    await expect(
+      page.getByTestId('minitoc').getByRole('link', { name: 'Desktop section' }),
     ).toBeVisible()
   })
 })