diff --git a/libs/domains/clusters/feature/src/lib/cluster-add/cluster-add.spec.tsx b/libs/domains/clusters/feature/src/lib/cluster-add/cluster-add.spec.tsx index 2817251cffb..16c9ce35e97 100644 --- a/libs/domains/clusters/feature/src/lib/cluster-add/cluster-add.spec.tsx +++ b/libs/domains/clusters/feature/src/lib/cluster-add/cluster-add.spec.tsx @@ -3,11 +3,13 @@ import { renderWithProviders, screen, waitFor } from '@qovery/shared/util-tests' import { type SelfManagedClusterCreationFlowProps } from '../self-managed-cluster-creation/self-managed-cluster-creation-flow' import { ClusterAdd } from './cluster-add' +const mockUseBlocker = jest.fn() jest.mock('@tanstack/react-router', () => { const React = jest.requireActual('react') return { ...jest.requireActual('@tanstack/react-router'), useParams: () => ({ organizationId: 'org-123' }), + useBlocker: (options: unknown) => mockUseBlocker(options), Link: React.forwardRef( ({ children, ...props }: { children?: React.ReactNode }, ref: React.Ref) => React.createElement('a', { ref, ...props }, children) @@ -42,7 +44,12 @@ jest.mock('../self-managed-cluster-creation/self-managed-cluster-creation-flow', ), })) +type BlockerOptions = { disabled: boolean; shouldBlockFn: () => boolean; enableBeforeUnload: boolean } +const getBlocker = () => mockUseBlocker.mock.calls.at(-1)?.[0] as BlockerOptions + describe('ClusterAdd', () => { + afterEach(() => jest.restoreAllMocks()) + async function openCreationFlow(userEvent: ReturnType['userEvent']) { await userEvent.click(screen.getByRole('button', { name: 'AWS' })) expect(await screen.findByText('Creation flow')).toBeInTheDocument() @@ -76,6 +83,7 @@ describe('ClusterAdd', () => { selectedCloudProvider: 'AWS', selectedInstallationType: 'self-managed', }) + expect(screen.getByRole('button', { name: 'Close' })).toBeInTheDocument() await userEvent.keyboard('{Escape}') await waitFor(() => expect(screen.queryByText('Creation flow')).not.toBeInTheDocument()) @@ -89,10 +97,49 @@ describe('ClusterAdd', () => { await userEvent.keyboard('{Escape}') expect(screen.getByText('Creation flow')).toBeInTheDocument() - expect(document.querySelector('.fa-xmark')).not.toBeInTheDocument() + expect(screen.queryByRole('button', { name: 'Close' })).not.toBeInTheDocument() await userEvent.click(screen.getByRole('button', { name: 'Operator connected' })) await waitFor(() => expect(screen.queryByText('Creation flow')).not.toBeInTheDocument()) }) + + describe('leaving the install step', () => { + it('does not guard the page before the cluster is created', async () => { + const { userEvent } = renderWithProviders() + + expect(getBlocker().disabled).toBe(true) + + await openCreationFlow(userEvent) + + expect(getBlocker().disabled).toBe(true) + }) + + it('asks for confirmation before leaving, and enables the before-unload warning', async () => { + const confirm = jest.spyOn(window, 'confirm') + const { userEvent } = renderWithProviders() + + await openCreationFlow(userEvent) + await userEvent.click(screen.getByRole('button', { name: 'Reach install step' })) + + expect(getBlocker()).toMatchObject({ disabled: false, enableBeforeUnload: true }) + confirm.mockReturnValueOnce(false) + expect(getBlocker().shouldBlockFn()).toBe(true) + confirm.mockReturnValueOnce(true) + expect(getBlocker().shouldBlockFn()).toBe(false) + }) + + it('lets the flow navigate away once the Operator connects', async () => { + const confirm = jest.spyOn(window, 'confirm') + const { userEvent } = renderWithProviders() + + await openCreationFlow(userEvent) + await userEvent.click(screen.getByRole('button', { name: 'Reach install step' })) + await userEvent.click(screen.getByRole('button', { name: 'Operator connected' })) + + expect(getBlocker().shouldBlockFn()).toBe(false) + expect(getBlocker().disabled).toBe(true) + expect(confirm).not.toHaveBeenCalled() + }) + }) }) diff --git a/libs/domains/clusters/feature/src/lib/cluster-add/cluster-add.tsx b/libs/domains/clusters/feature/src/lib/cluster-add/cluster-add.tsx index a76cf74f09f..878d244aad1 100644 --- a/libs/domains/clusters/feature/src/lib/cluster-add/cluster-add.tsx +++ b/libs/domains/clusters/feature/src/lib/cluster-add/cluster-add.tsx @@ -1,11 +1,11 @@ -import { Link, useParams } from '@tanstack/react-router' +import { Link, useBlocker, useParams } from '@tanstack/react-router' import Azure from 'devicon/icons/azure/azure-original.svg' import DigitalOcean from 'devicon/icons/digitalocean/digitalocean-original.svg' import GCP from 'devicon/icons/googlecloud/googlecloud-original.svg' import Kubernetes from 'devicon/icons/kubernetes/kubernetes-original.svg' import posthog from 'posthog-js' import { useFeatureFlagEnabled } from 'posthog-js/react' -import { type ReactElement, type ReactNode, cloneElement, useState } from 'react' +import { type ReactElement, type ReactNode, cloneElement, useRef, useState } from 'react' import { Callout, Heading, Icon, Modal, Section, useModal } from '@qovery/shared/ui' import { useSupportChat } from '@qovery/shared/util-hooks' import { twMerge } from '@qovery/shared/util-js' @@ -17,6 +17,9 @@ import { type SelfManagedClusterCreationStep, } from '../self-managed-cluster-creation/self-managed-cluster-creation-flow' +const LEAVE_INSTALL_STEP_MESSAGE = + 'The Operator installation instructions will be lost if you leave this page. Leave anyway?' + type ProviderCardProps = { title: string icon: string | ReactElement @@ -38,7 +41,9 @@ function ProviderCard({ title, icon, disabled = false, actionLabel, analytics, . const { organizationId = '' } = useParams({ strict: false }) const className = twMerge( 'flex h-[52px] items-center gap-2 rounded-md border border-neutral bg-surface-neutral px-4 text-left text-sm font-medium text-neutral transition-colors', - disabled ? 'cursor-not-allowed bg-surface-neutral-component text-neutral-subtle' : 'hover:border-brand-strong' + disabled ? 'cursor-not-allowed bg-surface-neutral-component text-neutral-subtle' : 'hover:border-brand-strong', + // Not available to the organization yet: it opens an access request, not the creation flow. + actionLabel && 'border-dashed text-neutral-subtle' ) const content = ( <> @@ -111,6 +116,17 @@ export function ClusterAdd() { const [isCreationFlowOpen, setIsCreationFlowOpen] = useState(false) const [creationStep, setCreationStep] = useState('general') + // The flow closes itself before navigating once the Operator connects: that navigation must not be blocked. + const isFlowClosingRef = useRef(false) + // Once the cluster exists, leaving the page would lose the Operator installation instructions. + const isInstalling = isCreationFlowOpen && creationStep === 'install' + + useBlocker({ + disabled: !isInstalling, + shouldBlockFn: () => !isFlowClosingRef.current && !window.confirm(LEAVE_INSTALL_STEP_MESSAGE), + enableBeforeUnload: isInstalling, + }) + const openInstallationGuideModal = ({ isDemo = false }: { isDemo?: boolean } = {}) => openModal({ options: { width: 500 }, @@ -118,6 +134,7 @@ export function ClusterAdd() { }) const openCreationFlow = () => { + isFlowClosingRef.current = false setCreationStep('general') setIsCreationFlowOpen(true) } @@ -248,7 +265,10 @@ export function ClusterAdd() { setIsCreationFlowOpen(false)} + onClose={() => { + isFlowClosingRef.current = true + setIsCreationFlowOpen(false) + }} /> diff --git a/libs/domains/clusters/feature/src/lib/cluster-profile/cluster-profile-context.tsx b/libs/domains/clusters/feature/src/lib/cluster-profile/cluster-profile-context.tsx index e55b044cfd0..64f07e48092 100644 --- a/libs/domains/clusters/feature/src/lib/cluster-profile/cluster-profile-context.tsx +++ b/libs/domains/clusters/feature/src/lib/cluster-profile/cluster-profile-context.tsx @@ -26,7 +26,7 @@ export type ProfileValues = Record> export type ClusterInputValues = Record> // Scalars are compared as the inputs display them: an unset toggle shows as off, an unset text as empty. -function getDisplayedScalarValue(field: PlatformScalarField, value: unknown) { +function getDisplayedScalarValue(field: Pick, value: unknown) { return getCatalogVariableValue(field, value) ?? (field.type === 'bool' ? false : '') } @@ -106,6 +106,7 @@ function getConfigurationRequest( } const nextClusterInputs = { ...configuration?.clusterInputs } for (const [componentKey, values] of Object.entries(clusterInputs)) { + if (!Object.keys(values).length) continue nextClusterInputs[componentKey] = { ...configuration?.clusterInputs[componentKey], ...values } } @@ -163,7 +164,14 @@ export function ClusterProfileProvider({ children }: PropsWithChildren) { } const updateClusterInput = (componentKey: string, field: PlatformFieldDescriptor, value: CatalogVariableValue) => { - setClusterInputs((currentValues) => updateComponentValue(currentValues, componentKey, field.key, String(value))) + // The input shows its default while unsaved: setting it back to what is displayed is no edit. + const nextValue = String(value) + const isDisplayedValue = + getDisplayedScalarValue(field, nextValue) === + getDisplayedScalarValue(field, configuration?.clusterInputs[componentKey]?.[field.key]) + setClusterInputs((currentValues) => + updateComponentValue(currentValues, componentKey, field.key, isDisplayedValue ? undefined : nextValue) + ) } const resetChanges = () => { diff --git a/libs/domains/clusters/feature/src/lib/cluster-profile/cluster-profile.spec.tsx b/libs/domains/clusters/feature/src/lib/cluster-profile/cluster-profile.spec.tsx index f8a073058f1..4ba9bc35d3a 100644 --- a/libs/domains/clusters/feature/src/lib/cluster-profile/cluster-profile.spec.tsx +++ b/libs/domains/clusters/feature/src/lib/cluster-profile/cluster-profile.spec.tsx @@ -869,6 +869,78 @@ describe('ClusterProfileFeature', () => { expect(mockDeployCluster).not.toHaveBeenCalled() }) + it('does not count a cluster input set back to its default as a change', async () => { + const [loki, alloy] = createComponentQueries(['loki', 'alloy']) + mockUsePlatformComponentConfigurations.mockReturnValue([ + { + ...loki, + data: { + ...createResolution('loki'), + requirements: [ + { + key: 'infra.region', + label: 'Region', + type: 'string', + required: false, + sensitive: false, + constraints: {}, + defaultValue: 'eu-west-1', + status: 'READY', + }, + ], + }, + }, + alloy, + ] as ReturnType) + const { userEvent } = renderWithProviders() + const region = screen.getByRole('textbox', { name: 'Region' }) + expect(region).toHaveValue('eu-west-1') + + await userEvent.type(region, '2') + expect(screen.getByRole('region', { name: 'Unsaved profile changes' })).toHaveTextContent('1 change ongoing') + + await userEvent.type(region, '{Backspace}') + + await waitFor(() => + expect(screen.queryByRole('region', { name: 'Unsaved profile changes' })).not.toBeInTheDocument() + ) + }) + + it('does not count a toggle cluster input switched back off as a change', async () => { + const [loki, alloy] = createComponentQueries(['loki', 'alloy']) + mockUsePlatformComponentConfigurations.mockReturnValue([ + { + ...loki, + data: { + ...createResolution('loki'), + requirements: [ + { + key: 'infra.useSpot', + label: 'Use spot instances', + type: 'bool', + required: false, + sensitive: false, + constraints: {}, + status: 'READY', + }, + ], + }, + }, + alloy, + ] as ReturnType) + const { userEvent } = renderWithProviders() + const useSpot = screen.getByRole('switch', { name: 'Use spot instances' }) + + await userEvent.click(useSpot) + expect(screen.getByRole('region', { name: 'Unsaved profile changes' })).toHaveTextContent('1 change ongoing') + + await userEvent.click(useSpot) + + await waitFor(() => + expect(screen.queryByRole('region', { name: 'Unsaved profile changes' })).not.toBeInTheDocument() + ) + }) + it('does not save edits that were reverted by hand', async () => { const { userEvent } = renderWithProviders() diff --git a/libs/shared/console-shared/src/lib/catalog-variable-input/catalog-variable-input.spec.tsx b/libs/shared/console-shared/src/lib/catalog-variable-input/catalog-variable-input.spec.tsx index 00ca9107a8a..bcc76b14dd2 100644 --- a/libs/shared/console-shared/src/lib/catalog-variable-input/catalog-variable-input.spec.tsx +++ b/libs/shared/console-shared/src/lib/catalog-variable-input/catalog-variable-input.spec.tsx @@ -1,3 +1,4 @@ +import { useState } from 'react' import { renderWithProviders, screen } from '@qovery/shared/util-tests' import { CatalogVariableInput, type CatalogVariableInputProps } from './catalog-variable-input' @@ -102,6 +103,82 @@ describe('CatalogVariableInput', () => { expect(screen.getByTestId('input-toggle').parentElement).toHaveClass('items-end') }) + it.each([ + ['a toggle in the row layout', { type: 'bool' }, 'row', 'switch', undefined], + ['a select in the row layout', { type: 'string', allowedValues: ['small', 'large'] }, 'row', 'combobox', undefined], + ['a toggle in the card layout', { type: 'bool' }, 'card', 'switch', undefined], + ['a checkbox in the card layout', { type: 'bool' }, 'card', 'checkbox', 'checkbox'], + ] as const)('ties the error to %s', (_, fieldType, layout, role, booleanControl) => { + renderWithProviders( + + ) + + const control = screen.getByRole(role, { name: 'Profile' }) + expect(control).toHaveAttribute('aria-invalid', 'true') + expect(control).toHaveAccessibleDescription('This value is not allowed.') + }) + + it('does not keep a stale option once the value is cleared', async () => { + const field = { key: 'profile', label: 'Profile', type: 'string', allowedValues: ['small', 'large'] } as const + function ClearableSelect() { + const [value, setValue] = useState('large') + return ( + <> + + + + ) + } + const { userEvent } = renderWithProviders() + expect(screen.getByRole('combobox', { name: 'Profile' })).toHaveValue('large') + + await userEvent.click(screen.getByRole('button', { name: 'Clear' })) + + expect(screen.getByRole('combobox', { name: 'Profile' })).not.toHaveValue('large') + }) + + it('keeps error ids unique when two inputs share a field key', () => { + const field = { key: 'enabled', label: 'Enabled', type: 'bool' } as const + renderWithProviders( + <> + + + + ) + + expect(screen.getByRole('switch', { name: 'First' })).toHaveAccessibleDescription('First error') + expect(screen.getByRole('switch', { name: 'Second' })).toHaveAccessibleDescription('Second error') + }) + + it('does not flag a control without error', () => { + renderWithProviders( + + ) + + const control = screen.getByRole('switch', { name: 'High availability' }) + expect(control).not.toHaveAttribute('aria-invalid') + expect(control).not.toHaveAttribute('aria-describedby') + }) + it('renders sensitive values as passwords', () => { renderWithProviders( & { errorId: string } + function CatalogVariableControl({ autoFocus, booleanControl = 'toggle', error, + errorId, field, onChange, value, -}: Omit) { +}: CatalogVariableControlProps) { + const errorProps = { + 'aria-describedby': error ? errorId : undefined, + 'aria-invalid': error ? true : undefined, + } + if (field.type === 'bool') { if (booleanControl === 'checkbox') { return ( @@ -60,6 +69,7 @@ function CatalogVariableControl({ name={field.key} id={field.key} aria-label={field.label} + {...errorProps} autoFocus={autoFocus} checked={typeof value === 'boolean' ? value : false} onCheckedChange={(checked) => { @@ -75,6 +85,8 @@ function CatalogVariableControl({ small value={typeof value === 'boolean' ? value : false} ariaLabel={field.label} + ariaDescribedBy={errorProps['aria-describedby']} + ariaInvalid={errorProps['aria-invalid']} autoFocus={autoFocus} onChange={onChange} /> @@ -84,8 +96,12 @@ function CatalogVariableControl({ if (field.allowedValues?.length) { return ( ({ label: allowedValue, value: allowedValue }))} @@ -116,11 +132,12 @@ function CatalogVariableInputRow({ autoFocus, booleanControl, error, + errorId, field, highlight, onChange, value, -}: Omit) { +}: CatalogVariableControlProps) { return (
@@ -136,12 +153,15 @@ function CatalogVariableInputRow({ autoFocus={autoFocus} booleanControl={booleanControl} error={error} + errorId={errorId} field={field} onChange={onChange} value={value} /> {error && (field.type === 'bool' || field.allowedValues?.length) ? ( -

{error}

+

+ {error} +

) : null}
@@ -158,12 +178,16 @@ export function CatalogVariableInput({ onChange, value, }: CatalogVariableInputProps) { + // Field keys can repeat on a page (nested fields, requirements): the error id must be unique per instance. + const errorId = useId() + if (layout === 'row') { return ( - {error ?

{error}

: null} + {error ? ( +

+ {error} +

+ ) : null} ) } @@ -200,6 +230,8 @@ export function CatalogVariableInput({ { @@ -214,7 +246,11 @@ export function CatalogVariableInput({ {field.description ? (

{field.description}

) : null} - {error ?

{error}

: null} + {error ? ( +

+ {error} +

+ ) : null} ) } diff --git a/libs/shared/ui/src/lib/components/inputs/input-select-small/input-select-small.tsx b/libs/shared/ui/src/lib/components/inputs/input-select-small/input-select-small.tsx index 7c60cab0551..ff06feaf1a4 100644 --- a/libs/shared/ui/src/lib/components/inputs/input-select-small/input-select-small.tsx +++ b/libs/shared/ui/src/lib/components/inputs/input-select-small/input-select-small.tsx @@ -7,6 +7,8 @@ export interface InputSelectSmallProps { name: string label?: string ariaLabel?: string + ariaDescribedBy?: string + ariaInvalid?: boolean items: Value[] getValue?: (name: string, value: Value | null) => void className?: string @@ -22,6 +24,8 @@ export function InputSelectSmall(props: InputSelectSmallProps) { name, label, ariaLabel, + ariaDescribedBy, + ariaInvalid, items, defaultValue, className = '', @@ -57,6 +61,8 @@ export function InputSelectSmall(props: InputSelectSmallProps) { data-testid={dataTestId || 'input-select-small'} name={name} aria-label={ariaLabel ?? label} + aria-describedby={ariaDescribedBy} + aria-invalid={ariaInvalid} value={value} disabled={disabled} className={twMerge( diff --git a/libs/shared/ui/src/lib/components/inputs/input-toggle/input-toggle.tsx b/libs/shared/ui/src/lib/components/inputs/input-toggle/input-toggle.tsx index a6c5e7987ac..278c0b944a1 100644 --- a/libs/shared/ui/src/lib/components/inputs/input-toggle/input-toggle.tsx +++ b/libs/shared/ui/src/lib/components/inputs/input-toggle/input-toggle.tsx @@ -11,6 +11,8 @@ export interface InputToggleProps { className?: string dataTestId?: string ariaLabel?: string + ariaDescribedBy?: string + ariaInvalid?: boolean align?: 'center' | 'top' disabled?: boolean name?: string @@ -29,6 +31,8 @@ export function InputToggle(props: InputToggleProps) { disabled = false, name, ariaLabel, + ariaDescribedBy, + ariaInvalid, autoFocus, } = props @@ -75,6 +79,8 @@ export function InputToggle(props: InputToggleProps) { type="button" data-testid={props.dataTestId || 'input-toggle-button'} aria-label={accessibleName} + aria-describedby={ariaDescribedBy} + aria-invalid={ariaInvalid} checked={isChecked} disabled={disabled} name={name} diff --git a/libs/shared/ui/src/lib/components/modal/modal.tsx b/libs/shared/ui/src/lib/components/modal/modal.tsx index 6248b8ea07a..35d08a067c0 100644 --- a/libs/shared/ui/src/lib/components/modal/modal.tsx +++ b/libs/shared/ui/src/lib/components/modal/modal.tsx @@ -181,6 +181,7 @@ export const Modal = (props: ModalProps) => {