From 17fd4d33e0fdc6a9862d42b137b289a4fa01d171 Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Mon, 1 Apr 2024 13:44:01 +0000 Subject: [PATCH 01/42] wip: update type definitions and parsing logic for config values --- .../backstage-plugin-coder/dev/DevPage.tsx | 4 +- .../CoderProvider/CoderAppConfigProvider.tsx | 21 ++++---- .../src/hooks/useCoderWorkspacesConfig.ts | 51 +++++++++++-------- .../src/testHelpers/mockBackstageData.ts | 4 +- 4 files changed, 46 insertions(+), 34 deletions(-) diff --git a/plugins/backstage-plugin-coder/dev/DevPage.tsx b/plugins/backstage-plugin-coder/dev/DevPage.tsx index 2d82cc6d..abc24008 100644 --- a/plugins/backstage-plugin-coder/dev/DevPage.tsx +++ b/plugins/backstage-plugin-coder/dev/DevPage.tsx @@ -24,8 +24,8 @@ const appConfig: CoderAppConfig = { }, workspaces: { - templateName: 'devcontainers', - mode: 'manual', + defaultTemplateName: 'devcontainers', + defaultMode: 'manual', repoUrlParamKeys: ['custom_repo', 'repo_url'], params: { repo: 'custom', diff --git a/plugins/backstage-plugin-coder/src/components/CoderProvider/CoderAppConfigProvider.tsx b/plugins/backstage-plugin-coder/src/components/CoderProvider/CoderAppConfigProvider.tsx index 5d383be6..e3422292 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderProvider/CoderAppConfigProvider.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderProvider/CoderAppConfigProvider.tsx @@ -3,23 +3,24 @@ import React, { createContext, useContext, } from 'react'; - -import type { YamlConfig } from '../../hooks/useCoderWorkspacesConfig'; +import type { WorkspaceCreationMode } from '../../hooks/useCoderWorkspacesConfig'; export type CoderAppConfig = Readonly<{ deployment: Readonly<{ accessUrl: string; }>; - workspaces: Readonly< - Exclude & { - // Only specified explicitly to make templateName required - templateName: string; + // Type is meant to be used with YamlConfig from useCoderWorkspacesConfig; + // not using a mapped type because there's just enough differences that + // maintaining a relationship that way would be a nightmare of ternaries + workspaces: Readonly<{ + defaultMode?: WorkspaceCreationMode; + defaultTemplateName?: string; + params?: Record; - // Defined like this to ensure array always has at least one element - repoUrlParamKeys: readonly [string, ...string[]]; - } - >; + // Defined like this to ensure array always has at least one element + repoUrlParamKeys: readonly [string, ...string[]]; + }>; }>; const AppConfigContext = createContext(null); diff --git a/plugins/backstage-plugin-coder/src/hooks/useCoderWorkspacesConfig.ts b/plugins/backstage-plugin-coder/src/hooks/useCoderWorkspacesConfig.ts index 999a60b7..a3b61870 100644 --- a/plugins/backstage-plugin-coder/src/hooks/useCoderWorkspacesConfig.ts +++ b/plugins/backstage-plugin-coder/src/hooks/useCoderWorkspacesConfig.ts @@ -23,19 +23,22 @@ import { useCoderAppConfig, } from '../components/CoderProvider'; +const workspaceCreationModeSchema = optional( + union( + [literal('manual'), literal('auto')], + "If defined, createMode must be 'manual' or 'auto'", + ), +); + +export type WorkspaceCreationMode = Output; + // Very loose parsing requirements to make interfacing with various kinds of // YAML files as easy as possible const yamlConfigSchema = union([ undefined_(), object({ templateName: optional(string()), - mode: optional( - union( - [literal('manual'), literal('auto')], - "If defined, createMode must be 'manual' or 'auto'", - ), - ), - + mode: workspaceCreationModeSchema, params: optional( record( string(), @@ -49,6 +52,11 @@ const yamlConfigSchema = union([ }), ]); +/** + * The set of properties that the Coder plugin is configured to parse from a + * repo's catalog-info.yaml file. The entire value will be undefined if a repo + * does not have the file + */ export type YamlConfig = Output; /** @@ -56,11 +64,11 @@ export type YamlConfig = Output; * sourced from CoderAppConfig and any entity data. */ export type CoderWorkspacesConfig = - // Was originally defined in terms of fancy mapped types; ended up being a bad - // idea, because it increased coupling in a bad way + // Was originally defined in terms of fancy mapped types based on YamlConfig; + // ended up being a bad idea, because it increased coupling in a bad way Readonly<{ - creationUrl: string; - templateName: string; + creationUrl?: string; + templateName?: string; repoUrlParamKeys: readonly string[]; mode: 'manual' | 'auto'; params: Record; @@ -76,7 +84,9 @@ export function compileCoderConfig( ): CoderWorkspacesConfig { const { workspaces, deployment } = appConfig; const yamlConfig = parse(yamlConfigSchema, rawYamlConfig); - const mode = yamlConfig?.mode ?? workspaces.mode ?? 'manual'; + const mode = yamlConfig?.mode ?? workspaces.defaultMode ?? 'manual'; + const templateName = + yamlConfig?.templateName ?? workspaces.defaultTemplateName; const urlParams = new URLSearchParams({ mode }); const compiledParams: Record = {}; @@ -112,21 +122,22 @@ export function compileCoderConfig( } } - const safeTemplate = encodeURIComponent( - yamlConfig?.templateName ?? workspaces.templateName, - ); + let creationUrl: string | undefined = undefined; + if (templateName) { + const safeTemplate = encodeURIComponent(templateName); - const creationUrl = `${ - deployment.accessUrl - }/templates/${safeTemplate}/workspace?${urlParams.toString()}`; + creationUrl = `${ + deployment.accessUrl + }/templates/${safeTemplate}/workspace?${urlParams.toString()}`; + } return { + mode, creationUrl, + templateName, repoUrl: cleanedRepoUrl, repoUrlParamKeys: workspaces.repoUrlParamKeys, params: compiledParams, - templateName: yamlConfig?.templateName ?? workspaces.templateName, - mode: yamlConfig?.mode ?? workspaces.mode ?? 'manual', }; } diff --git a/plugins/backstage-plugin-coder/src/testHelpers/mockBackstageData.ts b/plugins/backstage-plugin-coder/src/testHelpers/mockBackstageData.ts index 10b8723e..875d6ce5 100644 --- a/plugins/backstage-plugin-coder/src/testHelpers/mockBackstageData.ts +++ b/plugins/backstage-plugin-coder/src/testHelpers/mockBackstageData.ts @@ -89,8 +89,8 @@ export const mockAppConfig = { }, workspaces: { - templateName: 'devcontainers', - mode: 'manual', + defaultTemplateName: 'devcontainers', + defaultMode: 'manual', repoUrlParamKeys: ['custom_repo', 'repo_url'], params: { repo: 'custom', From a95a44bafbafdcbae4ec921845fdf53144d27983 Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Mon, 1 Apr 2024 13:54:52 +0000 Subject: [PATCH 02/42] refactor: update some code for clarity --- .../src/hooks/useCoderWorkspacesConfig.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/plugins/backstage-plugin-coder/src/hooks/useCoderWorkspacesConfig.ts b/plugins/backstage-plugin-coder/src/hooks/useCoderWorkspacesConfig.ts index a3b61870..e084ab9e 100644 --- a/plugins/backstage-plugin-coder/src/hooks/useCoderWorkspacesConfig.ts +++ b/plugins/backstage-plugin-coder/src/hooks/useCoderWorkspacesConfig.ts @@ -79,7 +79,7 @@ export type CoderWorkspacesConfig = export function compileCoderConfig( appConfig: CoderAppConfig, - rawYamlConfig: unknown, + rawYamlConfig: unknown, // Function parses this into more specific type repoUrl: string | undefined, ): CoderWorkspacesConfig { const { workspaces, deployment } = appConfig; @@ -91,7 +91,7 @@ export function compileCoderConfig( const urlParams = new URLSearchParams({ mode }); const compiledParams: Record = {}; - // Can't replace this with destructuring, because that is all-or-nothing; + // Can't replace section with destructuring, because that's all-or-nothing; // there's no easy way to granularly check each property without a loop const paramsPrecedence = [workspaces.params, yamlConfig?.params ?? {}]; for (const params of paramsPrecedence) { @@ -125,7 +125,6 @@ export function compileCoderConfig( let creationUrl: string | undefined = undefined; if (templateName) { const safeTemplate = encodeURIComponent(templateName); - creationUrl = `${ deployment.accessUrl }/templates/${safeTemplate}/workspace?${urlParams.toString()}`; From 18e80f016dc8475b7f139f4c0dfa6d111eb81634 Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Mon, 1 Apr 2024 15:05:05 +0000 Subject: [PATCH 03/42] fix: update property names in top-level config --- packages/app/src/components/catalog/EntityPage.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/app/src/components/catalog/EntityPage.tsx b/packages/app/src/components/catalog/EntityPage.tsx index 84f1e68a..6c4f9df1 100644 --- a/packages/app/src/components/catalog/EntityPage.tsx +++ b/packages/app/src/components/catalog/EntityPage.tsx @@ -137,8 +137,8 @@ const coderAppConfig: CoderAppConfig = { }, workspaces: { - templateName: 'devcontainers', - mode: 'manual', + defaultTemplateName: 'devcontainers', + defaultMode: 'manual', repoUrlParamKeys: ['custom_repo', 'repo_url'], params: { repo: 'custom', From 3cfc7c5bfbd02e0dee426ad8940b5baaac64dea6 Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Mon, 1 Apr 2024 15:33:53 +0000 Subject: [PATCH 04/42] wip: commit progress on link update --- .../CreateWorkspaceLink.tsx | 54 +++++++++++++++---- 1 file changed, 45 insertions(+), 9 deletions(-) diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/CreateWorkspaceLink.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/CreateWorkspaceLink.tsx index 10c8fb86..4acb1adf 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/CreateWorkspaceLink.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/CreateWorkspaceLink.tsx @@ -1,16 +1,22 @@ import React, { type AnchorHTMLAttributes, type ForwardedRef } from 'react'; -import { makeStyles } from '@material-ui/core'; +import { type Theme, makeStyles } from '@material-ui/core'; import { useWorkspacesCardContext } from './Root'; import { VisuallyHidden } from '../VisuallyHidden'; import AddIcon from '@material-ui/icons/AddCircleOutline'; import Tooltip, { type TooltipProps } from '@material-ui/core/Tooltip'; -const useStyles = makeStyles(theme => { +type StyleInput = Readonly<{ + canCreateWorkspace: boolean; +}>; + +type StyleKeys = 'root' | 'icon'; + +const useStyles = makeStyles(theme => { const padding = theme.spacing(0.5); return { - root: { + root: ({ canCreateWorkspace }) => ({ padding, width: theme.spacing(4) + padding, height: theme.spacing(4) + padding, @@ -24,12 +30,14 @@ const useStyles = makeStyles(theme => { '&:hover': { backgroundColor: theme.palette.action.hover, }, - }, + }), + + icon: {}, }; }); type CreateButtonLinkProps = Readonly< - AnchorHTMLAttributes & { + Omit, 'aria-disabled'> & { tooltipText?: string; tooltipProps?: Omit; tooltipRef?: ForwardedRef; @@ -45,22 +53,50 @@ export const CreateWorkspaceLink = ({ tooltipProps = {}, ...delegatedProps }: CreateButtonLinkProps) => { - const styles = useStyles(); const { workspacesConfig } = useWorkspacesCardContext(); + const canCreateWorkspace = Boolean(workspacesConfig.creationUrl); + const styles = useStyles({ canCreateWorkspace }); return ( - + + {/* eslint-disable-next-line jsx-a11y/no-redundant-roles -- + Some browsers will render out elements as having no role when the + href value is undefined. Need to make sure that the link role is + always defined, no matter what. The ESLint rule is wrong here. */} {children ?? } - {tooltipText} - {target === '_blank' && <> (Link opens in new tab)} + {canCreateWorkspace ? ( + <> + {tooltipText} + {target === '_blank' && <> (Link opens in new tab)} + + ) : ( + <> + This component does not have a usable template name. Please see + the disclosure section in this widget for steps on adding this + information. + + )} From ec1e7942eef27020e3ea356241958dee81c3ae5b Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Mon, 1 Apr 2024 15:42:18 +0000 Subject: [PATCH 05/42] chore: finish updates for CreateWorkspaceLink --- .../CoderWorkspacesCard/CreateWorkspaceLink.tsx | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/CreateWorkspaceLink.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/CreateWorkspaceLink.tsx index 4acb1adf..92b35296 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/CreateWorkspaceLink.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/CreateWorkspaceLink.tsx @@ -10,8 +10,7 @@ type StyleInput = Readonly<{ canCreateWorkspace: boolean; }>; -type StyleKeys = 'root' | 'icon'; - +type StyleKeys = 'root'; const useStyles = makeStyles(theme => { const padding = theme.spacing(0.5); @@ -20,19 +19,23 @@ const useStyles = makeStyles(theme => { padding, width: theme.spacing(4) + padding, height: theme.spacing(4) + padding, + cursor: 'pointer', display: 'flex', justifyContent: 'center', alignItems: 'center', backgroundColor: 'inherit', borderRadius: '9999px', lineHeight: 1, + color: canCreateWorkspace + ? theme.palette.text.primary + : theme.palette.text.disabled, '&:hover': { - backgroundColor: theme.palette.action.hover, + backgroundColor: canCreateWorkspace + ? theme.palette.action.hover + : 'inherit', }, }), - - icon: {}, }; }); From 50ad128d57d1457e288e4f40d27069b782c3642c Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Mon, 1 Apr 2024 16:16:23 +0000 Subject: [PATCH 06/42] chore: add new test case for disabled state --- .../CreateWorkspaceLink.test.tsx | 57 +++++++++++++++++-- .../CreateWorkspaceLink.tsx | 5 +- 2 files changed, 55 insertions(+), 7 deletions(-) diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/CreateWorkspaceLink.test.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/CreateWorkspaceLink.test.tsx index b26c86f1..334837b4 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/CreateWorkspaceLink.test.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/CreateWorkspaceLink.test.tsx @@ -1,17 +1,43 @@ import React from 'react'; import { screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; -import { mockAppConfig } from '../../testHelpers/mockBackstageData'; +import { + mockAppConfig, + mockCoderWorkspacesConfig, +} from '../../testHelpers/mockBackstageData'; import { renderInCoderEnvironment } from '../../testHelpers/setup'; -import { Root } from './Root'; +import { CardContext, WorkspacesCardContext } from './Root'; import { CreateWorkspaceLink } from './CreateWorkspaceLink'; +import type { CoderWorkspacesConfig } from '../../hooks/useCoderWorkspacesConfig'; + +type RenderInputs = Readonly<{ + hasTemplateName?: boolean; +}>; + +function render(inputs?: RenderInputs) { + const { hasTemplateName = true } = inputs ?? {}; + + const mockWorkspacesConfig: CoderWorkspacesConfig = { + ...mockCoderWorkspacesConfig, + creationUrl: hasTemplateName + ? mockCoderWorkspacesConfig.creationUrl + : undefined, + }; + + const mockContextValue: WorkspacesCardContext = { + workspacesConfig: mockWorkspacesConfig, + headerId: "Doesn't matter", + queryFilter: "Also doesn't matter", + onFilterChange: jest.fn(), + workspacesQuery: + null as unknown as WorkspacesCardContext['workspacesQuery'], + }; -function render() { return renderInCoderEnvironment({ children: ( - + - + ), }); } @@ -37,4 +63,25 @@ describe(`${CreateWorkspaceLink.name}`, () => { const tooltip = await screen.findByText('Add a new workspace'); expect(tooltip).toBeInTheDocument(); }); + + it('Will be disabled and will indicate to the user when there is no usable templateName value', async () => { + await render({ hasTemplateName: false }); + const link = screen.getByRole('link'); + + // Check that the link is "disabled" properly (see main component file for + // a link to resource explaining edge cases). Can't assert toBeDisabled, + // because links don't support the disabled attribute; also can't check + // the .role and .ariaDisabled properties on the link variable, because even + // though they exist in the output, RTL doesn't correctly pass them through. + // This is a niche edge case - have to check properties on the raw HTML node + expect(link.href).toBe(''); + expect(link.getAttribute('role')).toBe('link'); + expect(link.getAttribute('aria-disabled')).toBe('true'); + + // Make sure tooltip is also updated + const user = userEvent.setup(); + await user.hover(link); + const tooltip = await screen.findByText('Please add a template name value'); + expect(tooltip).toBeInTheDocument(); + }); }); diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/CreateWorkspaceLink.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/CreateWorkspaceLink.tsx index 92b35296..8e509bd8 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/CreateWorkspaceLink.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/CreateWorkspaceLink.tsx @@ -70,8 +70,9 @@ export const CreateWorkspaceLink = ({ > {/* eslint-disable-next-line jsx-a11y/no-redundant-roles -- Some browsers will render out elements as having no role when the - href value is undefined. Need to make sure that the link role is - always defined, no matter what. The ESLint rule is wrong here. */} + href value is undefined or an empty string. Need to make sure that the + link role is always defined, no matter what. The ESLint rule is wrong + here. */} Date: Mon, 1 Apr 2024 19:27:06 +0000 Subject: [PATCH 07/42] fix: cleanup markup and text for EntityDataReminder --- .../EntityDataReminder.tsx | 30 ++++++++++++------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/EntityDataReminder.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/EntityDataReminder.tsx index c6335d85..decbf825 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/EntityDataReminder.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/EntityDataReminder.tsx @@ -13,7 +13,8 @@ type UseStyleKeys = | 'button' | 'disclosureTriangle' | 'disclosureBody' - | 'snippet'; + | 'snippet' + | 'link'; const useStyles = makeStyles(theme => ({ root: ({ hasData }) => ({ @@ -21,6 +22,14 @@ const useStyles = makeStyles(theme => ({ borderTop: hasData ? 'none' : `1px solid ${theme.palette.divider}`, }), + link: { + color: theme.palette.link, + + '&:hover': { + textDecoration: 'underline', + }, + }, + button: { width: '100%', textAlign: 'left', @@ -41,12 +50,13 @@ const useStyles = makeStyles(theme => ({ display: 'inline-block', textAlign: 'right', width: theme.spacing(2.25), + fontSize: '0.7rem', }, disclosureBody: { margin: 0, padding: `${theme.spacing(0.5)}px ${theme.spacing(3.5)}px 0 ${theme.spacing( - 3.75, + 4, )}px`, }, @@ -89,24 +99,24 @@ export const EntityDataReminder = () => { {isExpanded ? '▼' : '►'} {' '} - {isExpanded ? 'Hide text' : 'Why am I seeing all workspaces?'} + Why am I not seeing any workspaces? {isExpanded && (

- This component displays all workspaces when the entity has no repo URL - to filter by. Consider disabling{' '} - readEntityData (details in our{' '} + This component displays only displays all workspaces when the value of + the readEntityData prop is{' '} + false. See{' '} - documentation + our documentation (link opens in new tab) - - ). + {' '} + for more info.

)} From 730e979ef14e9594bed4f7834333dac6ce20f208 Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Mon, 1 Apr 2024 20:01:21 +0000 Subject: [PATCH 08/42] chore: add readEntityData as context value --- .../CoderWorkspacesCard/CoderWorkspacesCard.tsx | 8 +++++--- .../CoderWorkspacesCard/CreateWorkspaceLink.test.tsx | 3 +++ .../src/components/CoderWorkspacesCard/Root.tsx | 10 +++------- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/CoderWorkspacesCard.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/CoderWorkspacesCard.tsx index 64bff808..ac53b0f0 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/CoderWorkspacesCard.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/CoderWorkspacesCard.tsx @@ -7,6 +7,7 @@ import { SearchBox } from './SearchBox'; import { WorkspacesList } from './WorkspacesList'; import { CreateWorkspaceLink } from './CreateWorkspaceLink'; import { ExtraActionsButton } from './ExtraActionsButton'; +import { ReminderAccordion } from './ReminderAccordion'; const useStyles = makeStyles(theme => ({ searchWrapper: { @@ -15,9 +16,9 @@ const useStyles = makeStyles(theme => ({ }, })); -export const CoderWorkspacesCard = ( - props: Omit, -) => { +type Props = Omit; + +export const CoderWorkspacesCard = (props: Props) => { const styles = useStyles(); return ( @@ -37,6 +38,7 @@ export const CoderWorkspacesCard = ( + ); }; diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/CreateWorkspaceLink.test.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/CreateWorkspaceLink.test.tsx index 334837b4..e9fd6a49 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/CreateWorkspaceLink.test.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/CreateWorkspaceLink.test.tsx @@ -26,6 +26,9 @@ function render(inputs?: RenderInputs) { const mockContextValue: WorkspacesCardContext = { workspacesConfig: mockWorkspacesConfig, + + // Everything below this comment doesn't matter for the test logic + readEntityData: false, headerId: "Doesn't matter", queryFilter: "Also doesn't matter", onFilterChange: jest.fn(), diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/Root.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/Root.tsx index 6829753a..e8fd6042 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/Root.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/Root.tsx @@ -22,11 +22,11 @@ import type { Workspace } from '../../typesConstants'; import { useCoderWorkspacesQuery } from '../../hooks/useCoderWorkspacesQuery'; import { Card } from '../Card'; import { CoderAuthWrapper } from '../CoderAuthWrapper'; -import { EntityDataReminder } from './EntityDataReminder'; export type WorkspacesQuery = UseQueryResult; export type WorkspacesCardContext = Readonly<{ + readEntityData: boolean; queryFilter: string; onFilterChange: (newFilter: string) => void; workspacesQuery: WorkspacesQuery; @@ -56,7 +56,6 @@ export const Root = ({ readEntityData = false, ...delegatedProps }: WorkspacesCardProps) => { - const hookId = useId(); const [innerFilter, setInnerFilter] = useState(defaultQueryFilter); const activeFilter = outerFilter ?? innerFilter; @@ -66,17 +65,15 @@ export const Root = ({ coderQuery: activeFilter, }); + const hookId = useId(); const headerId = `${hookId}-header`; - const showEntityDataReminder = - readEntityData && - !workspacesConfig.repoUrl && - workspacesQuery.data !== undefined; return ( elements automatically introduce */}
{children}
- {showEntityDataReminder && }
From cd804f607a852a350de39e6c91b0af115255c477 Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Mon, 1 Apr 2024 20:01:49 +0000 Subject: [PATCH 09/42] refactor: rename DataEntityReminder to ReminderAcoordionItem --- ...DataReminder.test.tsx => ReminderAccordionItem.test.tsx} | 6 +++--- .../{EntityDataReminder.tsx => ReminderAccordionItem.tsx} | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) rename plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/{EntityDataReminder.test.tsx => ReminderAccordionItem.test.tsx} (85%) rename plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/{EntityDataReminder.tsx => ReminderAccordionItem.tsx} (98%) diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/EntityDataReminder.test.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordionItem.test.tsx similarity index 85% rename from plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/EntityDataReminder.test.tsx rename to plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordionItem.test.tsx index 61536c72..5758fb37 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/EntityDataReminder.test.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordionItem.test.tsx @@ -3,19 +3,19 @@ import { screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { renderInCoderEnvironment } from '../../testHelpers/setup'; import { Root } from './Root'; -import { EntityDataReminder } from './EntityDataReminder'; +import { ReminderAccordionItem } from './ReminderAccordionItem'; function render() { return renderInCoderEnvironment({ children: ( - + ), }); } -describe(`${EntityDataReminder.name}`, () => { +describe(`${ReminderAccordionItem.name}`, () => { it('Will toggle between showing/hiding the disclosure info when the user clicks it', async () => { await render(); const user = userEvent.setup(); diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/EntityDataReminder.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordionItem.tsx similarity index 98% rename from plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/EntityDataReminder.tsx rename to plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordionItem.tsx index decbf825..7c3c3fe1 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/EntityDataReminder.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordionItem.tsx @@ -76,7 +76,7 @@ const useStyles = makeStyles(theme => ({ }, })); -export const EntityDataReminder = () => { +export const ReminderAccordionItem = () => { const [isExpanded, setIsExpanded] = useState(false); const { workspacesQuery } = useWorkspacesCardContext(); const styles = useStyles({ hasData: workspacesQuery.data !== undefined }); From 9e541c2d66afe129b5417adceba22834571cd4ee Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Mon, 1 Apr 2024 20:26:12 +0000 Subject: [PATCH 10/42] chore: extract core accordion item logic to parent --- .../CoderWorkspacesCard/ReminderAccordion.tsx | 124 ++++++++++++++++++ .../ReminderAccordionItem.tsx | 109 +++++---------- 2 files changed, 157 insertions(+), 76 deletions(-) create mode 100644 plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.tsx diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.tsx new file mode 100644 index 00000000..c0ca6345 --- /dev/null +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.tsx @@ -0,0 +1,124 @@ +import React, { Fragment, ReactNode, useState } from 'react'; +import { useWorkspacesCardContext } from './Root'; +import { ReminderAccordionItem } from './ReminderAccordionItem'; +import { VisuallyHidden } from '../VisuallyHidden'; +import { Theme, makeStyles } from '@material-ui/core'; + +type AccordionItemInfo = Readonly<{ + id: string; + canDisplay: boolean; + headerText: ReactNode; + bodyText: ReactNode; +}>; + +type UseStyleProps = Readonly<{ + hasData: boolean; +}>; + +type UseStyleKeys = 'root' | 'snippet' | 'link'; + +const useStyles = makeStyles(theme => ({ + root: ({ hasData }) => ({ + paddingTop: theme.spacing(1), + borderTop: hasData ? 'none' : `1px solid ${theme.palette.divider}`, + }), + + link: { + color: theme.palette.link, + + '&:hover': { + textDecoration: 'underline', + }, + }, + + snippet: { + color: theme.palette.text.primary, + borderRadius: theme.spacing(0.5), + padding: `${theme.spacing(0.2)}px ${theme.spacing(1)}px`, + backgroundColor: () => { + const defaultBackgroundColor = theme.palette.background.default; + const isDefaultSpotifyLightTheme = + defaultBackgroundColor.toUpperCase() === '#F8F8F8'; + + return isDefaultSpotifyLightTheme + ? 'hsl(0deg,0%,93%)' + : defaultBackgroundColor; + }, + }, +})); + +type Props = Readonly<{ + showEntityReminder?: boolean; + showTemplateNameReminder?: boolean; +}>; + +export function ReminderAccordion({ + showEntityReminder = true, + showTemplateNameReminder = true, +}: Props) { + const [activeItemId, setActiveItemId] = useState(); + const { readEntityData, workspacesConfig, workspacesQuery } = + useWorkspacesCardContext(); + const styles = useStyles({ hasData: workspacesQuery.data !== undefined }); + + const toggleAccordionGroup = (newItemId: string) => { + if (newItemId === activeItemId) { + setActiveItemId(undefined); + } else { + setActiveItemId(newItemId); + } + }; + + const accordionData: readonly AccordionItemInfo[] = [ + { + id: 'entity', + canDisplay: + showEntityReminder && + readEntityData && + !workspacesConfig.repoUrl && + workspacesQuery.data !== undefined, + headerText: 'Why am I not seeing any workspaces?', + bodyText: ( + <> + This component displays only displays all workspaces when the value of + the readEntityData prop is{' '} + false. See{' '} + + our documentation + (link opens in new tab) + {' '} + for more info. + + ), + }, + { + id: 'templateName', + canDisplay: showTemplateNameReminder && !workspacesConfig.creationUrl, + headerText: '', + bodyText: 'Blah', + }, + ]; + + return ( + <> + {accordionData.map(({ id, canDisplay, headerText, bodyText }) => ( + + {canDisplay && ( + toggleAccordionGroup(id)} + > + {bodyText} + + )} + + ))} + + ); +} diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordionItem.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordionItem.tsx index 7c3c3fe1..cbe46775 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordionItem.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordionItem.tsx @@ -1,51 +1,8 @@ -import React, { useState } from 'react'; +import React, { type PropsWithChildren, type ReactNode } from 'react'; import { useId } from '../../hooks/hookPolyfills'; -import { Theme, makeStyles } from '@material-ui/core'; -import { VisuallyHidden } from '../VisuallyHidden'; -import { useWorkspacesCardContext } from './Root'; - -type UseStyleProps = Readonly<{ - hasData: boolean; -}>; - -type UseStyleKeys = - | 'root' - | 'button' - | 'disclosureTriangle' - | 'disclosureBody' - | 'snippet' - | 'link'; - -const useStyles = makeStyles(theme => ({ - root: ({ hasData }) => ({ - paddingTop: theme.spacing(1), - borderTop: hasData ? 'none' : `1px solid ${theme.palette.divider}`, - }), - - link: { - color: theme.palette.link, - - '&:hover': { - textDecoration: 'underline', - }, - }, - - button: { - width: '100%', - textAlign: 'left', - color: theme.palette.text.primary, - backgroundColor: theme.palette.background.paper, - padding: theme.spacing(1), - border: 'none', - borderRadius: theme.shape.borderRadius, - fontSize: theme.typography.body2.fontSize, - cursor: 'pointer', - - '&:hover': { - backgroundColor: theme.palette.action.hover, - }, - }, +import { makeStyles } from '@material-ui/core'; +const useStyles = makeStyles(theme => ({ disclosureTriangle: { display: 'inline-block', textAlign: 'right', @@ -60,26 +17,38 @@ const useStyles = makeStyles(theme => ({ )}px`, }, - snippet: { + button: { + width: '100%', + textAlign: 'left', color: theme.palette.text.primary, - borderRadius: theme.spacing(0.5), - padding: `${theme.spacing(0.2)}px ${theme.spacing(1)}px`, - backgroundColor: () => { - const defaultBackgroundColor = theme.palette.background.default; - const isDefaultSpotifyLightTheme = - defaultBackgroundColor.toUpperCase() === '#F8F8F8'; + backgroundColor: theme.palette.background.paper, + padding: theme.spacing(1), + border: 'none', + borderRadius: theme.shape.borderRadius, + fontSize: theme.typography.body2.fontSize, + cursor: 'pointer', - return isDefaultSpotifyLightTheme - ? 'hsl(0deg,0%,93%)' - : defaultBackgroundColor; + '&:hover': { + backgroundColor: theme.palette.action.hover, }, }, })); -export const ReminderAccordionItem = () => { - const [isExpanded, setIsExpanded] = useState(false); - const { workspacesQuery } = useWorkspacesCardContext(); - const styles = useStyles({ hasData: workspacesQuery.data !== undefined }); +type AccordionItemProps = Readonly< + PropsWithChildren<{ + isExpanded: boolean; + onExpansion: () => void; + headerText: ReactNode; + }> +>; + +export const ReminderAccordionItem = ({ + isExpanded, + onExpansion, + headerText, + children, +}: AccordionItemProps) => { + const styles = useStyles(); const hookId = useId(); const disclosureBodyId = `${hookId}-disclosure-body`; @@ -88,35 +57,23 @@ export const ReminderAccordionItem = () => { // functionality with and elements. Would likely clean up // the component code a ton but might reduce control over screen reader output return ( -
+
{isExpanded && (

- This component displays only displays all workspaces when the value of - the readEntityData prop is{' '} - false. See{' '} - - our documentation - (link opens in new tab) - {' '} - for more info. + {children}

)}
From f445df4271e86df4c965e91ddf95268029eb606f Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Mon, 1 Apr 2024 20:52:23 +0000 Subject: [PATCH 11/42] chore: finish initial version of ReminderAccordion --- .../AccordionItem.test.tsx} | 8 +- .../AccordionItem.tsx} | 3 +- .../CoderWorkspacesCard/ReminderAccordion.tsx | 74 +++++++++---------- .../InlineCodeSnippet/InlineCodeSnippet.tsx | 32 ++++++++ 4 files changed, 71 insertions(+), 46 deletions(-) rename plugins/backstage-plugin-coder/src/components/{CoderWorkspacesCard/ReminderAccordionItem.test.tsx => AccordionItem/AccordionItem.test.tsx} (83%) rename plugins/backstage-plugin-coder/src/components/{CoderWorkspacesCard/ReminderAccordionItem.tsx => AccordionItem/AccordionItem.tsx} (97%) create mode 100644 plugins/backstage-plugin-coder/src/components/InlineCodeSnippet/InlineCodeSnippet.tsx diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordionItem.test.tsx b/plugins/backstage-plugin-coder/src/components/AccordionItem/AccordionItem.test.tsx similarity index 83% rename from plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordionItem.test.tsx rename to plugins/backstage-plugin-coder/src/components/AccordionItem/AccordionItem.test.tsx index 5758fb37..e8dbc5d8 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordionItem.test.tsx +++ b/plugins/backstage-plugin-coder/src/components/AccordionItem/AccordionItem.test.tsx @@ -2,20 +2,20 @@ import React from 'react'; import { screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { renderInCoderEnvironment } from '../../testHelpers/setup'; -import { Root } from './Root'; -import { ReminderAccordionItem } from './ReminderAccordionItem'; +import { Root } from '../CoderWorkspacesCard/Root'; +import { AccordionItem } from './AccordionItem'; function render() { return renderInCoderEnvironment({ children: ( - + ), }); } -describe(`${ReminderAccordionItem.name}`, () => { +describe(`${AccordionItem.name}`, () => { it('Will toggle between showing/hiding the disclosure info when the user clicks it', async () => { await render(); const user = userEvent.setup(); diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordionItem.tsx b/plugins/backstage-plugin-coder/src/components/AccordionItem/AccordionItem.tsx similarity index 97% rename from plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordionItem.tsx rename to plugins/backstage-plugin-coder/src/components/AccordionItem/AccordionItem.tsx index cbe46775..f5906d59 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordionItem.tsx +++ b/plugins/backstage-plugin-coder/src/components/AccordionItem/AccordionItem.tsx @@ -42,14 +42,13 @@ type AccordionItemProps = Readonly< }> >; -export const ReminderAccordionItem = ({ +export const AccordionItem = ({ isExpanded, onExpansion, headerText, children, }: AccordionItemProps) => { const styles = useStyles(); - const hookId = useId(); const disclosureBodyId = `${hookId}-disclosure-body`; diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.tsx index c0ca6345..811a637f 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.tsx @@ -1,8 +1,9 @@ -import React, { Fragment, ReactNode, useState } from 'react'; -import { useWorkspacesCardContext } from './Root'; -import { ReminderAccordionItem } from './ReminderAccordionItem'; +import React, { type ReactNode, Fragment, useState } from 'react'; +import { type Theme, makeStyles } from '@material-ui/core'; import { VisuallyHidden } from '../VisuallyHidden'; -import { Theme, makeStyles } from '@material-ui/core'; +import { useWorkspacesCardContext } from './Root'; +import { AccordionItem } from '../AccordionItem/AccordionItem'; +import { InlineCodeSnippet } from '../InlineCodeSnippet/InlineCodeSnippet'; type AccordionItemInfo = Readonly<{ id: string; @@ -11,13 +12,12 @@ type AccordionItemInfo = Readonly<{ bodyText: ReactNode; }>; -type UseStyleProps = Readonly<{ +type StyleKeys = 'root' | 'link'; +type StyleInputs = Readonly<{ hasData: boolean; }>; -type UseStyleKeys = 'root' | 'snippet' | 'link'; - -const useStyles = makeStyles(theme => ({ +const useStyles = makeStyles(theme => ({ root: ({ hasData }) => ({ paddingTop: theme.spacing(1), borderTop: hasData ? 'none' : `1px solid ${theme.palette.divider}`, @@ -25,26 +25,10 @@ const useStyles = makeStyles(theme => ({ link: { color: theme.palette.link, - '&:hover': { textDecoration: 'underline', }, }, - - snippet: { - color: theme.palette.text.primary, - borderRadius: theme.spacing(0.5), - padding: `${theme.spacing(0.2)}px ${theme.spacing(1)}px`, - backgroundColor: () => { - const defaultBackgroundColor = theme.palette.background.default; - const isDefaultSpotifyLightTheme = - defaultBackgroundColor.toUpperCase() === '#F8F8F8'; - - return isDefaultSpotifyLightTheme - ? 'hsl(0deg,0%,93%)' - : defaultBackgroundColor; - }, - }, })); type Props = Readonly<{ @@ -61,14 +45,6 @@ export function ReminderAccordion({ useWorkspacesCardContext(); const styles = useStyles({ hasData: workspacesQuery.data !== undefined }); - const toggleAccordionGroup = (newItemId: string) => { - if (newItemId === activeItemId) { - setActiveItemId(undefined); - } else { - setActiveItemId(newItemId); - } - }; - const accordionData: readonly AccordionItemInfo[] = [ { id: 'entity', @@ -81,8 +57,8 @@ export function ReminderAccordion({ bodyText: ( <> This component displays only displays all workspaces when the value of - the readEntityData prop is{' '} - false. See{' '} + the readEntityData prop is{' '} + false. See{' '} Why can't I make a new workspace?, + bodyText: ( + <> + This component cannot make a new workspace without a template name + value. Values can be provided via{' '} + defaultTemplateName in{' '} + CoderAppConfig or the{' '} + templateName property in a + repo's catalog-info.yaml file. + See our documentation for more information. + + ), }, ]; + const toggleAccordionGroup = (newItemId: string) => { + if (newItemId === activeItemId) { + setActiveItemId(undefined); + } else { + setActiveItemId(newItemId); + } + }; + return ( - <> +
{accordionData.map(({ id, canDisplay, headerText, bodyText }) => ( {canDisplay && ( - toggleAccordionGroup(id)} > {bodyText} - + )} ))} - +
); } diff --git a/plugins/backstage-plugin-coder/src/components/InlineCodeSnippet/InlineCodeSnippet.tsx b/plugins/backstage-plugin-coder/src/components/InlineCodeSnippet/InlineCodeSnippet.tsx new file mode 100644 index 00000000..7743bdc8 --- /dev/null +++ b/plugins/backstage-plugin-coder/src/components/InlineCodeSnippet/InlineCodeSnippet.tsx @@ -0,0 +1,32 @@ +import React, { HTMLAttributes } from 'react'; +import { makeStyles } from '@material-ui/core'; + +const useStyles = makeStyles(theme => ({ + root: { + fontSize: theme.typography.body2.fontSize, + color: theme.palette.text.primary, + borderRadius: theme.spacing(0.5), + padding: `${theme.spacing(0.2)}px ${theme.spacing(1)}px`, + backgroundColor: () => { + const isLightTheme = theme.palette.type === 'light'; + return isLightTheme + ? 'hsl(0deg,0%,93%)' + : theme.palette.background.default; + }, + }, +})); + +type Props = Readonly< + Omit, 'children'> & { + children: string; + } +>; + +export function InlineCodeSnippet({ children, ...delegatedProps }: Props) { + const styles = useStyles(); + return ( + + {children} + + ); +} From f841d1a929ae3dd32104d58edf9fa9cc9d6450e1 Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Mon, 1 Apr 2024 21:03:06 +0000 Subject: [PATCH 12/42] wip: commit test stubs for ReminderAccordion --- .../ReminderAccordion.test.tsx | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.test.tsx diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.test.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.test.tsx new file mode 100644 index 00000000..6fc4bc0d --- /dev/null +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.test.tsx @@ -0,0 +1,19 @@ +import { ReminderAccordion } from './ReminderAccordion'; + +describe(`${ReminderAccordion.name}`, () => { + it('Lets the user open a single accordion item', async () => { + expect.hasAssertions(); + }); + + it('Will close an open accordion item when that item is clicked', async () => { + expect.hasAssertions(); + }); + + it('Will close any other open accordion items when a new item is clicked', async () => { + expect.hasAssertions(); + }); + + it('Lets the user conditionally hide accordion items based on props', async () => { + expect.hasAssertions(); + }); +}); From 98a94c830bd0f9d57c51b535dfff3cb166460e55 Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Mon, 1 Apr 2024 21:03:40 +0000 Subject: [PATCH 13/42] chore: rename isReadingEntityData prop --- .../src/components/CoderWorkspacesCard/ReminderAccordion.tsx | 4 ++-- .../src/components/CoderWorkspacesCard/Root.tsx | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.tsx index 811a637f..1312988b 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.tsx @@ -41,7 +41,7 @@ export function ReminderAccordion({ showTemplateNameReminder = true, }: Props) { const [activeItemId, setActiveItemId] = useState(); - const { readEntityData, workspacesConfig, workspacesQuery } = + const { isReadingEntityData, workspacesConfig, workspacesQuery } = useWorkspacesCardContext(); const styles = useStyles({ hasData: workspacesQuery.data !== undefined }); @@ -50,7 +50,7 @@ export function ReminderAccordion({ id: 'entity', canDisplay: showEntityReminder && - readEntityData && + isReadingEntityData && !workspacesConfig.repoUrl && workspacesQuery.data !== undefined, headerText: 'Why am I not seeing any workspaces?', diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/Root.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/Root.tsx index e8fd6042..70a64df3 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/Root.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/Root.tsx @@ -26,7 +26,7 @@ import { CoderAuthWrapper } from '../CoderAuthWrapper'; export type WorkspacesQuery = UseQueryResult; export type WorkspacesCardContext = Readonly<{ - readEntityData: boolean; + isReadingEntityData: boolean; queryFilter: string; onFilterChange: (newFilter: string) => void; workspacesQuery: WorkspacesQuery; @@ -73,9 +73,9 @@ export const Root = ({ { setInnerFilter(newFilter); From d2ac1ac8185eaed50c170456b39457a492602104 Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Mon, 1 Apr 2024 21:04:24 +0000 Subject: [PATCH 14/42] chore: update mock context values in tests --- .../components/CoderWorkspacesCard/CreateWorkspaceLink.test.tsx | 2 +- .../components/CoderWorkspacesCard/ExtraActionsButton.test.tsx | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/CreateWorkspaceLink.test.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/CreateWorkspaceLink.test.tsx index e9fd6a49..23e1659d 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/CreateWorkspaceLink.test.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/CreateWorkspaceLink.test.tsx @@ -28,7 +28,7 @@ function render(inputs?: RenderInputs) { workspacesConfig: mockWorkspacesConfig, // Everything below this comment doesn't matter for the test logic - readEntityData: false, + isReadingEntityData: false, headerId: "Doesn't matter", queryFilter: "Also doesn't matter", onFilterChange: jest.fn(), diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ExtraActionsButton.test.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ExtraActionsButton.test.tsx index 732a859d..96ded1d6 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ExtraActionsButton.test.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ExtraActionsButton.test.tsx @@ -43,6 +43,7 @@ async function renderButton({ buttonText }: RenderInputs) { refetch, } as unknown as WorkspacesCardContext['workspacesQuery']; const mockContext: WorkspacesCardContext = { + isReadingEntityData: true, headerId: "Doesn't matter", queryFilter: "Doesn't matter", onFilterChange: jest.fn(), From b415ce588342a9c9923f643051dcdf7c88ec11fd Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Mon, 1 Apr 2024 21:04:46 +0000 Subject: [PATCH 15/42] wip: commit test stub for hiding cta button when there is no repo URL --- .../components/CoderWorkspacesCard/WorkspacesList.test.tsx | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/WorkspacesList.test.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/WorkspacesList.test.tsx index f2033a82..92ab025b 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/WorkspacesList.test.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/WorkspacesList.test.tsx @@ -16,6 +16,7 @@ function renderWorkspacesList(inputs?: RenderInputs) { const { renderListItem, workspacesQuery } = inputs ?? {}; const mockContext: WorkspacesCardContext = { + isReadingEntityData: true, headerId: "Doesn't matter", queryFilter: "Also doesn't matter", onFilterChange: jest.fn(), @@ -63,4 +64,8 @@ describe(`${WorkspacesList.name}`, () => { expect(listItem).toBeInstanceOf(HTMLLIElement); } }); + + it('Does not display the call-to-action button for making new workspaces when there is no workspace creation URL', async () => { + expect.hasAssertions(); + }); }); From 604c7a4ec432bf3ef1dbe353f97c9d16cbeb350c Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Mon, 1 Apr 2024 21:06:13 +0000 Subject: [PATCH 16/42] chore: hide CTA button when there is no repo URL --- .../src/components/CoderWorkspacesCard/WorkspacesList.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/WorkspacesList.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/WorkspacesList.tsx index 03860201..a9293dab 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/WorkspacesList.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/WorkspacesList.tsx @@ -97,7 +97,7 @@ export const WorkspacesList = ({ {workspacesQuery.data?.length === 0 && ( <> {emptyState ?? ( - + {repoUrl ? (
No workspaces found for repo From 86356638d3a23b4cb8f9ceef0c735cdd07c9f865 Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Tue, 2 Apr 2024 12:20:50 +0000 Subject: [PATCH 17/42] chore: rename AccordionItem to Disclosure --- .../CoderWorkspacesCard/ReminderAccordion.tsx | 8 ++++---- .../Disclosure.test.tsx} | 11 ++++++++--- .../AccordionItem.tsx => Disclosure/Disclosure.tsx} | 8 ++++---- 3 files changed, 16 insertions(+), 11 deletions(-) rename plugins/backstage-plugin-coder/src/components/{AccordionItem/AccordionItem.test.tsx => Disclosure/Disclosure.test.tsx} (83%) rename plugins/backstage-plugin-coder/src/components/{AccordionItem/AccordionItem.tsx => Disclosure/Disclosure.tsx} (92%) diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.tsx index 1312988b..dfa138ac 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.tsx @@ -2,7 +2,7 @@ import React, { type ReactNode, Fragment, useState } from 'react'; import { type Theme, makeStyles } from '@material-ui/core'; import { VisuallyHidden } from '../VisuallyHidden'; import { useWorkspacesCardContext } from './Root'; -import { AccordionItem } from '../AccordionItem/AccordionItem'; +import { Disclosure } from '../Disclosure/Disclosure'; import { InlineCodeSnippet } from '../InlineCodeSnippet/InlineCodeSnippet'; type AccordionItemInfo = Readonly<{ @@ -99,17 +99,17 @@ export function ReminderAccordion({ }; return ( -
+
{accordionData.map(({ id, canDisplay, headerText, bodyText }) => ( {canDisplay && ( - toggleAccordionGroup(id)} > {bodyText} - + )} ))} diff --git a/plugins/backstage-plugin-coder/src/components/AccordionItem/AccordionItem.test.tsx b/plugins/backstage-plugin-coder/src/components/Disclosure/Disclosure.test.tsx similarity index 83% rename from plugins/backstage-plugin-coder/src/components/AccordionItem/AccordionItem.test.tsx rename to plugins/backstage-plugin-coder/src/components/Disclosure/Disclosure.test.tsx index e8dbc5d8..2a788ea6 100644 --- a/plugins/backstage-plugin-coder/src/components/AccordionItem/AccordionItem.test.tsx +++ b/plugins/backstage-plugin-coder/src/components/Disclosure/Disclosure.test.tsx @@ -3,19 +3,24 @@ import { screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { renderInCoderEnvironment } from '../../testHelpers/setup'; import { Root } from '../CoderWorkspacesCard/Root'; -import { AccordionItem } from './AccordionItem'; +import { Disclosure } from './Disclosure'; + +type RenderInputs = Readonly<{ + headerText: string; + children: string; +}>; function render() { return renderInCoderEnvironment({ children: ( - + ), }); } -describe(`${AccordionItem.name}`, () => { +describe(`${Disclosure.name}`, () => { it('Will toggle between showing/hiding the disclosure info when the user clicks it', async () => { await render(); const user = userEvent.setup(); diff --git a/plugins/backstage-plugin-coder/src/components/AccordionItem/AccordionItem.tsx b/plugins/backstage-plugin-coder/src/components/Disclosure/Disclosure.tsx similarity index 92% rename from plugins/backstage-plugin-coder/src/components/AccordionItem/AccordionItem.tsx rename to plugins/backstage-plugin-coder/src/components/Disclosure/Disclosure.tsx index f5906d59..193ecda6 100644 --- a/plugins/backstage-plugin-coder/src/components/AccordionItem/AccordionItem.tsx +++ b/plugins/backstage-plugin-coder/src/components/Disclosure/Disclosure.tsx @@ -34,7 +34,7 @@ const useStyles = makeStyles(theme => ({ }, })); -type AccordionItemProps = Readonly< +type Props = Readonly< PropsWithChildren<{ isExpanded: boolean; onExpansion: () => void; @@ -42,19 +42,19 @@ type AccordionItemProps = Readonly< }> >; -export const AccordionItem = ({ +export const Disclosure = ({ isExpanded, onExpansion, headerText, children, -}: AccordionItemProps) => { +}: Props) => { const styles = useStyles(); const hookId = useId(); const disclosureBodyId = `${hookId}-disclosure-body`; // Might be worth revisiting the markup here to try implementing this // functionality with and elements. Would likely clean up - // the component code a ton but might reduce control over screen reader output + // the component code a bit but might reduce control over screen reader output return (
- {isExpanded && ( + {activeIsExpanded && (

{children}

From badd8ee654ee94a45ca4b5a146c0ed5f15594e75 Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Tue, 2 Apr 2024 12:41:52 +0000 Subject: [PATCH 19/42] chore: remove needless hasAssertions calls --- .../src/components/CoderAuthWrapper/CoderAuthWrapper.test.tsx | 2 -- .../components/CoderErrorBoundary/CoderErrorBoundary.test.tsx | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/plugins/backstage-plugin-coder/src/components/CoderAuthWrapper/CoderAuthWrapper.test.tsx b/plugins/backstage-plugin-coder/src/components/CoderAuthWrapper/CoderAuthWrapper.test.tsx index bf27a634..43199c04 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderAuthWrapper/CoderAuthWrapper.test.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderAuthWrapper/CoderAuthWrapper.test.tsx @@ -166,8 +166,6 @@ describe(`${CoderAuthWrapper.name}`, () => { unmount(); } - - expect.hasAssertions(); }); it('Lets the user submit a new token', async () => { diff --git a/plugins/backstage-plugin-coder/src/components/CoderErrorBoundary/CoderErrorBoundary.test.tsx b/plugins/backstage-plugin-coder/src/components/CoderErrorBoundary/CoderErrorBoundary.test.tsx index 5245cc4c..734defb0 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderErrorBoundary/CoderErrorBoundary.test.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderErrorBoundary/CoderErrorBoundary.test.tsx @@ -47,8 +47,8 @@ function setupBoundaryTest(component: ReactElement) { describe(`${CoderErrorBoundary.name}`, () => { it('Displays a fallback UI when a rendering error is encountered', () => { setupBoundaryTest(); - screen.getByText(fallbackText); - expect.hasAssertions(); + const fallbackUi = screen.getByText(fallbackText); + expect(fallbackUi).toBeInTheDocument(); }); it('Exposes rendering errors to Backstage Error API', () => { From e87e41e716f41a179ae676eec5f222f00eeb0d2d Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Tue, 2 Apr 2024 13:11:36 +0000 Subject: [PATCH 20/42] fix: update conditional logic for ReminderAccordion --- .../ReminderAccordion.test.tsx | 90 ++++++++++++++++++- .../CoderWorkspacesCard/ReminderAccordion.tsx | 13 +-- 2 files changed, 93 insertions(+), 10 deletions(-) diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.test.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.test.tsx index 6fc4bc0d..00586512 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.test.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.test.tsx @@ -1,12 +1,94 @@ -import { ReminderAccordion } from './ReminderAccordion'; +import React from 'react'; +import { screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { renderInCoderEnvironment } from '../../testHelpers/setup'; +import { + type WorkspacesCardContext, + CardContext, + WorkspacesQuery, +} from './Root'; +import { + type ReminderAccordionProps, + ReminderAccordion, +} from './ReminderAccordion'; +import { Workspace } from '../../typesConstants'; +import { mockCoderWorkspacesConfig } from '../../testHelpers/mockBackstageData'; + +type RenderInputs = Readonly< + ReminderAccordionProps & { + isReadingEntityData?: boolean; + repoUrl?: undefined | string; + queryData?: undefined | readonly Workspace[]; + } +>; + +function renderAccordion(inputs?: RenderInputs) { + const { + queryData = [], + isReadingEntityData = true, + showEntityReminder = true, + showTemplateNameReminder = true, + repoUrl = mockCoderWorkspacesConfig.repoUrl, + } = inputs ?? {}; + + const mockContext: WorkspacesCardContext = { + isReadingEntityData, + headerId: 'blah', + onFilterChange: jest.fn(), + queryFilter: 'blah blah blah', + workspacesConfig: { + ...mockCoderWorkspacesConfig, + repoUrl, + }, + workspacesQuery: { + data: queryData, + } as WorkspacesQuery, + }; + + return renderInCoderEnvironment({ + children: ( + + + + ), + }); +} describe(`${ReminderAccordion.name}`, () => { it('Lets the user open a single accordion item', async () => { - expect.hasAssertions(); + await renderAccordion(); + const entityToggle = await screen.findByRole('button', { + name: /Why am I not seeing any workspaces\?/i, + }); + + const user = userEvent.setup(); + await user.click(entityToggle); + + const entityText = await screen.findByText( + /^This component only displays all workspaces when/, + ); + + expect(entityText).toBeInTheDocument(); }); - it('Will close an open accordion item when that item is clicked', async () => { - expect.hasAssertions(); + it.only('Will close an open accordion item when that item is clicked', async () => { + await renderAccordion(); + const entityToggle = await screen.findByRole('button', { + name: /Why am I not seeing any workspaces\?/i, + }); + + const user = userEvent.setup(); + await user.click(entityToggle); + + const entityText = await screen.findByText( + /^This component only displays all workspaces when/, + ); + + await user.click(entityToggle); + expect(entityText).not.toBeInTheDocument(); }); it('Will close any other open accordion items when a new item is clicked', async () => { diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.tsx index dfa138ac..44d75b0a 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.tsx @@ -31,7 +31,7 @@ const useStyles = makeStyles(theme => ({ }, })); -type Props = Readonly<{ +export type ReminderAccordionProps = Readonly<{ showEntityReminder?: boolean; showTemplateNameReminder?: boolean; }>; @@ -39,7 +39,7 @@ type Props = Readonly<{ export function ReminderAccordion({ showEntityReminder = true, showTemplateNameReminder = true, -}: Props) { +}: ReminderAccordionProps) { const [activeItemId, setActiveItemId] = useState(); const { isReadingEntityData, workspacesConfig, workspacesQuery } = useWorkspacesCardContext(); @@ -51,13 +51,13 @@ export function ReminderAccordion({ canDisplay: showEntityReminder && isReadingEntityData && - !workspacesConfig.repoUrl && + Boolean(workspacesConfig.repoUrl) && workspacesQuery.data !== undefined, headerText: 'Why am I not seeing any workspaces?', bodyText: ( <> - This component displays only displays all workspaces when the value of - the readEntityData prop is{' '} + This component only displays all workspaces when the value of the{' '} + readEntityData prop is{' '} false. See{' '}
Why can't I make a new workspace?, bodyText: ( <> From 52a273148f97a8fe656f95965425e4151e349f5e Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Tue, 2 Apr 2024 13:37:18 +0000 Subject: [PATCH 21/42] fix: more accordion bug fixes --- .../ReminderAccordion.test.tsx | 103 +++++++++++++----- .../CoderWorkspacesCard/ReminderAccordion.tsx | 5 +- 2 files changed, 76 insertions(+), 32 deletions(-) diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.test.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.test.tsx index 00586512..73a5fb78 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.test.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.test.tsx @@ -18,17 +18,19 @@ type RenderInputs = Readonly< ReminderAccordionProps & { isReadingEntityData?: boolean; repoUrl?: undefined | string; + creationUrl?: undefined | string; queryData?: undefined | readonly Workspace[]; } >; function renderAccordion(inputs?: RenderInputs) { const { + repoUrl, + creationUrl, queryData = [], isReadingEntityData = true, showEntityReminder = true, showTemplateNameReminder = true, - repoUrl = mockCoderWorkspacesConfig.repoUrl, } = inputs ?? {}; const mockContext: WorkspacesCardContext = { @@ -39,6 +41,7 @@ function renderAccordion(inputs?: RenderInputs) { workspacesConfig: { ...mockCoderWorkspacesConfig, repoUrl, + creationUrl, }, workspacesQuery: { data: queryData, @@ -57,45 +60,87 @@ function renderAccordion(inputs?: RenderInputs) { }); } +const matchers = { + toggles: { + entity: /Why am I not seeing any workspaces\?/i, + templateName: /Why can't I make a new workspace\?/, + }, + bodyText: { + entity: /^This component only displays all workspaces when/, + templateName: + /^This component cannot make a new workspace without a template name value/, + }, +} as const satisfies Record>; + describe(`${ReminderAccordion.name}`, () => { - it('Lets the user open a single accordion item', async () => { - await renderAccordion(); - const entityToggle = await screen.findByRole('button', { - name: /Why am I not seeing any workspaces\?/i, - }); + describe('General behavior', () => { + it('Lets the user open a single accordion item', async () => { + await renderAccordion({ repoUrl: undefined }); + const entityToggle = await screen.findByRole('button', { + name: matchers.toggles.entity, + }); - const user = userEvent.setup(); - await user.click(entityToggle); + const user = userEvent.setup(); + await user.click(entityToggle); - const entityText = await screen.findByText( - /^This component only displays all workspaces when/, - ); + const entityText = await screen.findByText(matchers.bodyText.entity); + expect(entityText).toBeInTheDocument(); + }); - expect(entityText).toBeInTheDocument(); - }); + it('Will close an open accordion item when that item is clicked', async () => { + await renderAccordion({ repoUrl: undefined }); + const entityToggle = await screen.findByRole('button', { + name: matchers.toggles.entity, + }); - it.only('Will close an open accordion item when that item is clicked', async () => { - await renderAccordion(); - const entityToggle = await screen.findByRole('button', { - name: /Why am I not seeing any workspaces\?/i, + const user = userEvent.setup(); + await user.click(entityToggle); + + const entityText = await screen.findByText(matchers.bodyText.entity); + await user.click(entityToggle); + expect(entityText).not.toBeInTheDocument(); }); - const user = userEvent.setup(); - await user.click(entityToggle); + it('Only lets one accordion item be open at a time', async () => { + await renderAccordion({ + repoUrl: undefined, + creationUrl: undefined, + }); - const entityText = await screen.findByText( - /^This component only displays all workspaces when/, - ); + const entityToggle = await screen.findByRole('button', { + name: matchers.toggles.entity, + }); + const templateNameToggle = await screen.findByRole('button', { + name: matchers.toggles.templateName, + }); - await user.click(entityToggle); - expect(entityText).not.toBeInTheDocument(); - }); + const user = userEvent.setup(); + await user.click(entityToggle); + + const entityText = await screen.findByText(matchers.bodyText.entity); + expect(entityText).toBeInTheDocument(); - it('Will close any other open accordion items when a new item is clicked', async () => { - expect.hasAssertions(); + await user.click(templateNameToggle); + expect(entityText).not.toBeInTheDocument(); + + const templateText = await screen.findByText( + matchers.bodyText.templateName, + ); + expect(templateText).toBeInTheDocument(); + }); }); - it('Lets the user conditionally hide accordion items based on props', async () => { - expect.hasAssertions(); + describe('Conditionally displaying items', () => { + it('Lets the user conditionally hide accordion items based on props', async () => { + expect.hasAssertions(); + }); + + it('Will only display the entity data reminder when appropriate', async () => { + expect.hasAssertions(); + }); + + it('Will only display the template name data reminder when appropriate', async () => { + expect.hasAssertions(); + }); }); }); diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.tsx index 44d75b0a..fd5cfeab 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.tsx @@ -51,7 +51,7 @@ export function ReminderAccordion({ canDisplay: showEntityReminder && isReadingEntityData && - Boolean(workspacesConfig.repoUrl) && + !workspacesConfig.repoUrl && workspacesQuery.data !== undefined, headerText: 'Why am I not seeing any workspaces?', bodyText: ( @@ -74,8 +74,7 @@ export function ReminderAccordion({ }, { id: 'templateName', - canDisplay: - showTemplateNameReminder && Boolean(workspacesConfig.creationUrl), + canDisplay: showTemplateNameReminder && !workspacesConfig.creationUrl, headerText: <>Why can't I make a new workspace?, bodyText: ( <> From c45e132c0d985ad08d9b759e5179b8dadc8fa198 Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Tue, 2 Apr 2024 13:45:13 +0000 Subject: [PATCH 22/42] chore: finish another test case --- .../ReminderAccordion.test.tsx | 40 ++++++++++++++----- 1 file changed, 31 insertions(+), 9 deletions(-) diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.test.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.test.tsx index 73a5fb78..90f85e67 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.test.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.test.tsx @@ -75,7 +75,7 @@ const matchers = { describe(`${ReminderAccordion.name}`, () => { describe('General behavior', () => { it('Lets the user open a single accordion item', async () => { - await renderAccordion({ repoUrl: undefined }); + await renderAccordion(); const entityToggle = await screen.findByRole('button', { name: matchers.toggles.entity, }); @@ -88,7 +88,7 @@ describe(`${ReminderAccordion.name}`, () => { }); it('Will close an open accordion item when that item is clicked', async () => { - await renderAccordion({ repoUrl: undefined }); + await renderAccordion(); const entityToggle = await screen.findByRole('button', { name: matchers.toggles.entity, }); @@ -102,11 +102,7 @@ describe(`${ReminderAccordion.name}`, () => { }); it('Only lets one accordion item be open at a time', async () => { - await renderAccordion({ - repoUrl: undefined, - creationUrl: undefined, - }); - + await renderAccordion(); const entityToggle = await screen.findByRole('button', { name: matchers.toggles.entity, }); @@ -132,10 +128,36 @@ describe(`${ReminderAccordion.name}`, () => { describe('Conditionally displaying items', () => { it('Lets the user conditionally hide accordion items based on props', async () => { - expect.hasAssertions(); + type Configuration = Readonly<{ + props: ReminderAccordionProps; + expectedItemCount: number; + }>; + + const configurations: readonly Configuration[] = [ + { + expectedItemCount: 0, + props: { showEntityReminder: false, showTemplateNameReminder: false }, + }, + { + expectedItemCount: 1, + props: { showEntityReminder: false, showTemplateNameReminder: true }, + }, + { + expectedItemCount: 1, + props: { showEntityReminder: true, showTemplateNameReminder: false }, + }, + ]; + + for (const config of configurations) { + const { unmount } = await renderAccordion(config.props); + const accordionItems = screen.queryAllByRole('button'); + + expect(accordionItems.length).toBe(config.expectedItemCount); + unmount(); + } }); - it('Will only display the entity data reminder when appropriate', async () => { + it.only('Will only display the entity data reminder when appropriate', async () => { expect.hasAssertions(); }); From 0680934d18348ff831779d148a585d62faa2e231 Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Tue, 2 Apr 2024 13:51:15 +0000 Subject: [PATCH 23/42] chore: add another accordion test case --- .../ReminderAccordion.test.tsx | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.test.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.test.tsx index 90f85e67..08db8191 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.test.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.test.tsx @@ -157,11 +157,21 @@ describe(`${ReminderAccordion.name}`, () => { } }); - it.only('Will only display the entity data reminder when appropriate', async () => { - expect.hasAssertions(); + it.only('Will NOT display the template name reminder if there is a creation URL', async () => { + await renderAccordion({ + creationUrl: mockCoderWorkspacesConfig.creationUrl, + showTemplateNameReminder: true, + }); + + const templateToggle = screen.queryByRole('button', { + name: matchers.toggles.templateName, + }); + + console.log('Be sure to rename the show props to canShow!'); + expect(templateToggle).not.toBeInTheDocument(); }); - it('Will only display the template name data reminder when appropriate', async () => { + it('Will only display the entity data reminder when appropriate', async () => { expect.hasAssertions(); }); }); From 7ba9fcedb97f6d1b160272a9ea0dfa07584a6fde Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Tue, 2 Apr 2024 13:53:10 +0000 Subject: [PATCH 24/42] refactor: rename props for clarity --- .../ReminderAccordion.test.tsx | 30 ++++++++++++------- .../CoderWorkspacesCard/ReminderAccordion.tsx | 12 ++++---- 2 files changed, 25 insertions(+), 17 deletions(-) diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.test.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.test.tsx index 08db8191..f47914bc 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.test.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.test.tsx @@ -29,8 +29,8 @@ function renderAccordion(inputs?: RenderInputs) { creationUrl, queryData = [], isReadingEntityData = true, - showEntityReminder = true, - showTemplateNameReminder = true, + canShowEntityReminder = true, + canShowTemplateNameReminder = true, } = inputs ?? {}; const mockContext: WorkspacesCardContext = { @@ -52,8 +52,8 @@ function renderAccordion(inputs?: RenderInputs) { children: ( ), @@ -136,15 +136,24 @@ describe(`${ReminderAccordion.name}`, () => { const configurations: readonly Configuration[] = [ { expectedItemCount: 0, - props: { showEntityReminder: false, showTemplateNameReminder: false }, + props: { + canShowEntityReminder: false, + canShowTemplateNameReminder: false, + }, }, { expectedItemCount: 1, - props: { showEntityReminder: false, showTemplateNameReminder: true }, + props: { + canShowEntityReminder: false, + canShowTemplateNameReminder: true, + }, }, { expectedItemCount: 1, - props: { showEntityReminder: true, showTemplateNameReminder: false }, + props: { + canShowEntityReminder: true, + canShowTemplateNameReminder: false, + }, }, ]; @@ -157,21 +166,20 @@ describe(`${ReminderAccordion.name}`, () => { } }); - it.only('Will NOT display the template name reminder if there is a creation URL', async () => { + it('Will NOT display the template name reminder if there is a creation URL', async () => { await renderAccordion({ creationUrl: mockCoderWorkspacesConfig.creationUrl, - showTemplateNameReminder: true, + canShowTemplateNameReminder: true, }); const templateToggle = screen.queryByRole('button', { name: matchers.toggles.templateName, }); - console.log('Be sure to rename the show props to canShow!'); expect(templateToggle).not.toBeInTheDocument(); }); - it('Will only display the entity data reminder when appropriate', async () => { + it.only('Will only display the entity data reminder when appropriate', async () => { expect.hasAssertions(); }); }); diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.tsx index fd5cfeab..bde4c4a3 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.tsx @@ -32,13 +32,13 @@ const useStyles = makeStyles(theme => ({ })); export type ReminderAccordionProps = Readonly<{ - showEntityReminder?: boolean; - showTemplateNameReminder?: boolean; + canShowEntityReminder?: boolean; + canShowTemplateNameReminder?: boolean; }>; export function ReminderAccordion({ - showEntityReminder = true, - showTemplateNameReminder = true, + canShowEntityReminder = true, + canShowTemplateNameReminder = true, }: ReminderAccordionProps) { const [activeItemId, setActiveItemId] = useState(); const { isReadingEntityData, workspacesConfig, workspacesQuery } = @@ -49,7 +49,7 @@ export function ReminderAccordion({ { id: 'entity', canDisplay: - showEntityReminder && + canShowEntityReminder && isReadingEntityData && !workspacesConfig.repoUrl && workspacesQuery.data !== undefined, @@ -74,7 +74,7 @@ export function ReminderAccordion({ }, { id: 'templateName', - canDisplay: showTemplateNameReminder && !workspacesConfig.creationUrl, + canDisplay: canShowTemplateNameReminder && !workspacesConfig.creationUrl, headerText: <>Why can't I make a new workspace?, bodyText: ( <> From 14c4d860e4b1ea78c86288161f720005ec7c1c82 Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Tue, 2 Apr 2024 13:56:53 +0000 Subject: [PATCH 25/42] refactor: simplify condition for entity reminder --- .../CoderWorkspacesCard/ReminderAccordion.test.tsx | 6 ++++++ .../components/CoderWorkspacesCard/ReminderAccordion.tsx | 3 +-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.test.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.test.tsx index f47914bc..cb83dda4 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.test.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.test.tsx @@ -179,6 +179,12 @@ describe(`${ReminderAccordion.name}`, () => { expect(templateToggle).not.toBeInTheDocument(); }); + /** + * Assuming that the user hasn't disabled showing the reminder at all, it + * will only appear when: + * 1. The component is set up to read entity data + * 2. There is no repo URL that could be parsed from the entity data + */ it.only('Will only display the entity data reminder when appropriate', async () => { expect.hasAssertions(); }); diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.tsx index bde4c4a3..bb49268c 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.tsx @@ -51,8 +51,7 @@ export function ReminderAccordion({ canDisplay: canShowEntityReminder && isReadingEntityData && - !workspacesConfig.repoUrl && - workspacesQuery.data !== undefined, + !workspacesConfig.repoUrl, headerText: 'Why am I not seeing any workspaces?', bodyText: ( <> From 8f5081e0cb54babd949dd8de3c92e868f7dd00da Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Tue, 2 Apr 2024 14:56:38 +0000 Subject: [PATCH 26/42] refactor: update prop for Disclosure --- .../components/Disclosure/Disclosure.test.tsx | 39 +++++++++---------- .../src/components/Disclosure/Disclosure.tsx | 6 +-- 2 files changed, 21 insertions(+), 24 deletions(-) diff --git a/plugins/backstage-plugin-coder/src/components/Disclosure/Disclosure.test.tsx b/plugins/backstage-plugin-coder/src/components/Disclosure/Disclosure.test.tsx index d49706bc..09894e48 100644 --- a/plugins/backstage-plugin-coder/src/components/Disclosure/Disclosure.test.tsx +++ b/plugins/backstage-plugin-coder/src/components/Disclosure/Disclosure.test.tsx @@ -1,32 +1,29 @@ import React from 'react'; -import { screen } from '@testing-library/react'; +import { render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; -import { renderInCoderEnvironment } from '../../testHelpers/setup'; import { type DisclosureProps, Disclosure } from './Disclosure'; type RenderInputs = Partial; -function render(inputs?: RenderInputs) { - const { headerText, children, isExpanded, onExpansion } = inputs ?? {}; +function renderDisclosure(inputs?: RenderInputs) { + const { headerText, children, isExpanded, onExpansionToggle } = inputs ?? {}; - return renderInCoderEnvironment({ - children: ( - - {children} - - ), - }); + return render( + + {children} + , + ); } describe(`${Disclosure.name}`, () => { it('Will toggle between showing/hiding the disclosure info when the user clicks it', async () => { const headerText = 'Blah'; const children = 'Blah blah blah blah'; - await render({ headerText, children }); + renderDisclosure({ headerText, children }); const user = userEvent.setup(); const disclosureButton = screen.getByRole('button', { name: headerText }); @@ -40,10 +37,10 @@ describe(`${Disclosure.name}`, () => { it('Can flip from an uncontrolled input to a controlled one if additional props are passed in', async () => { const headerText = 'Blah'; const children = 'Blah blah blah blah'; - const onExpansion = jest.fn(); + const onExpansionToggle = jest.fn(); - const { rerender } = await render({ - onExpansion, + const { rerender } = renderDisclosure({ + onExpansionToggle, headerText, children, isExpanded: true, @@ -54,12 +51,12 @@ describe(`${Disclosure.name}`, () => { const disclosureButton = screen.getByRole('button', { name: headerText }); await user.click(disclosureButton); - expect(onExpansion).toHaveBeenCalled(); + expect(onExpansionToggle).toHaveBeenCalled(); rerender( {children} diff --git a/plugins/backstage-plugin-coder/src/components/Disclosure/Disclosure.tsx b/plugins/backstage-plugin-coder/src/components/Disclosure/Disclosure.tsx index 23fb17dd..686f7656 100644 --- a/plugins/backstage-plugin-coder/src/components/Disclosure/Disclosure.tsx +++ b/plugins/backstage-plugin-coder/src/components/Disclosure/Disclosure.tsx @@ -37,14 +37,14 @@ const useStyles = makeStyles(theme => ({ export type DisclosureProps = Readonly< PropsWithChildren<{ isExpanded?: boolean; - onExpansion?: () => void; + onExpansionToggle?: () => void; headerText: ReactNode; }> >; export const Disclosure = ({ isExpanded, - onExpansion, + onExpansionToggle, headerText, children, }: DisclosureProps) => { @@ -69,7 +69,7 @@ export const Disclosure = ({ className={styles.button} onClick={() => { setInternalIsExpanded(!internalIsExpanded); - onExpansion?.(); + onExpansionToggle?.(); }} > From 11b7b81acc3c61e14e5c9309d76484eba3836626 Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Tue, 2 Apr 2024 15:26:52 +0000 Subject: [PATCH 27/42] chore: finish all tests for accordion --- .../ReminderAccordion.test.tsx | 50 +++++++++++++++++-- .../CoderWorkspacesCard/ReminderAccordion.tsx | 2 +- 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.test.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.test.tsx index cb83dda4..ee2fddad 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.test.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.test.tsx @@ -181,12 +181,56 @@ describe(`${ReminderAccordion.name}`, () => { /** * Assuming that the user hasn't disabled showing the reminder at all, it - * will only appear when: + * will only appear when both of these are true: * 1. The component is set up to read entity data * 2. There is no repo URL that could be parsed from the entity data */ - it.only('Will only display the entity data reminder when appropriate', async () => { - expect.hasAssertions(); + it('Will only display the entity data reminder when appropriate', async () => { + type Config = Readonly<{ + isReadingEntityData: boolean; + repoUrl: string | undefined; + }>; + + const doNotDisplayConfigs: readonly Config[] = [ + { + isReadingEntityData: false, + repoUrl: mockCoderWorkspacesConfig.repoUrl, + }, + { + isReadingEntityData: false, + repoUrl: undefined, + }, + { + isReadingEntityData: true, + repoUrl: mockCoderWorkspacesConfig.repoUrl, + }, + ]; + + for (const config of doNotDisplayConfigs) { + const { unmount } = await renderAccordion({ + isReadingEntityData: config.isReadingEntityData, + repoUrl: config.repoUrl, + }); + + const entityToggle = screen.queryByRole('button', { + name: matchers.toggles.entity, + }); + + expect(entityToggle).not.toBeInTheDocument(); + unmount(); + } + + // Verify that toggle appears only this one time + await renderAccordion({ + isReadingEntityData: true, + repoUrl: undefined, + }); + + const entityToggle = await screen.findByRole('button', { + name: matchers.toggles.entity, + }); + + expect(entityToggle).toBeInTheDocument(); }); }); }); diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.tsx index bb49268c..bd2dc441 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.tsx @@ -105,7 +105,7 @@ export function ReminderAccordion({ toggleAccordionGroup(id)} + onExpansionToggle={() => toggleAccordionGroup(id)} > {bodyText} From cb2664e57c4dfb764c8844f0ca229ce05c9ccce9 Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Tue, 2 Apr 2024 15:36:41 +0000 Subject: [PATCH 28/42] refactor: update type definition for mock config --- .../src/testHelpers/mockBackstageData.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/backstage-plugin-coder/src/testHelpers/mockBackstageData.ts b/plugins/backstage-plugin-coder/src/testHelpers/mockBackstageData.ts index 875d6ce5..4235bfd8 100644 --- a/plugins/backstage-plugin-coder/src/testHelpers/mockBackstageData.ts +++ b/plugins/backstage-plugin-coder/src/testHelpers/mockBackstageData.ts @@ -99,7 +99,7 @@ export const mockAppConfig = { }, } as const satisfies CoderAppConfig; -export const mockCoderWorkspacesConfig: CoderWorkspacesConfig = (() => { +export const mockCoderWorkspacesConfig = (() => { const urlParams = new URLSearchParams({ mode: mockYamlConfig.mode, 'param.repo': mockAppConfig.workspaces.params.repo, @@ -124,7 +124,7 @@ export const mockCoderWorkspacesConfig: CoderWorkspacesConfig = (() => { custom_repo: cleanedRepoUrl, repo_url: cleanedRepoUrl, }, - }; + } as const satisfies CoderWorkspacesConfig; })(); const authedState = { From 41d6f20ab3729d6e969d610a25c9468b8421f83a Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Tue, 2 Apr 2024 15:37:02 +0000 Subject: [PATCH 29/42] refactor: polish up accordion tests --- .../CoderWorkspacesCard/ReminderAccordion.test.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.test.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.test.tsx index ee2fddad..5f6cf59f 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.test.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.test.tsx @@ -2,17 +2,17 @@ import React from 'react'; import { screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { renderInCoderEnvironment } from '../../testHelpers/setup'; +import type { Workspace } from '../../typesConstants'; +import { mockCoderWorkspacesConfig } from '../../testHelpers/mockBackstageData'; import { type WorkspacesCardContext, + type WorkspacesQuery, CardContext, - WorkspacesQuery, } from './Root'; import { type ReminderAccordionProps, ReminderAccordion, } from './ReminderAccordion'; -import { Workspace } from '../../typesConstants'; -import { mockCoderWorkspacesConfig } from '../../testHelpers/mockBackstageData'; type RenderInputs = Readonly< ReminderAccordionProps & { From b3b83f05c872474c6b85812c6813bde737847ebb Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Tue, 2 Apr 2024 15:45:08 +0000 Subject: [PATCH 30/42] chore: finish up all tests --- .../WorkspacesList.test.tsx | 30 +++++++++++++++---- .../CoderWorkspacesCard/WorkspacesList.tsx | 4 +-- 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/WorkspacesList.test.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/WorkspacesList.test.tsx index 92ab025b..abc298fc 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/WorkspacesList.test.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/WorkspacesList.test.tsx @@ -10,18 +10,22 @@ import { screen } from '@testing-library/react'; type RenderInputs = Readonly<{ workspacesQuery: Partial; renderListItem?: WorkspacesListProps['renderListItem']; + repoUrl?: string; }>; function renderWorkspacesList(inputs?: RenderInputs) { - const { renderListItem, workspacesQuery } = inputs ?? {}; + const { renderListItem, workspacesQuery, repoUrl } = inputs ?? {}; const mockContext: WorkspacesCardContext = { isReadingEntityData: true, headerId: "Doesn't matter", queryFilter: "Also doesn't matter", onFilterChange: jest.fn(), - workspacesConfig: mockCoderWorkspacesConfig, workspacesQuery: workspacesQuery as WorkspacesQuery, + workspacesConfig: { + ...mockCoderWorkspacesConfig, + repoUrl, + }, }; return renderInCoderEnvironment({ @@ -39,8 +43,8 @@ function renderWorkspacesList(inputs?: RenderInputs) { describe(`${WorkspacesList.name}`, () => { it('Allows the user to provide their own callback for iterating through each item', async () => { const workspaceNames = ['dog', 'cat', 'bird']; - await renderWorkspacesList({ + repoUrl: mockCoderWorkspacesConfig.repoUrl, workspacesQuery: { data: workspaceNames.map((name, index) => ({ ...mockWorkspaceWithMatch, @@ -65,7 +69,23 @@ describe(`${WorkspacesList.name}`, () => { } }); - it('Does not display the call-to-action button for making new workspaces when there is no workspace creation URL', async () => { - expect.hasAssertions(); + it('Displays the call-to-action link for making new workspaces when nothing is loading, but there is no data', async () => { + await renderWorkspacesList({ + repoUrl: mockCoderWorkspacesConfig.repoUrl, + workspacesQuery: { data: [] }, + }); + + const ctaLink = screen.getByRole('link', { name: /Create workspace/ }); + expect(ctaLink).toBeInTheDocument(); + }); + + it('Does NOT display the call-to-action link for making new workspaces when there is no workspace creation URL', async () => { + await renderWorkspacesList({ + repoUrl: undefined, + workspacesQuery: { data: [] }, + }); + + const ctaLink = screen.queryByRole('link', { name: /Create workspace/ }); + expect(ctaLink).not.toBeInTheDocument(); }); }); diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/WorkspacesList.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/WorkspacesList.tsx index a9293dab..1e47b08a 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/WorkspacesList.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/WorkspacesList.tsx @@ -99,10 +99,10 @@ export const WorkspacesList = ({ {emptyState ?? ( {repoUrl ? ( -
+ No workspaces found for repo {repoUrl} -
+
) : ( <>No workspaces returned for your query )} From b6c8dec0844c78bebcfdc339adaaac9c12906338 Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Tue, 2 Apr 2024 15:49:51 +0000 Subject: [PATCH 31/42] fix: add missing property to mock setup to help compiler pass --- .../src/components/CoderWorkspacesCard/SearchBox.test.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/SearchBox.test.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/SearchBox.test.tsx index ecb31bb7..368965e8 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/SearchBox.test.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/SearchBox.test.tsx @@ -31,6 +31,7 @@ async function renderSearchBox(input?: RenderInputs) { const mockContext: WorkspacesCardContext = { onFilterChange, queryFilter, + isReadingEntityData: true, headerId: "Doesn't matter", workspacesConfig: mockCoderWorkspacesConfig, workspacesQuery: From ff531b8766d51b93d27e0ba57c52265d0c23a97c Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Tue, 2 Apr 2024 18:41:06 +0000 Subject: [PATCH 32/42] refactor: move isReadingEntityData property to workspaces config --- .../CreateWorkspaceLink.test.tsx | 12 ++---------- .../ExtraActionsButton.test.tsx | 15 +++++---------- .../ReminderAccordion.test.tsx | 9 +++------ .../CoderWorkspacesCard/ReminderAccordion.tsx | 5 ++--- .../src/components/CoderWorkspacesCard/Root.tsx | 5 ----- .../CoderWorkspacesCard/SearchBox.test.tsx | 10 ++-------- .../CoderWorkspacesCard/WorkspacesList.test.tsx | 9 ++------- .../src/hooks/useCoderWorkspacesConfig.test.ts | 1 + .../src/hooks/useCoderWorkspacesConfig.ts | 2 ++ .../src/testHelpers/mockBackstageData.ts | 1 + 10 files changed, 20 insertions(+), 49 deletions(-) diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/CreateWorkspaceLink.test.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/CreateWorkspaceLink.test.tsx index 23e1659d..1ae49623 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/CreateWorkspaceLink.test.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/CreateWorkspaceLink.test.tsx @@ -24,21 +24,13 @@ function render(inputs?: RenderInputs) { : undefined, }; - const mockContextValue: WorkspacesCardContext = { + const mockContextValue: Partial = { workspacesConfig: mockWorkspacesConfig, - - // Everything below this comment doesn't matter for the test logic - isReadingEntityData: false, - headerId: "Doesn't matter", - queryFilter: "Also doesn't matter", - onFilterChange: jest.fn(), - workspacesQuery: - null as unknown as WorkspacesCardContext['workspacesQuery'], }; return renderInCoderEnvironment({ children: ( - + ), diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ExtraActionsButton.test.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ExtraActionsButton.test.tsx index 96ded1d6..008d931a 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ExtraActionsButton.test.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ExtraActionsButton.test.tsx @@ -39,22 +39,17 @@ async function renderButton({ buttonText }: RenderInputs) { * @todo Research how to test dependencies on useQuery */ const refetch = jest.fn(); - const mockWorkspacesQuery = { - refetch, - } as unknown as WorkspacesCardContext['workspacesQuery']; - const mockContext: WorkspacesCardContext = { - isReadingEntityData: true, - headerId: "Doesn't matter", - queryFilter: "Doesn't matter", - onFilterChange: jest.fn(), + const mockContext: Partial = { workspacesConfig: mockCoderWorkspacesConfig, - workspacesQuery: mockWorkspacesQuery, + workspacesQuery: { + refetch, + } as unknown as WorkspacesCardContext['workspacesQuery'], }; const renderOutput = await renderInCoderEnvironment({ auth, children: ( - + ), diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.test.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.test.tsx index 5f6cf59f..0ae1d918 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.test.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.test.tsx @@ -33,15 +33,12 @@ function renderAccordion(inputs?: RenderInputs) { canShowTemplateNameReminder = true, } = inputs ?? {}; - const mockContext: WorkspacesCardContext = { - isReadingEntityData, - headerId: 'blah', - onFilterChange: jest.fn(), - queryFilter: 'blah blah blah', + const mockContext: Partial = { workspacesConfig: { ...mockCoderWorkspacesConfig, repoUrl, creationUrl, + isReadingEntityData, }, workspacesQuery: { data: queryData, @@ -50,7 +47,7 @@ function renderAccordion(inputs?: RenderInputs) { return renderInCoderEnvironment({ children: ( - + (); - const { isReadingEntityData, workspacesConfig, workspacesQuery } = - useWorkspacesCardContext(); + const { workspacesConfig, workspacesQuery } = useWorkspacesCardContext(); const styles = useStyles({ hasData: workspacesQuery.data !== undefined }); const accordionData: readonly AccordionItemInfo[] = [ @@ -50,7 +49,7 @@ export function ReminderAccordion({ id: 'entity', canDisplay: canShowEntityReminder && - isReadingEntityData && + workspacesConfig.isReadingEntityData && !workspacesConfig.repoUrl, headerText: 'Why am I not seeing any workspaces?', bodyText: ( diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/Root.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/Root.tsx index 70a64df3..25845c7f 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/Root.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/Root.tsx @@ -1,9 +1,6 @@ /** * @file Wires up all the core logic for passing values down to the * sub-components in the same directory. - * - * Does not need any tests – test functionality covered by integration tests in - * CoderWorkspacesCard */ import React, { type HTMLAttributes, @@ -26,7 +23,6 @@ import { CoderAuthWrapper } from '../CoderAuthWrapper'; export type WorkspacesQuery = UseQueryResult; export type WorkspacesCardContext = Readonly<{ - isReadingEntityData: boolean; queryFilter: string; onFilterChange: (newFilter: string) => void; workspacesQuery: WorkspacesQuery; @@ -75,7 +71,6 @@ export const Root = ({ headerId, workspacesQuery, workspacesConfig, - isReadingEntityData: readEntityData, queryFilter: activeFilter, onFilterChange: newFilter => { setInnerFilter(newFilter); diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/SearchBox.test.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/SearchBox.test.tsx index 368965e8..a0894946 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/SearchBox.test.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/SearchBox.test.tsx @@ -2,7 +2,6 @@ import React from 'react'; import { renderInCoderEnvironment } from '../../testHelpers/setup'; import { CardContext, WorkspacesCardContext } from './Root'; import { SearchBox } from './SearchBox'; -import { mockCoderWorkspacesConfig } from '../../testHelpers/mockBackstageData'; import { screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; @@ -28,19 +27,14 @@ async function renderSearchBox(input?: RenderInputs) { const { queryFilter = 'owner:me' } = input ?? {}; const onFilterChange = jest.fn(); - const mockContext: WorkspacesCardContext = { + const mockContext: Partial = { onFilterChange, queryFilter, - isReadingEntityData: true, - headerId: "Doesn't matter", - workspacesConfig: mockCoderWorkspacesConfig, - workspacesQuery: - null as unknown as WorkspacesCardContext['workspacesQuery'], }; const renderOutput = await renderInCoderEnvironment({ children: ( - + ), diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/WorkspacesList.test.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/WorkspacesList.test.tsx index abc298fc..50bc1de1 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/WorkspacesList.test.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/WorkspacesList.test.tsx @@ -15,12 +15,7 @@ type RenderInputs = Readonly<{ function renderWorkspacesList(inputs?: RenderInputs) { const { renderListItem, workspacesQuery, repoUrl } = inputs ?? {}; - - const mockContext: WorkspacesCardContext = { - isReadingEntityData: true, - headerId: "Doesn't matter", - queryFilter: "Also doesn't matter", - onFilterChange: jest.fn(), + const mockContext: Partial = { workspacesQuery: workspacesQuery as WorkspacesQuery, workspacesConfig: { ...mockCoderWorkspacesConfig, @@ -30,7 +25,7 @@ function renderWorkspacesList(inputs?: RenderInputs) { return renderInCoderEnvironment({ children: ( - + ), diff --git a/plugins/backstage-plugin-coder/src/hooks/useCoderWorkspacesConfig.test.ts b/plugins/backstage-plugin-coder/src/hooks/useCoderWorkspacesConfig.test.ts index 8e189225..bfd079b5 100644 --- a/plugins/backstage-plugin-coder/src/hooks/useCoderWorkspacesConfig.test.ts +++ b/plugins/backstage-plugin-coder/src/hooks/useCoderWorkspacesConfig.test.ts @@ -111,6 +111,7 @@ describe(`${useCoderWorkspacesConfig.name}`, () => { ); expect(result.current).toEqual({ + isReadingEntityData: true, mode: mockYamlConfig.mode, repoUrl: cleanedRepoUrl, creationUrl: mockCoderWorkspacesConfig.creationUrl, diff --git a/plugins/backstage-plugin-coder/src/hooks/useCoderWorkspacesConfig.ts b/plugins/backstage-plugin-coder/src/hooks/useCoderWorkspacesConfig.ts index e084ab9e..67bbb556 100644 --- a/plugins/backstage-plugin-coder/src/hooks/useCoderWorkspacesConfig.ts +++ b/plugins/backstage-plugin-coder/src/hooks/useCoderWorkspacesConfig.ts @@ -67,6 +67,7 @@ export type CoderWorkspacesConfig = // Was originally defined in terms of fancy mapped types based on YamlConfig; // ended up being a bad idea, because it increased coupling in a bad way Readonly<{ + isReadingEntityData: boolean; creationUrl?: string; templateName?: string; repoUrlParamKeys: readonly string[]; @@ -135,6 +136,7 @@ export function compileCoderConfig( creationUrl, templateName, repoUrl: cleanedRepoUrl, + isReadingEntityData: yamlConfig !== undefined, repoUrlParamKeys: workspaces.repoUrlParamKeys, params: compiledParams, }; diff --git a/plugins/backstage-plugin-coder/src/testHelpers/mockBackstageData.ts b/plugins/backstage-plugin-coder/src/testHelpers/mockBackstageData.ts index 4235bfd8..049050cc 100644 --- a/plugins/backstage-plugin-coder/src/testHelpers/mockBackstageData.ts +++ b/plugins/backstage-plugin-coder/src/testHelpers/mockBackstageData.ts @@ -110,6 +110,7 @@ export const mockCoderWorkspacesConfig = (() => { return { mode: 'auto', + isReadingEntityData: true, templateName: mockYamlConfig.templateName, repoUrlParamKeys: ['custom_repo', 'repo_url'], repoUrl: cleanedRepoUrl, From 352352fede4e65f26705d0a9dd3a5d80018db545 Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Tue, 2 Apr 2024 19:16:15 +0000 Subject: [PATCH 33/42] fix: add overflow-y and max height behavior to accordion --- .../CoderWorkspacesCard/ReminderAccordion.tsx | 40 ++++++++++++------- 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.tsx index ade746a8..2fa82f66 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.tsx @@ -12,7 +12,7 @@ type AccordionItemInfo = Readonly<{ bodyText: ReactNode; }>; -type StyleKeys = 'root' | 'link'; +type StyleKeys = 'root' | 'link' | 'innerPadding'; type StyleInputs = Readonly<{ hasData: boolean; }>; @@ -20,9 +20,19 @@ type StyleInputs = Readonly<{ const useStyles = makeStyles(theme => ({ root: ({ hasData }) => ({ paddingTop: theme.spacing(1), + marginLeft: `-${theme.spacing(2)}px`, + marginRight: `-${theme.spacing(2)}px`, borderTop: hasData ? 'none' : `1px solid ${theme.palette.divider}`, + maxHeight: '240px', + overflowX: 'hidden', + overflowY: 'auto', }), + innerPadding: { + paddingLeft: theme.spacing(2), + paddingRight: theme.spacing(2), + }, + link: { color: theme.palette.link, '&:hover': { @@ -98,19 +108,21 @@ export function ReminderAccordion({ return (
- {accordionData.map(({ id, canDisplay, headerText, bodyText }) => ( - - {canDisplay && ( - toggleAccordionGroup(id)} - > - {bodyText} - - )} - - ))} +
+ {accordionData.map(({ id, canDisplay, headerText, bodyText }) => ( + + {canDisplay && ( + toggleAccordionGroup(id)} + > + {bodyText} + + )} + + ))} +
); } From 53ba23e75569474506bacfc27392ee60ff1d58c5 Mon Sep 17 00:00:00 2001 From: Parkreiner Date: Tue, 2 Apr 2024 19:29:48 +0000 Subject: [PATCH 34/42] chore: polish styling for accordion --- .../CoderWorkspacesCard/ReminderAccordion.tsx | 11 ++++++++++- .../src/components/Disclosure/Disclosure.tsx | 13 +++++++++---- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.tsx b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.tsx index 2fa82f66..25e0a145 100644 --- a/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.tsx +++ b/plugins/backstage-plugin-coder/src/components/CoderWorkspacesCard/ReminderAccordion.tsx @@ -12,7 +12,7 @@ type AccordionItemInfo = Readonly<{ bodyText: ReactNode; }>; -type StyleKeys = 'root' | 'link' | 'innerPadding'; +type StyleKeys = 'root' | 'link' | 'innerPadding' | 'disclosure'; type StyleInputs = Readonly<{ hasData: boolean; }>; @@ -22,6 +22,7 @@ const useStyles = makeStyles(theme => ({ paddingTop: theme.spacing(1), marginLeft: `-${theme.spacing(2)}px`, marginRight: `-${theme.spacing(2)}px`, + marginBottom: `-${theme.spacing(2)}px`, borderTop: hasData ? 'none' : `1px solid ${theme.palette.divider}`, maxHeight: '240px', overflowX: 'hidden', @@ -31,6 +32,7 @@ const useStyles = makeStyles(theme => ({ innerPadding: { paddingLeft: theme.spacing(2), paddingRight: theme.spacing(2), + paddingBottom: theme.spacing(2), }, link: { @@ -39,6 +41,12 @@ const useStyles = makeStyles(theme => ({ textDecoration: 'underline', }, }, + + disclosure: { + '&:not(:first-child)': { + paddingTop: theme.spacing(1), + }, + }, })); export type ReminderAccordionProps = Readonly<{ @@ -113,6 +121,7 @@ export function ReminderAccordion({ {canDisplay && ( toggleAccordionGroup(id)} diff --git a/plugins/backstage-plugin-coder/src/components/Disclosure/Disclosure.tsx b/plugins/backstage-plugin-coder/src/components/Disclosure/Disclosure.tsx index 686f7656..c53eca54 100644 --- a/plugins/backstage-plugin-coder/src/components/Disclosure/Disclosure.tsx +++ b/plugins/backstage-plugin-coder/src/components/Disclosure/Disclosure.tsx @@ -1,4 +1,4 @@ -import React, { useState, type PropsWithChildren, type ReactNode } from 'react'; +import React, { type HTMLAttributes, type ReactNode, useState } from 'react'; import { useId } from '../../hooks/hookPolyfills'; import { makeStyles } from '@material-ui/core'; @@ -31,15 +31,19 @@ const useStyles = makeStyles(theme => ({ '&:hover': { backgroundColor: theme.palette.action.hover, }, + + '&:not(:first-child)': { + paddingTop: theme.spacing(6), + }, }, })); export type DisclosureProps = Readonly< - PropsWithChildren<{ + HTMLAttributes & { isExpanded?: boolean; onExpansionToggle?: () => void; headerText: ReactNode; - }> + } >; export const Disclosure = ({ @@ -47,6 +51,7 @@ export const Disclosure = ({ onExpansionToggle, headerText, children, + ...delegatedProps }: DisclosureProps) => { const hookId = useId(); const styles = useStyles(); @@ -61,7 +66,7 @@ export const Disclosure = ({ // functionality with and
elements. Would likely clean up // the component code a bit but might reduce control over screen reader output return ( -
+