From 58ff4b44b989c4b1e9b30c72ecc88278e0ecf041 Mon Sep 17 00:00:00 2001 From: Kevin Heis Date: Mon, 17 Jul 2023 11:38:45 -0700 Subject: [PATCH] A11y: remove horizontal scroll (#39170) --- .../ui/MarkdownContent/stylesheets/code.scss | 6 +- data/learning-tracks/README.md | 5 -- data/learning-tracks/actions.yml | 1 - data/learning-tracks/admin.yml | 2 - data/learning-tracks/code-security.yml | 2 - lib/page.js | 6 +- .../lib/learning-tracks-schema.js | 3 - src/content-linter/tests/lint-files.js | 46 +---------- .../components/GuidesHero.module.scss | 15 ---- src/landings/components/GuidesHero.tsx | 81 +------------------ .../components/ProductGuidesContext.tsx | 13 +-- .../components/guides/LearningTrack.tsx | 4 +- .../lib/process-learning-tracks.js | 24 +----- .../components/RestCodeSamples.module.scss | 4 +- .../WebhookPayloadExample.module.scss | 4 +- stylesheets/annotate.scss | 9 --- stylesheets/syntax-highlighting.scss | 1 - .../data/learning-tracks/code-security.yml | 1 - tests/rendering-fixtures/guides.js | 2 +- tests/unit/page.js | 8 -- 20 files changed, 22 insertions(+), 215 deletions(-) delete mode 100644 src/landings/components/GuidesHero.module.scss diff --git a/components/ui/MarkdownContent/stylesheets/code.scss b/components/ui/MarkdownContent/stylesheets/code.scss index ff3bd837fd42..a6fe60f10464 100644 --- a/components/ui/MarkdownContent/stylesheets/code.scss +++ b/components/ui/MarkdownContent/stylesheets/code.scss @@ -5,9 +5,13 @@ border: 1px var(--color-border-default) solid; } + pre code { + white-space: pre-wrap; + } + [class~="height-constrained-code-block"] pre { max-height: 32rem; - overflow: auto; + overflow-y: auto; } [class~="code-example"] { diff --git a/data/learning-tracks/README.md b/data/learning-tracks/README.md index 69d08c435a05..7bb8339b55a1 100644 --- a/data/learning-tracks/README.md +++ b/data/learning-tracks/README.md @@ -23,10 +23,6 @@ Learning track data for a product is defined in two places: For example, in `data/learning-tracks/actions.yml`, each of the items from the content file's `learningTracks` array is represented with additional data such as `title`, `description`, and an array of `guides` links. - One learning track in this YAML **per version** must be designated as a "featured" learning track via `featured_track: true`, which will set it to appear at the top of the product guides page. A test will fail if this property is missing. - - The `featured_track` property can be a simple boolean (i.e., `featured_track: true`) or it can be a string that includes versioning statements (e.g., `featured_track: '{% ifversion fpt %}true{% else %}false{% endif %}'`). If you use versioning, you'll have multiple `featured_track`s per YML file, but make sure that only one will render in each currently supported version. A test will fail if there are more or less than one featured link for each version. - ## Versioning Versioning for learning tracks is processed at page render time. The code lives in [`lib/learning-tracks.js`](lib/learning-tracks.js), which is called by `page.render()`. The processed learning tracks are then rendered by `components/guides`. @@ -41,7 +37,6 @@ For example: learning_track_name: title: 'Learning track title' description: 'Learning track description' - featured_track: true versions: ghes: '>=3.0' guides: diff --git a/data/learning-tracks/actions.yml b/data/learning-tracks/actions.yml index a8ce4be264e7..3d393acb530f 100644 --- a/data/learning-tracks/actions.yml +++ b/data/learning-tracks/actions.yml @@ -10,7 +10,6 @@ getting_started: - /actions/using-workflows/about-workflows - /actions/using-workflows/reusing-workflows - /actions/security-guides/security-hardening-for-github-actions - featured_track: true adopting_github_actions_for_your_enterprise_ghec: title: Adopt GitHub Actions for your enterprise description: >- diff --git a/data/learning-tracks/admin.yml b/data/learning-tracks/admin.yml index 6b164e53434a..1551b86ac5d2 100644 --- a/data/learning-tracks/admin.yml +++ b/data/learning-tracks/admin.yml @@ -3,7 +3,6 @@ get_started_with_github_ae: description: >- Learn about {% data variables.product.prodname_ghe_managed %} and complete the initial configuration of a new enterprise. - featured_track: true versions: ghae: '*' guides: @@ -20,7 +19,6 @@ deploy_an_instance: description: >- Install {% data variables.product.prodname_ghe_server %} on your platform of choice and configure SAML authentication. - featured_track: true versions: ghes: '*' guides: diff --git a/data/learning-tracks/code-security.yml b/data/learning-tracks/code-security.yml index f0e5eb892f21..ec1d77993cab 100644 --- a/data/learning-tracks/code-security.yml +++ b/data/learning-tracks/code-security.yml @@ -3,7 +3,6 @@ security_advisories: description: >- Using repository security advisories to privately fix a reported vulnerability and get a CVE. - featured_track: '{% ifversion fpt or ghec %}true{% else %}false{% endif %}' guides: - >- /code-security/security-advisories/guidance-on-reporting-and-writing/about-coordinated-disclosure-of-security-vulnerabilities @@ -174,7 +173,6 @@ code_security_actions: description: >- Check your default branch and every pull request to keep vulnerabilities and errors out of your repository. - featured_track: '{% ifversion ghae or ghes %}true{% else %}false{% endif %}' guides: - >- /code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/about-code-scanning diff --git a/lib/page.js b/lib/page.js index 174b7ddfc03b..9036ba881c27 100644 --- a/lib/page.js +++ b/lib/page.js @@ -208,11 +208,7 @@ class Page { // Learning tracks may contain Liquid and need to have versioning processed. if (this.rawLearningTracks) { - const { featuredTrack, learningTracks } = await processLearningTracks( - this.rawLearningTracks, - context, - ) - this.featuredTrack = featuredTrack + const { learningTracks } = await processLearningTracks(this.rawLearningTracks, context) this.learningTracks = learningTracks } diff --git a/src/content-linter/lib/learning-tracks-schema.js b/src/content-linter/lib/learning-tracks-schema.js index 4025e18ea739..8543a5caf8dd 100644 --- a/src/content-linter/lib/learning-tracks-schema.js +++ b/src/content-linter/lib/learning-tracks-schema.js @@ -26,9 +26,6 @@ export default { type: 'array', items: { type: 'string' }, }, - featured_track: { - type: ['boolean', 'string'], - }, versions: versionsProps, }, }, diff --git a/src/content-linter/tests/lint-files.js b/src/content-linter/tests/lint-files.js index 3abec84fec8f..915f339ba542 100755 --- a/src/content-linter/tests/lint-files.js +++ b/src/content-linter/tests/lint-files.js @@ -18,18 +18,13 @@ import { frontmatter, deprecatedProperties } from '../../../lib/frontmatter.js' import languages from '../../../lib/languages.js' import releaseNotesSchema from '../lib/release-notes-schema.js' import learningTracksSchema from '../lib/learning-tracks-schema.js' -import { renderContent, liquid } from '#src/content-render/index.js' -import getApplicableVersions from '../../../lib/get-applicable-versions.js' -import { allVersions } from '../../../lib/all-versions.js' +import { liquid } from '#src/content-render/index.js' import { getDiffFiles } from '../lib/diff-files.js' import { formatAjvErrors } from '../../../tests/helpers/schemas.js' jest.useFakeTimers({ legacyFakeTimers: true }) const __dirname = path.dirname(fileURLToPath(import.meta.url)) -const enterpriseServerVersions = Object.keys(allVersions).filter((v) => - v.startsWith('enterprise-server@'), -) const rootDir = path.join(__dirname, '../../..') const contentDir = path.join(rootDir, 'content') @@ -1023,45 +1018,6 @@ describe('lint learning tracks', () => { expect(valid, errors).toBe(true) }) - it('has one and only one featured track per supported version', async () => { - // Use the YAML filename to determine which product this refers to, and then peek - // inside the product TOC frontmatter to see which versions the product is available in. - const product = path.posix.basename(yamlRelPath, '.yml') - const productTocPath = path.posix.join('content', product, 'index.md') - const productContents = await fs.readFile(productTocPath, 'utf8') - const { data } = frontmatter(productContents) - const productVersions = getApplicableVersions(data.versions, productTocPath) - - const featuredTracks = {} - const context = { enterpriseServerVersions } - - // For each of the product's versions, render the learning track data and look for a featured track. - await Promise.all( - productVersions.map(async (version) => { - const featuredTracksPerVersion = [] - - for (const entry of Object.values(dictionary)) { - if (!entry.featured_track) return - context.currentVersion = version - context[allVersions[version].shortName] = true - const isFeaturedLink = - typeof entry.featured_track === 'boolean' || - (await renderContent(entry.featured_track, context, { - textOnly: true, - })) === 'true' - featuredTracksPerVersion.push(isFeaturedLink) - } - - featuredTracks[version] = featuredTracksPerVersion.length - }), - ) - - Object.entries(featuredTracks).forEach(([version, numOfFeaturedTracks]) => { - const errorMessage = `Expected 1 featured learning track but found ${numOfFeaturedTracks} for ${version} in ${yamlAbsPath}` - expect(numOfFeaturedTracks, errorMessage).toBe(1) - }) - }) - it('contains valid liquid', () => { const toLint = [] Object.values(dictionary).forEach(({ title, description }) => { diff --git a/src/landings/components/GuidesHero.module.scss b/src/landings/components/GuidesHero.module.scss deleted file mode 100644 index 63c86caf312f..000000000000 --- a/src/landings/components/GuidesHero.module.scss +++ /dev/null @@ -1,15 +0,0 @@ -.fadeLeft { - background: linear-gradient( - to right, - var(--color-canvas-default), - transparent - ); -} - -.fadeRight { - background: linear-gradient( - to left, - var(--color-canvas-default), - transparent - ); -} diff --git a/src/landings/components/GuidesHero.tsx b/src/landings/components/GuidesHero.tsx index 14c657e82b44..bf0536534137 100644 --- a/src/landings/components/GuidesHero.tsx +++ b/src/landings/components/GuidesHero.tsx @@ -1,92 +1,17 @@ -import cx from 'classnames' import { useProductGuidesContext } from 'src/landings/components/ProductGuidesContext' -import { ArrowRightIcon, StarFillIcon } from '@primer/octicons-react' -import { useTranslation } from 'components/hooks/useTranslation' -import { Link } from 'components/Link' -import { TruncateLines } from 'components/ui/TruncateLines' import { Lead } from 'components/ui/Lead' -import styles from './GuidesHero.module.scss' -export const GuidesHero = () => { - const { title, intro, featuredTrack } = useProductGuidesContext() - const { t } = useTranslation('product_guides') - const cardWidth = 280 - const firstCardWidth = cardWidth + 10 // so the english text doesn't wrap - - const guideItems = featuredTrack?.guides?.map((guide) => ( -
  • - -
    -
    - {featuredTrack.guides && ( - - {featuredTrack.guides?.indexOf(guide) + 1} - - )} -
    -
    - {t('guide_types')[guide.page?.type || '']} -
    -
    -

    {guide.title}

    - - - - -
    • - )) +export function GuidesHero() { + const { title, intro } = useProductGuidesContext() return (
    -
    +

    {title}

    {intro && {intro}}
    - {featuredTrack && featuredTrack.guides && featuredTrack.guides.length > 0 && ( -
    -
      -
    • -
      -
      - -
      -

      {featuredTrack.title}

      -
      {featuredTrack.description}
      - - {t(`start_path`)} - - -
      -
      • - {guideItems} -
    -
    -
    -
    - )}
    ) } diff --git a/src/landings/components/ProductGuidesContext.tsx b/src/landings/components/ProductGuidesContext.tsx index 54929984192a..aa8c274250f5 100644 --- a/src/landings/components/ProductGuidesContext.tsx +++ b/src/landings/components/ProductGuidesContext.tsx @@ -1,7 +1,7 @@ import { createContext, useContext } from 'react' import pick from 'lodash/pick' -export type FeaturedTrack = { +export type LearningTrack = { trackName: string trackProduct: string title: string @@ -20,8 +20,7 @@ export type ArticleGuide = { export type ProductGuidesContextT = { title: string intro: string - featuredTrack?: FeaturedTrack - learningTracks?: Array + learningTracks?: Array includeGuides?: Array allTopics?: Array } @@ -45,14 +44,6 @@ export const getProductGuidesContextFromRequest = (req: any): ProductGuidesConte return { ...pick(page, ['title', 'intro', 'allTopics']), - featuredTrack: page.featuredTrack - ? { - ...pick(page.featuredTrack, ['title', 'description', 'trackName', 'trackProduct']), - guides: (page.featuredTrack?.guides || []).map((guide: any) => { - return pick(guide, ['title', 'intro', 'href', 'page.type']) - }), - } - : null, learningTracks: (page.learningTracks || []).map((track: any) => ({ ...pick(track, ['title', 'description', 'trackName', 'trackProduct']), guides: (track.guides || []).map((guide: any) => { diff --git a/src/learning-track/components/guides/LearningTrack.tsx b/src/learning-track/components/guides/LearningTrack.tsx index a71efad55eb2..b86e7d5bcb55 100644 --- a/src/learning-track/components/guides/LearningTrack.tsx +++ b/src/learning-track/components/guides/LearningTrack.tsx @@ -3,14 +3,14 @@ import { useTranslation } from 'components/hooks/useTranslation' import { ArrowRightIcon } from '@primer/octicons-react' import { ActionList } from '@primer/react' import { useState } from 'react' -import { FeaturedTrack } from 'src/landings/components/ProductGuidesContext' +import { LearningTrack as LearningTrackT } from 'src/landings/components/ProductGuidesContext' import { TruncateLines } from 'components/ui/TruncateLines' import { slug } from 'github-slugger' import styles from './LearningTrack.module.scss' import { Link } from 'components/Link' type Props = { - track: FeaturedTrack + track: LearningTrackT } const DEFAULT_VISIBLE_GUIDES = 4 diff --git a/src/learning-track/lib/process-learning-tracks.js b/src/learning-track/lib/process-learning-tracks.js index a24c2449854f..b956c8ca89f1 100644 --- a/src/learning-track/lib/process-learning-tracks.js +++ b/src/learning-track/lib/process-learning-tracks.js @@ -11,15 +11,11 @@ const renderOpts = { textOnly: true } export default async function processLearningTracks(rawLearningTracks, context) { const learningTracks = [] - let featuredTrack - if (!context.currentProduct) { throw new Error(`Missing context.currentProduct value.`) } for (const rawTrackName of rawLearningTracks) { - let isFeaturedTrack = false - // Track names in frontmatter may include Liquid conditionals. const renderedTrackName = await renderContent(rawTrackName, context, renderOpts) if (!renderedTrackName) continue @@ -48,7 +44,7 @@ export default async function processLearningTracks(rawLearningTracks, context) // We do this for two reasons: // // 1. For each learning-track .yml file (in data) always want the - // English values for `guides`, `versions`, `featured_track`. + // English values for `guides`, `versions`. // Meaning, for the translated learning tracks we only keep the // `title` and `description`. // @@ -69,7 +65,6 @@ export default async function processLearningTracks(rawLearningTracks, context) // from the English equivalent. track.guides = enTrack.guides track.versions = enTrack.versions - track.featured_track = enTrack.featured_track } // If there is no `versions` prop in the learning track frontmatter, assume the track should display in all versions. @@ -103,24 +98,11 @@ export default async function processLearningTracks(rawLearningTracks, context) guides: await getLinkData(track.guides, context), } - // Determine if this is the featured track. - if (track.featured_track) { - // Featured track properties may be booleans or string that include Liquid conditionals with versioning. - // We need to parse any strings to determine if the featured track is relevant for this version. - isFeaturedTrack = - track.featured_track === true || - (await renderContent(track.featured_track, context, renderOpts)) === 'true' - - if (isFeaturedTrack) { - featuredTrack = learningTrack - } - } - // Only add the track to the array of tracks if there are guides in this version and it's not the featured track. - if (learningTrack.guides.length && !isFeaturedTrack) { + if (learningTrack.guides.length) { learningTracks.push(learningTrack) } } - return { featuredTrack, learningTracks } + return { learningTracks } } diff --git a/src/rest/components/RestCodeSamples.module.scss b/src/rest/components/RestCodeSamples.module.scss index 342dac6b7940..4910c0bc0403 100644 --- a/src/rest/components/RestCodeSamples.module.scss +++ b/src/rest/components/RestCodeSamples.module.scss @@ -1,7 +1,7 @@ @import "@primer/css/support/index.scss"; .codeBlock { - overflow: auto; + overflow-y: auto; margin-bottom: 1rem; line-height: 1.45; background-color: var(--color-canvas-subtle); @@ -10,8 +10,8 @@ code { background-color: transparent; padding: 8px 8px 16px; - white-space: pre; max-width: 90vw; + white-space: pre-wrap; @include breakpoint(lg) { max-width: 40vw; diff --git a/src/webhooks/components/WebhookPayloadExample.module.scss b/src/webhooks/components/WebhookPayloadExample.module.scss index 05121c8aa522..4984f4e7533a 100644 --- a/src/webhooks/components/WebhookPayloadExample.module.scss +++ b/src/webhooks/components/WebhookPayloadExample.module.scss @@ -1,7 +1,7 @@ @import "@primer/css/support/index.scss"; .payloadExample { - overflow: auto; + overflow-y: auto; margin-bottom: 1rem; line-height: 1.45; background-color: var(--color-canvas-subtle); @@ -11,6 +11,6 @@ code { background-color: transparent; padding: 8px 8px 16px; - white-space: pre; + white-space: pre-wrap; } } diff --git a/stylesheets/annotate.scss b/stylesheets/annotate.scss index aa96c33adb11..5059bd1fbdc2 100644 --- a/stylesheets/annotate.scss +++ b/stylesheets/annotate.scss @@ -23,15 +23,6 @@ pre { border: 0px !important; margin: 0px !important; - - .hljs { - overflow-x: auto; - white-space: pre-wrap; - white-space: -moz-pre-wrap; - white-space: -pre-wrap; - white-space: -o-pre-wrap; - word-wrap: break-word; - } } } diff --git a/stylesheets/syntax-highlighting.scss b/stylesheets/syntax-highlighting.scss index af482d83dc91..791d64163f02 100644 --- a/stylesheets/syntax-highlighting.scss +++ b/stylesheets/syntax-highlighting.scss @@ -7,7 +7,6 @@ from https://unpkg.com/highlight.js@9.15.8/styles/github.css .hljs { display: block; - overflow-x: auto; padding: 0.5em; color: var(--color-fg-default); background: var(--color-canvas-subtle); diff --git a/tests/fixtures/data/learning-tracks/code-security.yml b/tests/fixtures/data/learning-tracks/code-security.yml index 2dfa2c831886..6fc60784924c 100644 --- a/tests/fixtures/data/learning-tracks/code-security.yml +++ b/tests/fixtures/data/learning-tracks/code-security.yml @@ -1,7 +1,6 @@ foo_bar: title: Fix the plumbing description: A couple easy ones - featured_track: '{% ifversion fpt or ghec %}true{% else %}false{% endif %}' guides: - /code-security/getting-started/quickstart - /code-security/getting-started/securing-your-organization diff --git a/tests/rendering-fixtures/guides.js b/tests/rendering-fixtures/guides.js index 4ddf18c88ffb..0610607adc7f 100644 --- a/tests/rendering-fixtures/guides.js +++ b/tests/rendering-fixtures/guides.js @@ -17,7 +17,7 @@ describe('guides', () => { describe('learning tracks', () => { test('start the first learning track and come back via the navigation banner', async () => { const $ = await getDOM('/code-security/guides') - const links = $('[data-testid=feature-track] a') + const links = $('[data-testid=learning-track] a') const link = links.filter((_, el) => $(el).text() === 'Start learning path').first() expect(link.attr('href')).toMatch('learn=foo_bar') expect(link.attr('href')).toMatch('learnProduct=code-security') diff --git a/tests/unit/page.js b/tests/unit/page.js index 6f1f343f8f45..27a7d167e57c 100644 --- a/tests/unit/page.js +++ b/tests/unit/page.js @@ -289,15 +289,11 @@ describe('Page class', () => { title: 'title', description: 'description', guides, - featured_track: - '{% if currentVersion == "free-pro-team@latest" %}true{% else %}false{% endif %}', }, track_2: { title: 'title', description: 'description', guides, - featured_track: - '{% if enterpriseServerVersions contains currentVersion %}true{% else %}false{% endif %}', }, dotcom_only_track: { title: 'title', @@ -325,8 +321,6 @@ describe('Page class', () => { const dotcomTrackNames = page.learningTracks.map((t) => t.trackName) expect(dotcomTrackNames.includes('track_2')).toBe(true) expect(dotcomTrackNames.includes('dotcom_only_track')).toBe(true) - expect(page.featuredTrack.trackName === 'track_1').toBeTruthy() - expect(page.featuredTrack.trackName === 'track_2').toBeFalsy() // Switch to Enterprise. context.currentVersion = `enterprise-server@${latest}` @@ -336,8 +330,6 @@ describe('Page class', () => { const ghesTrackNames = page.learningTracks.map((t) => t.trackName) expect(ghesTrackNames.includes('track_1')).toBe(true) expect(ghesTrackNames.includes('enterprise_only_track')).toBe(true) - expect(page.featuredTrack.trackName === 'track_1').toBeFalsy() - expect(page.featuredTrack.trackName === 'track_2').toBeTruthy() }) })