From 3b825a46d2e237e697dd3a5548a9a70e0eb721d6 Mon Sep 17 00:00:00 2001 From: Hector Alfaro Date: Mon, 23 May 2022 10:48:49 -0400 Subject: [PATCH 1/5] pull out tools from page.js and ToolPicker component --- components/article/ToolPicker.tsx | 34 ++++----------------------- components/context/ArticleContext.tsx | 2 ++ lib/all-tools.js | 12 ++++++++++ lib/page.js | 19 ++++++--------- 4 files changed, 26 insertions(+), 41 deletions(-) create mode 100644 lib/all-tools.js diff --git a/components/article/ToolPicker.tsx b/components/article/ToolPicker.tsx index 4392bdac8799..4d6dcc409dbd 100644 --- a/components/article/ToolPicker.tsx +++ b/components/article/ToolPicker.tsx @@ -12,35 +12,11 @@ import { useArticleContext } from 'components/context/ArticleContext' // Nota bene: tool === application // Nota bene: picker === switcher -const supportedTools = [ - 'cli', - 'desktop', - 'webui', - 'curl', - 'codespaces', - 'vscode', - 'importer_cli', - 'graphql', - 'powershell', - 'bash', -] -const toolTitles = { - webui: 'Web browser', - cli: 'GitHub CLI', - curl: 'cURL', - desktop: 'Desktop', - codespaces: 'Codespaces', - vscode: 'Visual Studio Code', - importer_cli: 'GitHub Enterprise Importer CLI', - graphql: 'GraphQL API', - powershell: 'PowerShell', - bash: 'Bash', -} as Record - // Imperatively modify article content to show only the selected tool // find all platform-specific *block* elements and hide or show as appropriate // example: {% webui %} block content {% endwebui %} -function showToolSpecificContent(tool: string) { +function showToolSpecificContent(tool: string, supportedTools: Array) { + // const supportedTools = Object.keys(allTools) const markdowns = Array.from(document.querySelectorAll('.extended-markdown')) markdowns .filter((el) => supportedTools.some((tool) => el.classList.contains(tool))) @@ -77,7 +53,7 @@ type Props = { } export const ToolPicker = ({ variant = 'subnav' }: Props) => { const { asPath } = useRouter() - const { defaultTool, detectedTools } = useArticleContext() + const { defaultTool, detectedTools, allTools } = useArticleContext() const [currentTool, setCurrentTool] = useState(getDefaultTool(defaultTool, detectedTools)) const sharedContainerProps = { @@ -100,7 +76,7 @@ export const ToolPicker = ({ variant = 'subnav' }: Props) => { // Whenever the currentTool is changed, update the article content useEffect(() => { preserveAnchorNodePosition(document, () => { - showToolSpecificContent(currentTool) + showToolSpecificContent(currentTool, Object.keys(allTools)) }) }, [currentTool, asPath]) @@ -137,7 +113,7 @@ export const ToolPicker = ({ variant = 'subnav' }: Props) => { onClickTool(tool) }} > - {toolTitles[tool]} + {allTools[tool]} ))} diff --git a/components/context/ArticleContext.tsx b/components/context/ArticleContext.tsx index 966d99c665d9..70643f23d33a 100644 --- a/components/context/ArticleContext.tsx +++ b/components/context/ArticleContext.tsx @@ -29,6 +29,7 @@ export type ArticleContextT = { currentLearningTrack?: LearningTrack detectedPlatforms: Array detectedTools: Array + allTools: Record } export const ArticleContext = createContext(null) @@ -70,5 +71,6 @@ export const getArticleContextFromRequest = (req: any): ArticleContextT => { currentLearningTrack: req.context.currentLearningTrack, detectedPlatforms: page.detectedPlatforms || [], detectedTools: page.detectedTools || [], + allTools: page.allToolsParsed || [], } } diff --git a/lib/all-tools.js b/lib/all-tools.js new file mode 100644 index 000000000000..19a55f211a15 --- /dev/null +++ b/lib/all-tools.js @@ -0,0 +1,12 @@ +export const allTools = { + bash: 'Bash', + cli: 'GitHub CLI', + codespaces: 'Codespaces', + curl: 'cURL', + desktop: 'Dekstop', + importer_cli: 'GitHub Enterprise Importer CLI', + graphql: 'GraphQL API', + powershell: 'PowerShell', + vscode: 'Visual Studio Code', + webui: 'Web browser', +} diff --git a/lib/page.js b/lib/page.js index eacb1ae9683a..4d7ad3cb9cb2 100644 --- a/lib/page.js +++ b/lib/page.js @@ -17,6 +17,7 @@ import readFileContents from './read-file-contents.js' import getLinkData from './get-link-data.js' import getDocumentType from './get-document-type.js' import { union } from 'lodash-es' +import { allTools } from './all-tools.js' // We're going to check a lot of pages' "ID" (the first part of // the relativePath) against `productMap` to make sure it's valid. @@ -280,18 +281,12 @@ class Page { this.includesPlatformSpecificContent = this.detectedPlatforms.length > 0 // set flags for webui, cli, etc switcher element - this.detectedTools = [ - 'cli', - 'desktop', - 'webui', - 'curl', - 'codespaces', - 'vscode', - `importer_cli`, - `graphql`, - 'powershell', - 'bash', - ].filter((tool) => html.includes(`extended-markdown ${tool}`) || html.includes(`tool-${tool}`)) + this.detectedTools = Object.keys(allTools).filter( + (tool) => html.includes(`extended-markdown ${tool}`) || html.includes(`tool-${tool}`) + ) + + this.allToolsParsed = allTools + this.includesToolSpecificContent = this.detectedTools.length > 0 return html From 76a215abcc3775883e9bc8c368f02eb03dded241 Mon Sep 17 00:00:00 2001 From: Hector Alfaro Date: Mon, 23 May 2022 11:52:50 -0400 Subject: [PATCH 2/5] abstract tools from frontmatter and event stuff --- lib/frontmatter.js | 15 +++----------- lib/liquid-tags/extended-markdown.js | 20 ++++++++---------- lib/schema-event.js | 31 +++++----------------------- 3 files changed, 17 insertions(+), 49 deletions(-) diff --git a/lib/frontmatter.js b/lib/frontmatter.js index 97bded646780..54a7b20b5c06 100644 --- a/lib/frontmatter.js +++ b/lib/frontmatter.js @@ -3,6 +3,7 @@ import path from 'path' import parse from './read-frontmatter.js' import semver from 'semver' import { allVersions } from './all-versions.js' +import { allTools } from './all-tools.js' const layoutNames = [ 'default', @@ -20,6 +21,7 @@ const semverRange = { message: 'Must be a valid SemVer range', } const versionObjs = Object.values(allVersions) + const guideTypes = ['overview', 'quick_start', 'tutorial', 'how_to', 'reference'] const featureVersions = fs .readdirSync(path.posix.join(process.cwd(), 'data/features')) @@ -180,18 +182,7 @@ export const schema = { // Tool-specific content preference defaultTool: { type: 'string', - enum: [ - 'webui', - 'cli', - 'desktop', - 'curl', - 'codespaces', - 'vscode', - 'importer_cli', - 'graphql', - 'powershell', - 'bash', - ], + enum: Object.keys(allTools), }, // Documentation contributed by a third party, such as a GitHub Partner contributor: { diff --git a/lib/liquid-tags/extended-markdown.js b/lib/liquid-tags/extended-markdown.js index 20eec546592a..df3f8cccba0f 100644 --- a/lib/liquid-tags/extended-markdown.js +++ b/lib/liquid-tags/extended-markdown.js @@ -1,17 +1,13 @@ -export const tags = { +import { allTools } from '../all-tools.js' + +const toolTags = Object.keys(allTools).reduce((accumulator, key) => { + return { ...accumulator, [key]: '' } +}, {}) + +const subsetTags = { mac: '', windows: '', linux: '', - cli: '', - desktop: '', - webui: '', - curl: '', - codespaces: '', - vscode: '', - importer_cli: '', - graphql: '', - powershell: '', - bash: '', all: '', tip: 'border rounded-1 mb-4 p-3 color-border-accent-emphasis color-bg-accent f5', note: 'border rounded-1 mb-4 p-3 color-border-accent-emphasis color-bg-accent f5', @@ -19,6 +15,8 @@ export const tags = { danger: 'border rounded-1 mb-4 p-3 color-border-danger color-bg-danger f5', } +export const tags = Object.assign({}, subsetTags, toolTags) + export const template = '
{{ output }}
' diff --git a/lib/schema-event.js b/lib/schema-event.js index ec9564573eba..02345185ee81 100644 --- a/lib/schema-event.js +++ b/lib/schema-event.js @@ -1,7 +1,7 @@ import { languageKeys } from './languages.js' import { allVersionKeys } from './all-versions.js' import { productIds } from './all-products.js' - +import { allTools } from './all-tools.js' const context = { type: 'object', additionalProperties: false, @@ -146,18 +146,7 @@ const context = { }, application_preference: { type: 'string', - enum: [ - 'webui', - 'cli', - 'desktop', - 'curl', - 'codespaces', - 'vscode', - 'importer_cli', - 'graphql', - 'powershell', - 'bash', - ], + enum: Object.keys(allTools), description: 'The application selected by the user.', }, color_mode_preference: { @@ -451,17 +440,7 @@ const preferenceSchema = { }, preference_value: { type: 'string', - enum: [ - 'webui', - 'cli', - 'desktop', - 'curl', - 'codespaces', - 'vscode', - 'importer_cli', - 'graphql', - 'powershell', - 'bash', + enum: Object.keys(allTools).concat( 'dark', 'light', 'auto', @@ -469,8 +448,8 @@ const preferenceSchema = { 'auto:light', 'linux', 'mac', - 'windows', - ], + 'windows' + ), description: 'The application, color_mode, or os selected by the user.', }, }, From 91778790d8ac915d2622e1f61ed8704ddd733ec2 Mon Sep 17 00:00:00 2001 From: Hector Alfaro Date: Tue, 24 May 2022 10:40:46 -0400 Subject: [PATCH 3/5] clean up and comment tool picker stuff --- components/article/ToolPicker.tsx | 2 +- components/context/ArticleContext.tsx | 2 +- lib/all-tools.js | 1 + lib/frontmatter.js | 3 ++- lib/liquid-tags/extended-markdown.js | 5 +++-- lib/page.js | 1 + 6 files changed, 9 insertions(+), 5 deletions(-) diff --git a/components/article/ToolPicker.tsx b/components/article/ToolPicker.tsx index 4d6dcc409dbd..ad64fe29aae9 100644 --- a/components/article/ToolPicker.tsx +++ b/components/article/ToolPicker.tsx @@ -16,7 +16,6 @@ import { useArticleContext } from 'components/context/ArticleContext' // find all platform-specific *block* elements and hide or show as appropriate // example: {% webui %} block content {% endwebui %} function showToolSpecificContent(tool: string, supportedTools: Array) { - // const supportedTools = Object.keys(allTools) const markdowns = Array.from(document.querySelectorAll('.extended-markdown')) markdowns .filter((el) => supportedTools.some((tool) => el.classList.contains(tool))) @@ -53,6 +52,7 @@ type Props = { } export const ToolPicker = ({ variant = 'subnav' }: Props) => { const { asPath } = useRouter() + // allTools comes from the ArticleContext which contains the list of tools available const { defaultTool, detectedTools, allTools } = useArticleContext() const [currentTool, setCurrentTool] = useState(getDefaultTool(defaultTool, detectedTools)) diff --git a/components/context/ArticleContext.tsx b/components/context/ArticleContext.tsx index 70643f23d33a..bd2d198ebc69 100644 --- a/components/context/ArticleContext.tsx +++ b/components/context/ArticleContext.tsx @@ -71,6 +71,6 @@ export const getArticleContextFromRequest = (req: any): ArticleContextT => { currentLearningTrack: req.context.currentLearningTrack, detectedPlatforms: page.detectedPlatforms || [], detectedTools: page.detectedTools || [], - allTools: page.allToolsParsed || [], + allTools: page.allToolsParsed || [], // this is set at the page level, see lib/page.js } } diff --git a/lib/all-tools.js b/lib/all-tools.js index 19a55f211a15..9cb24ff39f38 100644 --- a/lib/all-tools.js +++ b/lib/all-tools.js @@ -1,3 +1,4 @@ +// all the tools available for the Tool Picker export const allTools = { bash: 'Bash', cli: 'GitHub CLI', diff --git a/lib/frontmatter.js b/lib/frontmatter.js index 54a7b20b5c06..fdf7d3d8a9bf 100644 --- a/lib/frontmatter.js +++ b/lib/frontmatter.js @@ -179,7 +179,8 @@ export const schema = { type: 'string', enum: ['mac', 'windows', 'linux'], }, - // Tool-specific content preference + // Tool-specific content preference, the list of tools are kept in + // make it easier to update in a single place defaultTool: { type: 'string', enum: Object.keys(allTools), diff --git a/lib/liquid-tags/extended-markdown.js b/lib/liquid-tags/extended-markdown.js index df3f8cccba0f..1b87e7a06fc3 100644 --- a/lib/liquid-tags/extended-markdown.js +++ b/lib/liquid-tags/extended-markdown.js @@ -1,7 +1,8 @@ import { allTools } from '../all-tools.js' -const toolTags = Object.keys(allTools).reduce((accumulator, key) => { - return { ...accumulator, [key]: '' } +// we do this to get an object that combines all possible liquid tags +const toolTags = Object.keys(allTools).reduce((accumulator, tool) => { + return { ...accumulator, [tool]: '' } }, {}) const subsetTags = { diff --git a/lib/page.js b/lib/page.js index 4d7ad3cb9cb2..bbbcc93f3d11 100644 --- a/lib/page.js +++ b/lib/page.js @@ -285,6 +285,7 @@ class Page { (tool) => html.includes(`extended-markdown ${tool}`) || html.includes(`tool-${tool}`) ) + // pass the list of all possible tools around to components and utilities that will need it later on this.allToolsParsed = allTools this.includesToolSpecificContent = this.detectedTools.length > 0 From 13d309e1e9f31f757cfb5ef11921d9336645f427 Mon Sep 17 00:00:00 2001 From: Hector Alfaro Date: Tue, 24 May 2022 10:48:42 -0400 Subject: [PATCH 4/5] add documentation on modifying the tool list --- contributing/content-markup-reference.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contributing/content-markup-reference.md b/contributing/content-markup-reference.md index 7d8fe5dd2e0f..632507dc4fd7 100644 --- a/contributing/content-markup-reference.md +++ b/contributing/content-markup-reference.md @@ -128,7 +128,7 @@ You can define a default platform in the frontmatter. For more information, see ## Tool tags -We occasionally need to write documentation for different tools (GitHub UI, GitHub CLI, GitHub Desktop, cURL, Codespaces, VS Code, GitHub Enterprise Importer CLI, GraphQL API). Each tool may require a different set of instructions. We use tool tags to demarcate information for each tool. +We occasionally need to write documentation for different tools (GitHub UI, GitHub CLI, GitHub Desktop, cURL, Codespaces, VS Code, GitHub Enterprise Importer CLI, GraphQL API). Each tool may require a different set of instructions. We use tool tags to demarcate information for each tool. To modify the list of possible tools, edit [`lib/all-tools.js`](../lib/all-tools.js). ### Usage From 1b3f03579b067ae936134f5012c12156ba4dfd0a Mon Sep 17 00:00:00 2001 From: Hector Alfaro Date: Wed, 25 May 2022 10:03:47 -0400 Subject: [PATCH 5/5] use @heiskr's suggestion for the tool list --- lib/liquid-tags/extended-markdown.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/liquid-tags/extended-markdown.js b/lib/liquid-tags/extended-markdown.js index 1b87e7a06fc3..f44ebeb67d60 100644 --- a/lib/liquid-tags/extended-markdown.js +++ b/lib/liquid-tags/extended-markdown.js @@ -1,9 +1,7 @@ import { allTools } from '../all-tools.js' // we do this to get an object that combines all possible liquid tags -const toolTags = Object.keys(allTools).reduce((accumulator, tool) => { - return { ...accumulator, [tool]: '' } -}, {}) +const toolTags = Object.fromEntries(Object.keys(allTools).map((tool) => [tool, ''])) const subsetTags = { mac: '',