From 4cd427842aed34a210a36c2f69e982b47012dd8b Mon Sep 17 00:00:00 2001 From: Rachael Sewell Date: Tue, 20 Feb 2024 14:36:36 -0800 Subject: [PATCH] Permissions redesign (#48873) --- data/ui.yml | 2 +- .../components/AutomatedPage.tsx | 5 +- .../fixtures/content/get-started/foo/index.md | 1 + ...ge-with-permissions-and-product-callout.md | 17 +++++++ src/fixtures/fixtures/data/ui.yml | 2 +- src/fixtures/tests/alerts.js | 26 ---------- src/fixtures/tests/markdown.js | 11 ----- src/fixtures/tests/permissions-callout.js | 48 +++++++++++++++++++ .../components/article/ArticleGridLayout.tsx | 2 +- src/frame/components/article/ArticlePage.tsx | 5 +- .../PermissionsStatement.module.scss | 3 -- .../PermissionsStatement.tsx | 37 +++++++++----- src/landings/components/TocLanding.tsx | 4 +- src/rest/components/RestReferencePage.tsx | 4 +- 14 files changed, 100 insertions(+), 67 deletions(-) create mode 100644 src/fixtures/fixtures/content/get-started/foo/page-with-permissions-and-product-callout.md delete mode 100644 src/fixtures/tests/alerts.js create mode 100644 src/fixtures/tests/permissions-callout.js delete mode 100644 src/frame/components/ui/PermissionsStatement/PermissionsStatement.module.scss diff --git a/data/ui.yml b/data/ui.yml index fac8d11cd4..f37d9cbe0f 100644 --- a/data/ui.yml +++ b/data/ui.yml @@ -47,7 +47,7 @@ pages: miniToc: In this article all_enterprise_releases: All Enterprise Server releases about_versions: About versions - permissions_statement: Who can use this feature + permissions_callout_title: Who can use this feature? video_from_transcript: See video for this transcript support: still_need_help: Still need help? diff --git a/src/automated-pipelines/components/AutomatedPage.tsx b/src/automated-pipelines/components/AutomatedPage.tsx index 5f6bc2d4fd..3625ab5668 100644 --- a/src/automated-pipelines/components/AutomatedPage.tsx +++ b/src/automated-pipelines/components/AutomatedPage.tsx @@ -7,7 +7,6 @@ import { ArticleGridLayout } from 'src/frame/components/article/ArticleGridLayou import { MiniTocs } from 'src/frame/components/ui/MiniTocs' import { useAutomatedPageContext } from 'src/automated-pipelines/components/AutomatedPageContext' import { ClientSideHighlight } from 'src/frame/components/ClientSideHighlight' -import { Alert } from 'src/frame/components/ui/Alert' import { Breadcrumbs } from 'src/frame/components/page-header/Breadcrumbs' type Props = { @@ -40,9 +39,7 @@ export const AutomatedPage = ({ children }: Props) => { )} - {permissions && } - - {product && } + } toc={miniTocItems.length > 1 && } diff --git a/src/fixtures/fixtures/content/get-started/foo/index.md b/src/fixtures/fixtures/content/get-started/foo/index.md index ddf783e674..e49f677283 100644 --- a/src/fixtures/fixtures/content/get-started/foo/index.md +++ b/src/fixtures/fixtures/content/get-started/foo/index.md @@ -15,6 +15,7 @@ children: - /for-playwright - /html-short-title - /page-with-callout + - /page-with-permissions-and-product-callout - /table-with-ifversions - /code-snippet-with-hashbang --- diff --git a/src/fixtures/fixtures/content/get-started/foo/page-with-permissions-and-product-callout.md b/src/fixtures/fixtures/content/get-started/foo/page-with-permissions-and-product-callout.md new file mode 100644 index 0000000000..97bc2d9dcc --- /dev/null +++ b/src/fixtures/fixtures/content/get-started/foo/page-with-permissions-and-product-callout.md @@ -0,0 +1,17 @@ +--- +title: Page with permissions and product callout +shortTitle: Callout page +intro: A very short intro +permissions: This is a permission callout +product: '{% data reusables.gated-features.not-for-ghes %}' +versions: + fpt: '*' + ghes: '*' + ghec: '*' +--- + +## Heading + +Note that this page uses the `product` and `permissions` frontmatter property. So it should +result in a call out box rendered with two messages. But only if the version is *not* +Enterprise Server. diff --git a/src/fixtures/fixtures/data/ui.yml b/src/fixtures/fixtures/data/ui.yml index fac8d11cd4..f37d9cbe0f 100644 --- a/src/fixtures/fixtures/data/ui.yml +++ b/src/fixtures/fixtures/data/ui.yml @@ -47,7 +47,7 @@ pages: miniToc: In this article all_enterprise_releases: All Enterprise Server releases about_versions: About versions - permissions_statement: Who can use this feature + permissions_callout_title: Who can use this feature? video_from_transcript: See video for this transcript support: still_need_help: Still need help? diff --git a/src/fixtures/tests/alerts.js b/src/fixtures/tests/alerts.js deleted file mode 100644 index b0ce3b15c7..0000000000 --- a/src/fixtures/tests/alerts.js +++ /dev/null @@ -1,26 +0,0 @@ -import { getDOM } from '#src/tests/helpers/e2etest.js' - -describe('alerts', () => { - test('article page', async () => { - const $ = await getDOM('/get-started/foo/page-with-callout') - const callout = $('[data-testid=alert] div') - expect(callout.html()).toBe('

Callout for HubGit Pages

') - }) - - test('callout disappears depend on Liquid inside it', async () => { - // This page has `product:` property which is a piece of Liquid - // which makes it so that the rendered output of that becomes - // an empty string. - // This test tests that alert is not rendered if its output - // "exits" but is empty. - const $ = await getDOM('/enterprise-server@latest/get-started/foo/page-with-callout') - const callout = $('[data-testid=alert]') - expect(callout.length).toBe(0) - }) - - test('toc landing page', async () => { - const $ = await getDOM('/actions/category') - const callout = $('[data-testid=alert] div') - expect(callout.html()).toBe('

This is the callout text

') - }) -}) diff --git a/src/fixtures/tests/markdown.js b/src/fixtures/tests/markdown.js index 1f87671f80..4cc02dd830 100644 --- a/src/fixtures/tests/markdown.js +++ b/src/fixtures/tests/markdown.js @@ -8,17 +8,6 @@ describe('markdown rendering', () => { expect(html).toMatch('syntax') expect(html).toMatch('HubGit') }) - - test('page with permission frontmatter', async () => { - const $ = await getDOM('/get-started/markdown/permissions') - const html = $('[data-testid="permissions-statement"]').html() - // part of the UI - expect(html).toMatch('Who can use this feature') - // Markdown - expect(html).toMatch('admin') - // Liquid - expect(html).toMatch('HubGit Pages site') - }) }) describe('alerts', () => { diff --git a/src/fixtures/tests/permissions-callout.js b/src/fixtures/tests/permissions-callout.js new file mode 100644 index 0000000000..90affa11cc --- /dev/null +++ b/src/fixtures/tests/permissions-callout.js @@ -0,0 +1,48 @@ +import { getDOM } from '#src/tests/helpers/e2etest.js' + +describe('permission statements', () => { + test('article page product statement', async () => { + const $ = await getDOM('/get-started/foo/page-with-callout') + const callout = $('[data-testid=product-statement] div') + expect(callout.html()).toBe('

Callout for HubGit Pages

') + }) + + test('callout disappears depend on Liquid inside it', async () => { + // This page has `product:` property which is a piece of Liquid + // which makes it so that the rendered output of that becomes + // an empty string. + // This test tests that alert is not rendered if its output + // "exits" but is empty. + const $ = await getDOM('/enterprise-server@latest/get-started/foo/page-with-callout') + const callout = $('[data-testid=product-statement]') + expect(callout.length).toBe(0) + }) + + test('toc landing page', async () => { + const $ = await getDOM('/actions/category') + const callout = $('[data-testid=product-statement] div') + expect(callout.html()).toBe('

This is the callout text

') + }) + + test('page with permission frontmatter', async () => { + const $ = await getDOM('/get-started/markdown/permissions') + const html = $('[data-testid=permissions-statement] div').html() + // Markdown + expect(html).toMatch('admin') + // Liquid + expect(html).toMatch('HubGit Pages site') + }) + + test('page with permission frontmatter and product statement', async () => { + const $ = await getDOM('/get-started/foo/page-with-permissions-and-product-callout.md') + const html = $('[data-testid=permissions-callout] div').html() + // part of the UI + expect(html).toMatch('Who can use this feature') + + const permission = $('[data-testid=permissions-statement] div') + expect(permission.html()).toBe('

This is a permission callout

') + + const product = $('[data-testid=product-statement] div') + expect(product.html()).toBe('

Callout for HubGit Pages

') + }) +}) diff --git a/src/frame/components/article/ArticleGridLayout.tsx b/src/frame/components/article/ArticleGridLayout.tsx index 01a859a1b2..f9796ff8fc 100644 --- a/src/frame/components/article/ArticleGridLayout.tsx +++ b/src/frame/components/article/ArticleGridLayout.tsx @@ -25,7 +25,7 @@ export const ArticleGridLayout = ({ {topper && {topper}} {intro && ( - + {intro} )} diff --git a/src/frame/components/article/ArticlePage.tsx b/src/frame/components/article/ArticlePage.tsx index fd71cba8cf..425b39dab5 100644 --- a/src/frame/components/article/ArticlePage.tsx +++ b/src/frame/components/article/ArticlePage.tsx @@ -3,7 +3,6 @@ import dynamic from 'next/dynamic' import cx from 'classnames' import { LinkExternalIcon } from '@primer/octicons-react' -import { Alert } from 'src/frame/components/ui/Alert' import { DefaultLayout } from 'src/frame/components/DefaultLayout' import { ArticleTitle } from 'src/frame/components/article/ArticleTitle' import { useArticleContext } from 'src/frame/components/context/ArticleContext' @@ -62,12 +61,10 @@ export const ArticlePage = () => { const introCalloutsProp = ( <> - {permissions && } + {includesPlatformSpecificContent && } {includesToolSpecificContent && } - - {product && } ) diff --git a/src/frame/components/ui/PermissionsStatement/PermissionsStatement.module.scss b/src/frame/components/ui/PermissionsStatement/PermissionsStatement.module.scss deleted file mode 100644 index 212be008f2..0000000000 --- a/src/frame/components/ui/PermissionsStatement/PermissionsStatement.module.scss +++ /dev/null @@ -1,3 +0,0 @@ -.permissions_statement { - border-left: 0.25rem solid var(--color-border-muted); -} diff --git a/src/frame/components/ui/PermissionsStatement/PermissionsStatement.tsx b/src/frame/components/ui/PermissionsStatement/PermissionsStatement.tsx index cd646119f2..6af5e45cf5 100644 --- a/src/frame/components/ui/PermissionsStatement/PermissionsStatement.tsx +++ b/src/frame/components/ui/PermissionsStatement/PermissionsStatement.tsx @@ -1,22 +1,35 @@ -import cx from 'classnames' +import { Box } from '@primer/react' +import { PersonIcon, BriefcaseIcon } from '@primer/octicons-react' -import styles from './PermissionsStatement.module.scss' import { useTranslation } from 'src/languages/components/useTranslation' type Props = { - permissions: string + product?: string + permissions?: string } -export function PermissionsStatement({ permissions }: Props) { +export function PermissionsStatement({ product, permissions }: Props) { const { t } = useTranslation('pages') + if (!permissions && !product) return null return ( -
-
{t('permissions_statement')}
-
-
+ +
+
+

{t('permissions_callout_title')}

+
+ {permissions && ( +
+ +
+
+ )} + {product && ( +
+ +
+
+ )} +
+ ) } diff --git a/src/landings/components/TocLanding.tsx b/src/landings/components/TocLanding.tsx index b757169223..ca840d17a1 100644 --- a/src/landings/components/TocLanding.tsx +++ b/src/landings/components/TocLanding.tsx @@ -9,7 +9,7 @@ import { ArticleTitle } from 'src/frame/components/article/ArticleTitle' import { MarkdownContent } from 'src/frame/components/ui/MarkdownContent' import { ArticleList } from 'src/landings/components/ArticleList' import { ArticleGridLayout } from 'src/frame/components/article/ArticleGridLayout' -import { Alert } from 'src/frame/components/ui/Alert' +import { PermissionsStatement } from 'src/frame/components/ui/PermissionsStatement' import { Lead } from 'src/frame/components/ui/Lead' import { LearningTrackNav } from 'src/learning-track/components/article/LearningTrackNav' import { ClientSideRedirects } from 'src/rest/components/ClientSideRedirects' @@ -46,7 +46,7 @@ export const TocLanding = () => { {intro && {intro}} - {productCallout && } +
diff --git a/src/rest/components/RestReferencePage.tsx b/src/rest/components/RestReferencePage.tsx index eef983c42c..599a528af7 100644 --- a/src/rest/components/RestReferencePage.tsx +++ b/src/rest/components/RestReferencePage.tsx @@ -16,7 +16,7 @@ export type StructuredContentT = { } export const RestReferencePage = ({ restOperations }: StructuredContentT) => { - const { title, intro, renderedPage, permissions } = useAutomatedPageContext() + const { title, intro, renderedPage, permissions, product } = useAutomatedPageContext() // Scrollable code blocks in our REST API docs and elsewhere aren't accessible // via keyboard navigation without setting tabindex="0". But we don't want to set @@ -54,7 +54,7 @@ export const RestReferencePage = ({ restOperations }: StructuredContentT) => { )} - {permissions && } + {renderedPage && {renderedPage}} {restOperations.length > 0 && (