diff --git a/.claude/skills/a11y-review/SKILL.md b/.claude/skills/a11y-review/SKILL.md new file mode 100644 index 000000000..9566b4030 --- /dev/null +++ b/.claude/skills/a11y-review/SKILL.md @@ -0,0 +1,115 @@ +--- +name: a11y-review +description: Accessibility (WCAG 2.1 AA) review of UI changes - keyboard operation, accessible names/roles/states, focus management, announcements, and the repo's a11y gates. Use before finishing any change that adds or modifies interactive UI, when asked to review a branch or PR for accessibility, or when a user reports a keyboard or screen reader problem. +--- + +# Accessibility review + +Jetstream targets WCAG 2.1 AA (program docs in `docs/accessibility/`). This skill is the review +pass that the 2026 audit and code review used; run it on the current diff before calling UI work +done, or on the files the user names. + +## Scope + +- Default scope is `git diff --name-only main...HEAD` plus uncommitted changes; the user may + narrow it to files or an area. Only `.tsx` files that render UI (and their specs) matter. +- **Read each file in full**, not just the hunk: the defect is usually in how the changed element + interacts with the rest of the component (a role on the parent, a handler on the row). +- Check `docs/accessibility/audit-2026/findings.md` ("Still open after the review") before + reporting so known, deliberately-open items are not re-raised. + +## Checklist + +Walk every interactive element in scope and answer each question. "Interactive" includes anything +with an `onClick`, `onKeyDown`, `href`, `tabIndex`, or an ARIA widget role. + +1. **Name, role, state.** Does it have an accessible name that matches the visible text (icon-only + buttons need `title` or `aria-label`; the name must include the visible label - WCAG 2.5.3)? + Is the role native (` )} @@ -53,12 +60,12 @@ export const SettingsDeleteAccount: FunctionComponent
diff --git a/libs/features/formula-evaluator/src/deploy/FormulaEvaluatorPermissions.tsx b/libs/features/formula-evaluator/src/deploy/FormulaEvaluatorPermissions.tsx index 9ebc46a0d..d4eec281d 100644 --- a/libs/features/formula-evaluator/src/deploy/FormulaEvaluatorPermissions.tsx +++ b/libs/features/formula-evaluator/src/deploy/FormulaEvaluatorPermissions.tsx @@ -41,6 +41,7 @@ export function FormulaEvaluatorPermissions({ serverUrl={serverUrl} skipFrontDoorAuth={skipFrontDoorAuth} recordId={item.id} + buttonTitle={`View details for ${item.label}`} recordType={recordType} meta={item.meta as PermissionSetWithProfileRecord | PermissionSetNoProfileRecord | undefined} /> diff --git a/libs/features/load-records-multi-object/src/LoadRecordsMultiObject.tsx b/libs/features/load-records-multi-object/src/LoadRecordsMultiObject.tsx index a2d7aea36..37f614c41 100644 --- a/libs/features/load-records-multi-object/src/LoadRecordsMultiObject.tsx +++ b/libs/features/load-records-multi-object/src/LoadRecordsMultiObject.tsx @@ -1,3 +1,4 @@ +import { css } from '@emotion/react'; import { ANALYTICS_KEYS, INPUT_ACCEPT_FILETYPES, TITLES } from '@jetstream/shared/constants'; import { APP_ROUTES } from '@jetstream/shared/ui-router'; import { @@ -25,6 +26,7 @@ import { PageHeaderRow, PageHeaderTitle, Spinner, + ariaDisabledButtonProps, fireToast, } from '@jetstream/ui'; import { SkipDataHistoryCheckbox, useAmplitude } from '@jetstream/ui-core'; @@ -201,6 +203,10 @@ export const LoadRecordsMultiObject = () => { target="_blank" rel="noreferrer" download + // Underline so the link is distinguishable from surrounding text without relying on color (WCAG 1.4.1) + css={css` + text-decoration: underline; + `} onClick={() => trackEvent(ANALYTICS_KEYS.load_multi_obj_TemplateDownloaded)} > Excel template @@ -256,8 +262,7 @@ export const LoadRecordsMultiObject = () => { diff --git a/libs/features/load-records-multi-object/src/load/LoadRecordsMultiObjectLoad.tsx b/libs/features/load-records-multi-object/src/load/LoadRecordsMultiObjectLoad.tsx index fb5b2ebc8..8e4f38b64 100644 --- a/libs/features/load-records-multi-object/src/load/LoadRecordsMultiObjectLoad.tsx +++ b/libs/features/load-records-multi-object/src/load/LoadRecordsMultiObjectLoad.tsx @@ -6,7 +6,7 @@ import { Maybe, SalesforceOrgUi, SalesforceOrgUiType } from '@jetstream/types'; import { Badge, ConfirmationModalPromise, DropDown, Grid, Icon, ScopedNotification } from '@jetstream/ui'; import { ConfirmPageChange, useAmplitude } from '@jetstream/ui-core'; import { useAtomValue } from 'jotai'; -import { FunctionComponent, useMemo } from 'react'; +import { FunctionComponent, useEffect, useMemo, useRef } from 'react'; import { LoadMultiObjectRun } from '../load-records-multi-object-types'; import { buildRetryRequests } from '../load-records-multi-object-utils'; import { groupsByRefIdState, loadProgressState, requestsState, totalRecordsToLoadState } from '../load-records-multi-object.state'; @@ -104,6 +104,27 @@ export const LoadRecordsMultiObjectLoad: FunctionComponent Cancel on start, Cancel -> Load on finish), but only when focus was + // actually dropped to so we never steal it from a user who moved elsewhere. + const loadButtonRef = useRef(null); + const cancelButtonRef = useRef(null); + const wasLoadingRef = useRef(loading); + useEffect(() => { + const wasLoading = wasLoadingRef.current; + wasLoadingRef.current = loading; + if (wasLoading === loading) { + return; + } + window.setTimeout(() => { + const active = document.activeElement; + if (active && active !== document.body) { + return; + } + (loading ? cancelButtonRef.current : loadButtonRef.current)?.focus(); + }); + }, [loading]); + return (
@@ -131,7 +152,7 @@ export const LoadRecordsMultiObjectLoad: FunctionComponent )} {!loading && totalRecordCount > 0 && ( - )} diff --git a/libs/features/load-records-multi-object/src/review/LoadRecordsMultiObjectReview.tsx b/libs/features/load-records-multi-object/src/review/LoadRecordsMultiObjectReview.tsx index 8c19851bc..500efd4ed 100644 --- a/libs/features/load-records-multi-object/src/review/LoadRecordsMultiObjectReview.tsx +++ b/libs/features/load-records-multi-object/src/review/LoadRecordsMultiObjectReview.tsx @@ -196,9 +196,9 @@ export const LoadRecordsMultiObjectReview: FunctionComponent = () => { labelHelp="Specify the format of any date fields in your file. Jetstream just needs to know the order of the month and the day and will auto-detect the exact format." > +
-

Summary

+

Summary

@@ -296,21 +298,34 @@ export const PerformLoadCustomMetadata = ({ {selectedOrg.username}
-
-

Results

+

Results

+ {/* Progress and failures render inline with no focus change — live regions announce them. The + upload status is mirrored into a persistent region: a live-region role on the heading itself + removed its heading semantics, and a region that mounts already containing text is skipped */} + {hasLoaded && !results && (

Uploading custom metadata

)} - {prepareMetadataError && {prepareMetadataError}} - {hasError && errorMessage && {errorMessage}} + {prepareMetadataError && ( + + {prepareMetadataError} + + )} + {hasError && errorMessage && ( + + {errorMessage} + + )} {hasLoaded && results && ( {deployStatusUrl && ( diff --git a/libs/features/load-records/src/utils/__tests__/continue-blocked-reason.spec.ts b/libs/features/load-records/src/utils/__tests__/continue-blocked-reason.spec.ts new file mode 100644 index 000000000..0113b346a --- /dev/null +++ b/libs/features/load-records/src/utils/__tests__/continue-blocked-reason.spec.ts @@ -0,0 +1,272 @@ +import { FieldMapping, FieldMappingItemCsv } from '@jetstream/types'; +import { describe, expect, it } from 'vitest'; +import { + countFieldMappingErrors, + FieldMappingStepState, + getFieldMappingBlockedReason, + getFieldMappingErrorStatusMessage, + getSelectObjectAndFileBlockedReason, + SelectObjectAndFileStepState, +} from '../continue-blocked-reason'; + +function csvMapping(csvField: string, overrides: Partial = {}): FieldMappingItemCsv { + return { + type: 'CSV', + csvField, + targetField: null, + mappedToLookup: false, + fieldMetadata: undefined, + lookupOptionUseFirstMatch: 'FIRST', + lookupOptionNullIfNoMatch: false, + isBinaryBodyField: false, + ...overrides, + }; +} + +describe('getSelectObjectAndFileBlockedReason', () => { + const completeState: SelectObjectAndFileStepState = { + selectedSObject: { name: 'Account' }, + inputFileData: [{ Name: 'Acme' }], + loadType: 'INSERT', + externalId: '', + loadingFields: false, + }; + + it('returns null once an object is selected and a file with data is uploaded', () => { + expect(getSelectObjectAndFileBlockedReason(completeState)).toBeNull(); + }); + + it('asks for both the object and the file when neither is present', () => { + expect(getSelectObjectAndFileBlockedReason({ ...completeState, selectedSObject: null, inputFileData: null })).toBe( + 'Select an object and upload a file to continue', + ); + }); + + it('narrows to the object when only the object is missing', () => { + expect(getSelectObjectAndFileBlockedReason({ ...completeState, selectedSObject: undefined })).toBe('Select an object to continue'); + }); + + it('narrows to the file when only the file is missing', () => { + expect(getSelectObjectAndFileBlockedReason({ ...completeState, inputFileData: null })).toBe('Upload a file to continue'); + }); + + it('explains that a parsed file without data rows is not enough', () => { + expect(getSelectObjectAndFileBlockedReason({ ...completeState, inputFileData: [] })).toBe( + 'Upload a file with at least one data row to continue', + ); + }); + + it('asks for a load type when none is chosen', () => { + expect(getSelectObjectAndFileBlockedReason({ ...completeState, loadType: null })).toBe('Choose a load type to continue'); + }); + + it('asks for an external Id only for an upsert without one', () => { + expect(getSelectObjectAndFileBlockedReason({ ...completeState, loadType: 'UPSERT', externalId: '' })).toBe( + 'Select an external Id field to continue', + ); + expect(getSelectObjectAndFileBlockedReason({ ...completeState, loadType: 'UPSERT', externalId: 'External_Id__c' })).toBeNull(); + expect(getSelectObjectAndFileBlockedReason({ ...completeState, loadType: 'UPDATE', externalId: '' })).toBeNull(); + }); + + it('asks the user to wait while the fields for the object are loading', () => { + expect(getSelectObjectAndFileBlockedReason({ ...completeState, loadingFields: true })).toBe( + "Wait for the object's fields to finish loading to continue", + ); + }); + + it('lists three or more outstanding conditions with an Oxford comma', () => { + expect( + getSelectObjectAndFileBlockedReason({ + ...completeState, + selectedSObject: null, + inputFileData: null, + loadType: 'UPSERT', + externalId: '', + }), + ).toBe('Select an object, upload a file, and select an external Id field to continue'); + }); +}); + +describe('getFieldMappingBlockedReason', () => { + const mappedName = csvMapping('Name', { targetField: 'Name' }); + const completeState: FieldMappingStepState = { + fieldMapping: { Name: mappedName }, + loadType: 'INSERT', + externalId: '', + isCustomMetadataObject: false, + allowBinaryAttachment: false, + inputZipFilename: null, + binaryAttachmentBodyField: null, + }; + + it('returns null once at least one field is mapped without errors', () => { + expect(getFieldMappingBlockedReason(completeState)).toBeNull(); + }); + + it('requires at least one mapped field', () => { + expect(getFieldMappingBlockedReason({ ...completeState, fieldMapping: undefined })).toBe('Map at least one field to continue'); + expect(getFieldMappingBlockedReason({ ...completeState, fieldMapping: {} })).toBe('Map at least one field to continue'); + expect(getFieldMappingBlockedReason({ ...completeState, fieldMapping: { Name: csvMapping('Name') } })).toBe( + 'Map at least one field to continue', + ); + }); + + it('counts mapping errors with singular and plural wording', () => { + const oneError: FieldMapping = { + Name: mappedName, + Id: csvMapping('Id', { targetField: 'Id', fieldErrorMsg: 'Including a Record Id in an upsert will cause the load to fail' }), + }; + expect(getFieldMappingBlockedReason({ ...completeState, fieldMapping: oneError })).toBe('Resolve 1 mapping error to continue'); + + const twoErrors: FieldMapping = { + Name: csvMapping('Name', { targetField: 'Name', fieldErrorMsg: 'Each Salesforce field should only be mapped once' }), + 'Account Name': csvMapping('Account Name', { + targetField: 'Name', + fieldErrorMsg: 'Each Salesforce field should only be mapped once', + }), + }; + expect(getFieldMappingBlockedReason({ ...completeState, fieldMapping: twoErrors })).toBe('Resolve 2 mapping errors to continue'); + }); + + it('combines the mapped-field requirement with mapping errors', () => { + const fieldMapping: FieldMapping = { Name: csvMapping('Name', { fieldErrorMsg: 'Some error' }) }; + expect(getFieldMappingBlockedReason({ ...completeState, fieldMapping })).toBe( + 'Map at least one field and resolve 1 mapping error to continue', + ); + }); + + it('requires a related field for every lookup mapping that has none', () => { + const oneLookup: FieldMapping = { + Name: mappedName, + Owner: csvMapping('Owner', { targetField: 'OwnerId', mappedToLookup: true, targetLookupField: undefined }), + }; + expect(getFieldMappingBlockedReason({ ...completeState, fieldMapping: oneLookup })).toBe( + 'Select a related field for 1 lookup mapping to continue', + ); + + const twoLookups: FieldMapping = { + ...oneLookup, + Parent: csvMapping('Parent', { targetField: 'ParentId', mappedToLookup: true }), + }; + expect(getFieldMappingBlockedReason({ ...completeState, fieldMapping: twoLookups })).toBe( + 'Select a related field for 2 lookup mappings to continue', + ); + + const configuredLookup: FieldMapping = { + Name: mappedName, + Owner: csvMapping('Owner', { targetField: 'OwnerId', mappedToLookup: true, targetLookupField: 'Username' }), + }; + expect(getFieldMappingBlockedReason({ ...completeState, fieldMapping: configuredLookup })).toBeNull(); + }); + + describe('custom metadata', () => { + const customMetadataState: FieldMappingStepState = { + ...completeState, + isCustomMetadataObject: true, + loadType: 'UPSERT', + externalId: 'DeveloperName', + }; + + it('requires both DeveloperName and Label', () => { + expect(getFieldMappingBlockedReason({ ...customMetadataState, fieldMapping: { Name: mappedName } })).toBe( + 'Map the DeveloperName and Label fields to continue', + ); + }); + + it('narrows to the missing field', () => { + const missingLabel: FieldMapping = { DeveloperName: csvMapping('DeveloperName', { targetField: 'DeveloperName' }) }; + expect(getFieldMappingBlockedReason({ ...customMetadataState, fieldMapping: missingLabel })).toBe('Map the Label field to continue'); + + const missingDeveloperName: FieldMapping = { Label: csvMapping('Label', { targetField: 'Label' }) }; + expect(getFieldMappingBlockedReason({ ...customMetadataState, fieldMapping: missingDeveloperName })).toBe( + 'Map the DeveloperName field to continue', + ); + }); + + it('returns null once both are mapped', () => { + const fieldMapping: FieldMapping = { + DeveloperName: csvMapping('DeveloperName', { targetField: 'DeveloperName' }), + Label: csvMapping('Label', { targetField: 'Label' }), + }; + expect(getFieldMappingBlockedReason({ ...customMetadataState, fieldMapping })).toBeNull(); + }); + }); + + describe('upsert', () => { + const upsertState: FieldMappingStepState = { ...completeState, loadType: 'UPSERT', externalId: 'External_Id__c' }; + + it('requires the external Id field to be mapped', () => { + expect(getFieldMappingBlockedReason(upsertState)).toBe('Map the external Id field External_Id__c to continue'); + }); + + it('returns null once the external Id field is mapped', () => { + const fieldMapping: FieldMapping = { ...upsertState.fieldMapping, ExtId: csvMapping('ExtId', { targetField: 'External_Id__c' }) }; + expect(getFieldMappingBlockedReason({ ...upsertState, fieldMapping })).toBeNull(); + }); + + it('points back to the previous step when no external Id was chosen', () => { + expect(getFieldMappingBlockedReason({ ...upsertState, externalId: '' })).toBe( + 'Select an external Id field on the previous step to continue', + ); + }); + }); + + describe('binary attachments', () => { + const attachmentState: FieldMappingStepState = { + ...completeState, + allowBinaryAttachment: true, + inputZipFilename: 'attachments.zip', + binaryAttachmentBodyField: 'Body', + }; + + it('requires the body field once a zip file is provided', () => { + expect(getFieldMappingBlockedReason(attachmentState)).toBe('Map the Body field for the attachments zip file to continue'); + }); + + it('does not require the body field without a zip file', () => { + expect(getFieldMappingBlockedReason({ ...attachmentState, inputZipFilename: null })).toBeNull(); + }); + + it('returns null once the body field is mapped', () => { + const fieldMapping: FieldMapping = { ...attachmentState.fieldMapping, Path: csvMapping('Path', { targetField: 'Body' }) }; + expect(getFieldMappingBlockedReason({ ...attachmentState, fieldMapping })).toBeNull(); + }); + }); + + it('combines every outstanding condition into one sentence', () => { + const fieldMapping: FieldMapping = { + Name: csvMapping('Name', { targetField: 'Name', fieldErrorMsg: 'Each Salesforce field should only be mapped once' }), + 'Account Name': csvMapping('Account Name', { + targetField: 'Name', + fieldErrorMsg: 'Each Salesforce field should only be mapped once', + }), + Owner: csvMapping('Owner', { targetField: 'OwnerId', mappedToLookup: true }), + }; + expect(getFieldMappingBlockedReason({ ...completeState, fieldMapping, loadType: 'UPSERT', externalId: 'External_Id__c' })).toBe( + 'Map the external Id field External_Id__c, resolve 2 mapping errors, and select a related field for 1 lookup mapping to continue', + ); + }); +}); + +describe('field mapping error status', () => { + it('counts only rows with an error message', () => { + expect(countFieldMappingErrors(undefined)).toBe(0); + expect(countFieldMappingErrors({ Name: csvMapping('Name', { targetField: 'Name' }) })).toBe(0); + expect( + countFieldMappingErrors({ + Name: csvMapping('Name', { targetField: 'Name', fieldErrorMsg: 'Duplicate' }), + Other: csvMapping('Other', { targetField: 'Name', fieldErrorMsg: 'Duplicate' }), + Clean: csvMapping('Clean', { targetField: 'Clean__c' }), + }), + ).toBe(2); + }); + + it('builds an empty message when there is nothing to resolve', () => { + expect(getFieldMappingErrorStatusMessage(0)).toBe(''); + }); + + it('pluralizes the announcement', () => { + expect(getFieldMappingErrorStatusMessage(1)).toBe('1 field mapping error to resolve'); + expect(getFieldMappingErrorStatusMessage(2)).toBe('2 field mapping errors to resolve'); + }); +}); diff --git a/libs/features/load-records/src/utils/continue-blocked-reason.ts b/libs/features/load-records/src/utils/continue-blocked-reason.ts new file mode 100644 index 000000000..1c0fa72d2 --- /dev/null +++ b/libs/features/load-records/src/utils/continue-blocked-reason.ts @@ -0,0 +1,138 @@ +import { DescribeGlobalSObjectResult, FieldMapping, InsertUpdateUpsertDelete, Maybe } from '@jetstream/types'; + +/** + * Builds the human-readable reason the "Continue" button on a Load Records step is disabled. + * + * Each step function returns `null` once every condition is satisfied, so callers derive the + * disabled flag from the reason (`reason !== null`) — the message and the gate can never disagree. + */ + +export interface SelectObjectAndFileStepState { + selectedSObject: Maybe>; + inputFileData: Maybe; + loadType: Maybe; + externalId: Maybe; + loadingFields: boolean; +} + +export interface FieldMappingStepState { + fieldMapping: Maybe; + loadType: InsertUpdateUpsertDelete; + externalId: Maybe; + isCustomMetadataObject: boolean; + allowBinaryAttachment: boolean; + inputZipFilename: Maybe; + binaryAttachmentBodyField: Maybe; +} + +/** The final step has no next step; the button stays disabled with this explanation. */ +export const LAST_STEP_BLOCKED_REASON = 'This is the last step. Use Start Over to load another file.'; + +export function getSelectObjectAndFileBlockedReason({ + selectedSObject, + inputFileData, + loadType, + externalId, + loadingFields, +}: SelectObjectAndFileStepState): string | null { + const requiredActions: string[] = []; + if (!selectedSObject) { + requiredActions.push('select an object'); + } + if (!inputFileData) { + requiredActions.push('upload a file'); + } else if (inputFileData.length === 0) { + // Empty rows are stripped on parse, so a header-only file ends up here + requiredActions.push('upload a file with at least one data row'); + } + if (!loadType) { + requiredActions.push('choose a load type'); + } + if (loadType === 'UPSERT' && !externalId) { + requiredActions.push('select an external Id field'); + } + if (loadingFields) { + requiredActions.push("wait for the object's fields to finish loading"); + } + return toContinueSentence(requiredActions); +} + +export function getFieldMappingBlockedReason({ + fieldMapping, + loadType, + externalId, + isCustomMetadataObject, + allowBinaryAttachment, + inputZipFilename, + binaryAttachmentBodyField, +}: FieldMappingStepState): string | null { + const mappingItems = Object.values(fieldMapping || {}); + const isTargetFieldMapped = (targetField: Maybe) => mappingItems.some((item) => item.targetField === targetField); + const requiredActions: string[] = []; + + if (!mappingItems.some((item) => !!item.targetField)) { + requiredActions.push('map at least one field'); + } else { + // Custom metadata forces an upsert on DeveloperName, so a missing DeveloperName is reported once + // here rather than again as the external Id below + const missingRequiredFields: string[] = []; + if (isCustomMetadataObject) { + missingRequiredFields.push(...['DeveloperName', 'Label'].filter((requiredField) => !isTargetFieldMapped(requiredField))); + } + if (missingRequiredFields.length > 0) { + requiredActions.push(`map the ${missingRequiredFields.join(' and ')} ${missingRequiredFields.length === 1 ? 'field' : 'fields'}`); + } + if (loadType === 'UPSERT' && !externalId) { + requiredActions.push('select an external Id field on the previous step'); + } else if (loadType === 'UPSERT' && externalId && !missingRequiredFields.includes(externalId) && !isTargetFieldMapped(externalId)) { + requiredActions.push(`map the external Id field ${externalId}`); + } + if (allowBinaryAttachment && inputZipFilename && !isTargetFieldMapped(binaryAttachmentBodyField)) { + requiredActions.push(`map the ${binaryAttachmentBodyField || 'Body'} field for the attachments zip file`); + } + } + + const errorCount = countFieldMappingErrors(fieldMapping); + if (errorCount > 0) { + requiredActions.push(`resolve ${errorCount} mapping ${errorCount === 1 ? 'error' : 'errors'}`); + } + + const incompleteLookupCount = mappingItems.filter((item) => item.mappedToLookup && !item.targetLookupField).length; + if (incompleteLookupCount > 0) { + requiredActions.push( + `select a related field for ${incompleteLookupCount} lookup ${incompleteLookupCount === 1 ? 'mapping' : 'mappings'}`, + ); + } + + return toContinueSentence(requiredActions); +} + +/** Rows showing an error message (duplicate target field, record Id in an upsert) */ +export function countFieldMappingErrors(fieldMapping: Maybe): number { + return Object.values(fieldMapping || {}).filter((item) => !!item.fieldErrorMsg).length; +} + +/** + * Live-region text for the field mapping step. Empty when there is nothing to resolve so the + * region stays mounted and only the message changes. + */ +export function getFieldMappingErrorStatusMessage(errorCount: number): string { + if (errorCount === 0) { + return ''; + } + return `${errorCount} field mapping ${errorCount === 1 ? 'error' : 'errors'} to resolve`; +} + +/** "select an object" + "upload a file" -> "Select an object and upload a file to continue" */ +function toContinueSentence(requiredActions: string[]): string | null { + if (requiredActions.length === 0) { + return null; + } + let actionList: string; + if (requiredActions.length <= 2) { + actionList = requiredActions.join(' and '); + } else { + actionList = `${requiredActions.slice(0, -1).join(', ')}, and ${requiredActions[requiredActions.length - 1]}`; + } + return `${actionList.charAt(0).toUpperCase()}${actionList.slice(1)} to continue`; +} diff --git a/libs/features/load-records/tsconfig.lib.json b/libs/features/load-records/tsconfig.lib.json index 538d7d66f..e6a4318ee 100644 --- a/libs/features/load-records/tsconfig.lib.json +++ b/libs/features/load-records/tsconfig.lib.json @@ -66,6 +66,9 @@ }, { "path": "../../shared/client-logger/tsconfig.lib.json" + }, + { + "path": "../../test-utils/tsconfig.lib.json" } ] } diff --git a/libs/features/manage-permissions/src/ManagePermissionsEditor.tsx b/libs/features/manage-permissions/src/ManagePermissionsEditor.tsx index 2b706fab0..f48499e69 100644 --- a/libs/features/manage-permissions/src/ManagePermissionsEditor.tsx +++ b/libs/features/manage-permissions/src/ManagePermissionsEditor.tsx @@ -47,6 +47,7 @@ import { ToolbarItemGroup, Tooltip, fireToast, + getAriaKeyshortcuts, getModifierKey, } from '@jetstream/ui'; import { ConfirmPageChange, RequireMetadataApiBanner, fromJetstreamEvents, fromPermissionsState, useAmplitude } from '@jetstream/ui-core'; @@ -810,7 +811,12 @@ export const ManagePermissionsEditor: FunctionComponent } > - + Go Back @@ -827,6 +833,7 @@ export const ManagePermissionsEditor: FunctionComponent @@ -835,6 +842,7 @@ export const ManagePermissionsEditor: FunctionComponent diff --git a/libs/features/manage-permissions/src/PermissionColumnGroupHeader.tsx b/libs/features/manage-permissions/src/PermissionColumnGroupHeader.tsx index 987702b3c..cd805c675 100644 --- a/libs/features/manage-permissions/src/PermissionColumnGroupHeader.tsx +++ b/libs/features/manage-permissions/src/PermissionColumnGroupHeader.tsx @@ -54,6 +54,7 @@ export function PermissionColumnGroupHeader({ id, label, type }: PermissionColum onKeyDown={(event) => event.stopPropagation()} > ({ id={`${row.key}-${id}-${actionKey}`} checked={value} tabIndex={-1} + // The visible context lives in the column-group header far above and the row label far to + // the left — name each checkbox with both so arrowing down a column announces which + // object/field/tab/permission is being toggled, and for which profile/permission set + aria-label={`${actionType} ${row.label} for ${label} (${type})`} + // Grid arrow navigation focuses the checkbox itself (APG single-widget cell) so its role, + // checked state, and toggle affordance are announced + {...(disabled ? {} : { 'data-grid-inner-focus': true })} // Stop the click from also reaching the wrapping div's onClick — otherwise a direct click // (or programmatic keyboard activation) toggles via both handlers. onChange owns the toggle. onClick={(ev) => ev.stopPropagation()} @@ -874,7 +881,12 @@ function getColumnForProfileOrPermSet({ if (args.row.type === 'HEADING') { return ; } - return )} />; + return ( + )} + contextLabel={`${actionType} for ${label} (${type})`} + /> + ); }, // On grouped tables (field permissions) the group header shows how many of the group's child rows // have this permission checked. Object/tab tables aren't grouped, so this never renders there. @@ -1692,11 +1704,24 @@ export function resetRow(type: Permis /** * Pinned row selection renderer */ -export const PinnedSelectAllRendererWrapper = ({ column }: RenderSummaryCellProps) => { - const { onColumnAction } = useContext(DataTableGenericContext) as PermissionManagerTableContext; +export const PinnedSelectAllRendererWrapper = ({ + column, + contextLabel, +}: RenderSummaryCellProps & { contextLabel?: string }) => { + const { onColumnAction, announce } = useContext(DataTableGenericContext) as PermissionManagerTableContext; + // e.g. "Read for Admin (Profile)" — every column renders these same three buttons, so the + // accessible names and the outcome announcement must say which column they act on + const scopedSuffix = contextLabel ? `: ${contextLabel}` : ''; function handleSelection(action: 'selectAll' | 'unselectAll' | 'reset') { onColumnAction(action, column.key); + const outcome = + action === 'selectAll' + ? `Selected all visible rows${scopedSuffix}` + : action === 'unselectAll' + ? `Unselected all visible rows${scopedSuffix}` + : `Reset visible rows to previous selection${scopedSuffix}`; + announce(outcome); } return ( @@ -1709,29 +1734,29 @@ export const PinnedSelectAllRendererWrapper = ({ column }: RenderSummaryCellProp
); @@ -2007,6 +2032,8 @@ export const RowActionRenderer = ({ commitEdit, row }: RenderCellProps @@ -2172,7 +2199,7 @@ export const BulkActionRenderer = () => {
)} - diff --git a/libs/features/org-groups/src/lib/DeleteOrgsModal.tsx b/libs/features/org-groups/src/lib/DeleteOrgsModal.tsx index 19cd19044..746f940fc 100644 --- a/libs/features/org-groups/src/lib/DeleteOrgsModal.tsx +++ b/libs/features/org-groups/src/lib/DeleteOrgsModal.tsx @@ -1,9 +1,20 @@ -import { css } from '@emotion/react'; import { ANALYTICS_KEYS } from '@jetstream/shared/constants'; import { deleteOrg } from '@jetstream/shared/data'; import { pluralizeFromNumber } from '@jetstream/shared/utils'; import { SalesforceOrgUi } from '@jetstream/types'; -import { Badge, Checkbox, Grid, Icon, Modal, RadioButton, RadioGroup, Spinner, fireToast } from '@jetstream/ui'; +import { + ariaDisabledButtonProps, + Badge, + Checkbox, + fireToast, + Grid, + Icon, + Modal, + RadioButton, + RadioGroup, + Spinner, + useAnnouncer, +} from '@jetstream/ui'; import { useAmplitude } from '@jetstream/ui-core'; import classNames from 'classnames'; import groupBy from 'lodash/groupBy'; @@ -64,9 +75,13 @@ export const DeleteOrgsModal = ({ orgs, onClose, onDeleted }: DeleteOrgsModalPro } } + // The delete button's label swap to "Are you sure?" is not announced on its own + const { announce, announcer } = useAnnouncer(); + async function handleConfirmOrDelete() { if (!confirmDelete) { setConfirmDelete(true); + announce(`Press again to confirm deleting ${selectedOrgIds.size} ${pluralizeFromNumber('org', selectedOrgIds.size)}`); return; } setIsDeleting(true); @@ -131,8 +146,8 @@ export const DeleteOrgsModal = ({ orgs, onClose, onDeleted }: DeleteOrgsModalPro 'slds-button_text-destructive': !confirmDelete, 'slds-button_destructive': confirmDelete, })} - onClick={() => handleConfirmOrDelete()} - disabled={selectedOrgIds.size === 0 || isDeleting} + // aria-disabled keeps focus on the button while its second click disables it + {...ariaDisabledButtonProps(selectedOrgIds.size === 0 || isDeleting, () => handleConfirmOrDelete())} > {confirmDelete ? ( @@ -148,9 +163,11 @@ export const DeleteOrgsModal = ({ orgs, onClose, onDeleted }: DeleteOrgsModalPro } > + {announcer} {orgsWithErrors.length > 0 && ( setFilterMode(value as 'all' | 'errors')} />
-
- onChange()} - > - Org Id: {org.organizationId} - -
-
- onChange()} - > - {org.instanceUrl} - -
+
Org Id: {org.organizationId}
+
{org.instanceUrl}
- {updatedAtText && ( -
- onChange()} - > - {updatedAtText} - -
- )} + {updatedAtText &&
{updatedAtText}
} {org.connectionError && Connection Error} } diff --git a/libs/features/org-groups/src/lib/OrgGroupCard.tsx b/libs/features/org-groups/src/lib/OrgGroupCard.tsx index 007c4b036..786838e05 100644 --- a/libs/features/org-groups/src/lib/OrgGroupCard.tsx +++ b/libs/features/org-groups/src/lib/OrgGroupCard.tsx @@ -39,7 +39,7 @@ export function OrgGroupCardCard({ const { ref: dropRef, isDropTarget } = useDroppable({ id, accept: (source) => (source.data as DraggableSfdcCard).organizationId !== id, - data: { action: 'add', orgGroupId: id } satisfies SfdcCardDropTarget, + data: { action: 'add', orgGroupId: id, label: name } satisfies SfdcCardDropTarget, }); const tertiaryActionMenuItems = useMemo(() => { @@ -79,6 +79,7 @@ export function OrgGroupCardCard({ {!isActive && ( - )} !!(source.data as DraggableSfdcCard).organizationId, - data: { action: 'remove' } satisfies SfdcCardDropTarget, + data: { action: 'remove', label: UNASSIGNED_ORGS_DROP_LABEL } satisfies SfdcCardDropTarget, }); return ( @@ -42,6 +42,7 @@ export function OrgGroupCardNoOrganization({ {!isActive && ( - )} diff --git a/libs/features/org-groups/src/lib/OrgGroupModal.tsx b/libs/features/org-groups/src/lib/OrgGroupModal.tsx index 7d3c576f1..8abef2f28 100644 --- a/libs/features/org-groups/src/lib/OrgGroupModal.tsx +++ b/libs/features/org-groups/src/lib/OrgGroupModal.tsx @@ -13,6 +13,7 @@ interface OrgGroupModalProps { export function OrgGroupModal({ orgGroup, onSubmit, onClose }: OrgGroupModalProps) { const isMounted = useRef(true); + const nameInputRef = useRef(null); const [loading, setLoading] = useState(false); const [updatedOrg, setUpdatedOrg] = useState(() => ({ name: orgGroup?.name || '', @@ -58,6 +59,7 @@ export function OrgGroupModal({ orgGroup, onSubmit, onClose }: OrgGroupModalProp } size="sm" + initialFocus={nameInputRef} onClose={onClose} >
@@ -71,10 +73,10 @@ export function OrgGroupModal({ orgGroup, onSubmit, onClose }: OrgGroupModalProp > }>({ open: false }); + // Deleting a group unmounts its card along with the menu that opened the confirmation, and deleting the last + // org unmounts the org actions menu - so those flows hand focus to this always-present control instead of + const createGroupButtonRef = useRef(null); const { handleAddOrg } = useUpdateOrgs(); @@ -163,18 +167,32 @@ export function OrgGroups({ onAddOrgHandlerFn }: { onAddOrgHandlerFn?: AddOrgHan content: 'Any Salesforce Orgs will be removed from this organization but will not be deleted.', }) ) { - await deleteOrgGroup(organization.id); - setOrgGroupsFromDb(getOrgGroups()); - setOrgs( - allOrgs.map((org) => { - if (org.jetstreamOrganizationId !== organization.id) { - return org; - } - return { ...org, jetstreamOrganizationId: null }; - }), - ); - handleCloseOrganizationModal(); - trackEvent(ANALYTICS_KEYS.organizations_deleted, { priorCount: groups.length }); + try { + await deleteOrgGroup(organization.id); + setOrgGroupsFromDb(getOrgGroups()); + setOrgs( + allOrgs.map((org) => { + if (org.jetstreamOrganizationId !== organization.id) { + return org; + } + return { ...org, jetstreamOrganizationId: null }; + }), + ); + handleCloseOrganizationModal(); + createGroupButtonRef.current?.focus(); + trackEvent(ANALYTICS_KEYS.organizations_deleted, { priorCount: groups.length }); + fireToast({ + message: `Organization "${organization.name}" deleted successfully`, + type: 'success', + }); + } catch (ex) { + tracker.error('Org Group: Error deleting group', ex); + logger.error('Org Group: Error deleting group', ex); + fireToast({ + message: `Failed to delete group. Please try again.`, + type: 'error', + }); + } } }; @@ -205,6 +223,7 @@ export function OrgGroups({ onAddOrgHandlerFn }: { onAddOrgHandlerFn?: AddOrgHan // Remove deleted orgs from state setOrgs(allOrgs.filter((org) => org.jetstreamOrganizationId !== organization.id)); handleCloseOrganizationModal(); + createGroupButtonRef.current?.focus(); trackEvent(ANALYTICS_KEYS.organizations_deleted_with_orgs, { priorCount: groups.length, deletedOrgCount: organization.orgs.length, @@ -277,6 +296,9 @@ export function OrgGroups({ onAddOrgHandlerFn }: { onAddOrgHandlerFn?: AddOrgHan setOrgGroupsFromDb(getOrgGroups()); const refreshedOrgs = await getOrgs(); setOrgs(refreshedOrgs); + if (refreshedOrgs.length === 0) { + createGroupButtonRef.current?.focus(); + } }; return ( @@ -298,6 +320,7 @@ export function OrgGroups({ onAddOrgHandlerFn }: { onAddOrgHandlerFn?: AddOrgHan /> {allOrgs.length > 0 && } - + {/* Stays focusable while its own click disables it — native disabled would drop focus to */} + + + {/* Announced once when the refresh starts (a per-org count would queue an announcement for every + completion); the completion toast announces the outcome */} + + ); }; diff --git a/libs/features/org-groups/src/lib/SalesforceOrgCardConnectionRefresh.tsx b/libs/features/org-groups/src/lib/SalesforceOrgCardConnectionRefresh.tsx index 1082671c2..bd2e52f6c 100644 --- a/libs/features/org-groups/src/lib/SalesforceOrgCardConnectionRefresh.tsx +++ b/libs/features/org-groups/src/lib/SalesforceOrgCardConnectionRefresh.tsx @@ -3,11 +3,16 @@ import { ANALYTICS_KEYS } from '@jetstream/shared/constants'; import { checkOrgHealth, getOrgs } from '@jetstream/shared/data'; import { ORG_INACTIVITY_EXPIRATION_DAYS, pluralizeFromNumber } from '@jetstream/shared/utils'; import { AddOrgHandlerFn, BadgeType, Maybe, SalesforceOrgUi } from '@jetstream/types'; -import { Badge, ConfirmationModalPromise, Grid, Icon, Spinner, Tooltip, fireToast } from '@jetstream/ui'; +import { ariaDisabledButtonProps, Badge, ConfirmationModalPromise, fireToast, Grid, Icon, Spinner, Tooltip } from '@jetstream/ui'; import { AddOrg, OrgExpirationStatus, useAmplitude, useOrgExpiration, useUpdateOrgs } from '@jetstream/ui-core'; import { fromAppState } from '@jetstream/ui/app-state'; import { useSetAtom } from 'jotai'; -import { useState } from 'react'; +import { useEffect, useRef, useState } from 'react'; + +/** Id of an org card's heading — the focus target when the card's connection controls disappear under the user */ +export function getOrgCardHeadingId(orgUniqueId: string) { + return `org-card-heading-${orgUniqueId}`; +} interface SalesforceOrgCardConnectionRefreshProps { org: SalesforceOrgUi; @@ -63,6 +68,20 @@ export function SalesforceOrgCardConnectionRefresh({ const [isRefreshing, setIsRefreshing] = useState(false); const setOrgs = useSetAtom(fromAppState.salesforceOrgsAsyncState); + const showsConnectionControls = !!orgExpiration.isExpiring || !!org.connectionError; + + // A successful refresh clears the expiry / error state, which removes this whole block — and the + // Refresh button the keyboard user just activated — so land focus on the card heading instead of + // letting it fall to + const previouslyShowedControlsRef = useRef(showsConnectionControls); + useEffect(() => { + const controlsDisappeared = previouslyShowedControlsRef.current && !showsConnectionControls; + previouslyShowedControlsRef.current = showsConnectionControls; + if (controlsDisappeared && document.activeElement === document.body) { + document.getElementById(getOrgCardHeadingId(org.uniqueId))?.focus(); + } + }, [showsConnectionControls, org.uniqueId]); + const handleRefreshOrg = async () => { setIsRefreshing(true); let success = true; @@ -106,7 +125,7 @@ export function SalesforceOrgCardConnectionRefresh({ } }; - if (!orgExpiration.isExpiring && !org.connectionError) { + if (!showsConnectionControls) { return null; } @@ -118,19 +137,29 @@ export function SalesforceOrgCardConnectionRefresh({ {badge.isVisible && ( - {badge.label} + + {badge.label} + {/* The explanation is otherwise tooltip-only on an element that cannot take focus */} + {badge.tooltip} + )} {refreshIcon.isVisible && ( + {/* Stays focusable while its own click disables it — native disabled would drop focus to */} )} @@ -148,11 +177,11 @@ export function SalesforceOrgCardConnectionRefresh({ /> )} diff --git a/libs/features/org-groups/src/lib/SalesforceOrgCardDraggable.tsx b/libs/features/org-groups/src/lib/SalesforceOrgCardDraggable.tsx index 5c1c6ccb2..55b9886b0 100644 --- a/libs/features/org-groups/src/lib/SalesforceOrgCardDraggable.tsx +++ b/libs/features/org-groups/src/lib/SalesforceOrgCardDraggable.tsx @@ -5,7 +5,7 @@ import { AddOrgHandlerFn, SalesforceOrgUi } from '@jetstream/types'; import { Badge, Grid, Icon } from '@jetstream/ui'; import { OrgInfoPopover, useUpdateOrgs } from '@jetstream/ui-core'; import { DraggableSfdcCard } from './organization-group.types'; -import { SalesforceOrgCardConnectionRefresh } from './SalesforceOrgCardConnectionRefresh'; +import { getOrgCardHeadingId, SalesforceOrgCardConnectionRefresh } from './SalesforceOrgCardConnectionRefresh'; interface SalesforceOrgCardDraggableProps { org: SalesforceOrgUi; @@ -21,10 +21,12 @@ export function SalesforceOrgCardDraggable({ org, isActive, onAddOrgHandlerFn }: const { actionInProgress, orgLoading, handleAddOrg, handleRemoveOrg, handleUpdateOrg } = useUpdateOrgs(); const orgType = getOrgType(org); - const { ref, isDragging } = useDraggable({ + // A dedicated drag handle keeps dnd-kit's role="button"/tabindex off the card itself — the card + // contains interactive children (org popover, refresh), which a button role must not + const { ref, handleRef, isDragging } = useDraggable({ id: org.uniqueId, type: 'SalesforceOrg', - data: { uniqueId: org.uniqueId, organizationId: org.jetstreamOrganizationId ?? null }, + data: { uniqueId: org.uniqueId, organizationId: org.jetstreamOrganizationId ?? null, label: org.label }, }); return ( @@ -41,7 +43,11 @@ export function SalesforceOrgCardDraggable({ org, isActive, onAddOrgHandlerFn }:
-
+
-

{org.label}

+ + + {/* Focus target when the card's connection controls disappear after a refresh */} +

+ {org.label} +

+
{orgType && ( diff --git a/libs/features/org-groups/src/lib/SalesforceOrgsActions.tsx b/libs/features/org-groups/src/lib/SalesforceOrgsActions.tsx index afe1303a5..acb9d84bd 100644 --- a/libs/features/org-groups/src/lib/SalesforceOrgsActions.tsx +++ b/libs/features/org-groups/src/lib/SalesforceOrgsActions.tsx @@ -31,6 +31,7 @@ export const SalesforceOrgsActions = ({ orgs, onOrgsDeleted }: SalesforceOrgsAct | OrgGroup[]>([]); +const salesforceOrgsAsyncState = atom | SalesforceOrgUi[]>([]); + +vi.doMock('@jetstream/ui/app-state', () => { + const orgGroupsState = unwrap(orgGroupsAsyncState, (prev) => prev ?? []); + const salesforceOrgsState = unwrap(salesforceOrgsAsyncState, (prev) => prev ?? []); + const selectedOrgIdState = atom(null); + const ActiveOrgGroupState = atom(null); + const orgGroupsWithOrgsSelector = atom( + (get) => { + const orgs = get(salesforceOrgsState); + return get(orgGroupsState).map((group) => ({ + ...group, + orgs: group.orgs.map(({ uniqueId }) => orgs.find((org) => org.uniqueId === uniqueId)).filter(Boolean), + })); + }, + (_get, set, newValue: OrgGroupWithOrgs[]) => + set( + orgGroupsState, + newValue.map((group) => ({ ...group, orgs: group.orgs.map(({ uniqueId }) => ({ uniqueId })) })), + ), + ); + return { + fromAppState: { + orgGroupsState, + orgGroupsWithOrgsSelector, + ActiveOrgGroupState, + salesforceOrgsState, + salesforceOrgsAsyncState, + selectedOrgIdState, + salesforceOrgsWithoutGroupSelector: atom((get) => get(salesforceOrgsState).filter((org) => !org.jetstreamOrganizationId)), + selectedOrgStateWithoutPlaceholder: atom((get) => { + const selectedOrgId = get(selectedOrgIdState); + return get(salesforceOrgsState).find((org) => org.uniqueId === selectedOrgId); + }), + }, + getRecentlySelectedOrgForGroup: () => null, + }; +}); + +vi.doMock('@jetstream/ui-core', () => ({ + AddOrg: ({ label }: { label: string }) => , + ConfirmPageChange: () => null, + OrgInfoPopover: () => null, + useAmplitude: () => ({ trackEvent }), + useOrgExpiration: () => ({ isExpiring: false, isExpired: false }), + useUpdateOrgs: () => ({ + actionInProgress: false, + orgLoading: false, + handleAddOrg: vi.fn(), + handleRemoveOrg: vi.fn(), + handleUpdateOrg: vi.fn(), + }), +})); + +vi.doMock('@jetstream/shared/data', () => ({ + addOrgToGroup: vi.fn(), + checkOrgHealth: vi.fn(), + createOrgGroup: (...args: unknown[]) => createOrgGroup(...args), + deleteOrg: (...args: unknown[]) => deleteOrg(...args), + deleteOrgGroup: (...args: unknown[]) => deleteOrgGroup(...args), + deleteOrgGroupAndAllOrgs: vi.fn(), + getOrgGroups: () => getOrgGroups(), + getOrgs: () => getOrgs(), + updateOrgGroup: (...args: unknown[]) => updateOrgGroup(...args), +})); + +const { OrgGroups } = await import('../OrgGroups'); + +function buildGroup(overrides: Partial = {}): OrgGroup { + return { id: 'group-1', name: 'Production', description: '', orgs: [], ...overrides } as OrgGroup; +} + +function buildOrg(overrides: Partial = {}): SalesforceOrgUi { + return { + uniqueId: 'org-1', + label: 'Acme Sandbox', + username: 'admin@acme.com', + organizationId: '00D000000000001', + instanceUrl: 'https://acme.my.salesforce.com', + jetstreamOrganizationId: null, + ...overrides, + } as SalesforceOrgUi; +} + +function setup({ groups = [], orgs = [] }: { groups?: OrgGroup[]; orgs?: SalesforceOrgUi[] } = {}) { + const store = createStore(); + store.set(orgGroupsAsyncState, groups); + store.set(salesforceOrgsAsyncState, orgs); + render( + + + + , + ); +} + +function getCreateButton() { + return screen.getByRole('button', { name: 'Create New Group' }); +} + +async function submitGroupName(name: string) { + const nameInput = await screen.findByLabelText(/Group Name/); + await waitFor(() => expect(document.activeElement).toBe(nameInput)); + userEvent.clear(nameInput); + await userEvent.type(nameInput, name); + userEvent.click(screen.getByRole('button', { name: 'Save' })); + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); +} + +describe('OrgGroups', () => { + beforeEach(() => { + vi.clearAllMocks(); + getOrgGroups.mockResolvedValue([]); + getOrgs.mockResolvedValue([]); + }); + + describe('focus management', () => { + it('returns focus to "Create New Group" after a group is created through the modal', async () => { + createOrgGroup.mockResolvedValue(buildGroup({ id: 'group-2', name: 'Sandboxes' })); + setup(); + const createButton = getCreateButton(); + + userEvent.click(createButton); + await submitGroupName('Sandboxes'); + + expect(createOrgGroup).toHaveBeenCalledWith({ name: 'Sandboxes', description: '' }); + await screen.findByTestId('org-group-card-Sandboxes'); + await waitFor(() => expect(document.activeElement).toBe(createButton)); + }); + + it("returns focus to the group's Edit button after the group is updated", async () => { + const group = buildGroup(); + updateOrgGroup.mockResolvedValue({ ...group, name: 'Prod' }); + setup({ groups: [group] }); + const editButton = screen.getByRole('button', { name: 'Edit - Production' }); + + userEvent.click(editButton); + await submitGroupName('Prod'); + + expect(updateOrgGroup).toHaveBeenCalledWith('group-1', { name: 'Prod', description: '' }); + await screen.findByTestId('org-group-card-Prod'); + await waitFor(() => expect(document.activeElement).toBe(editButton)); + }); + + it('moves focus to "Create New Group" after a group is deleted, since the group\'s own menu is gone', async () => { + deleteOrgGroup.mockResolvedValue(undefined); + setup({ groups: [buildGroup()] }); + + userEvent.click(screen.getByRole('button', { name: 'More Actions - Production' })); + userEvent.click(screen.getByRole('menuitem', { name: 'Delete Group' })); + userEvent.click(await screen.findByRole('button', { name: 'Delete' })); + + await waitFor(() => expect(deleteOrgGroup).toHaveBeenCalledWith('group-1')); + await waitFor(() => expect(screen.queryByTestId('org-group-card-Production')).toBeNull()); + await waitFor(() => expect(document.activeElement).toBe(getCreateButton())); + }); + + it('moves focus to "Create New Group" after the last org is deleted, since the org actions menu unmounts with it', async () => { + deleteOrg.mockResolvedValue(undefined); + setup({ orgs: [buildOrg()] }); + + userEvent.click(screen.getByRole('button', { name: 'Salesforce org actions' })); + userEvent.click(screen.getByRole('menuitem', { name: 'Delete Salesforce Orgs' })); + userEvent.click(await screen.findByLabelText('Select All')); + // Two presses: the first swaps the label to "Are you sure?", the second deletes + userEvent.click(screen.getByRole('button', { name: 'Delete 1 Org' })); + userEvent.click(screen.getByRole('button', { name: 'Are you sure?' })); + + await waitFor(() => expect(deleteOrg).toHaveBeenCalledTimes(1)); + await waitFor(() => expect(screen.queryByRole('dialog')).toBeNull()); + await waitFor(() => expect(screen.queryByRole('button', { name: 'Salesforce org actions' })).toBeNull()); + await waitFor(() => expect(document.activeElement).toBe(getCreateButton())); + }); + }); +}); diff --git a/libs/features/org-groups/src/lib/__tests__/RefreshAllOrgsButton.spec.tsx b/libs/features/org-groups/src/lib/__tests__/RefreshAllOrgsButton.spec.tsx index 4ff8056e2..12b3691b7 100644 --- a/libs/features/org-groups/src/lib/__tests__/RefreshAllOrgsButton.spec.tsx +++ b/libs/features/org-groups/src/lib/__tests__/RefreshAllOrgsButton.spec.tsx @@ -94,7 +94,8 @@ describe('RefreshAllOrgsButton', () => { await clickRefresh(); - expect(button.hasAttribute('disabled')).toBe(true); + // aria-disabled (not native disabled) so the button keeps keyboard focus while the refresh runs + expect(button.getAttribute('aria-disabled')).toBe('true'); expect(button.textContent).toContain('Refreshing 0 of 3'); // The queue caps in-flight checks, so the remaining orgs stay queued until an earlier one settles expect(checkOrgHealth).toHaveBeenCalledTimes(2); @@ -107,7 +108,7 @@ describe('RefreshAllOrgsButton', () => { } }); expect(checkOrgHealth).toHaveBeenCalledTimes(3); - await waitFor(() => expect(button.hasAttribute('disabled')).toBe(false)); + await waitFor(() => expect(button.getAttribute('aria-disabled')).toBeNull()); }); it('should explain the inactivity expiration in a tooltip', async () => { diff --git a/libs/features/org-groups/src/lib/__tests__/SalesforceOrgCardDraggable.spec.tsx b/libs/features/org-groups/src/lib/__tests__/SalesforceOrgCardDraggable.spec.tsx new file mode 100644 index 000000000..73d767431 --- /dev/null +++ b/libs/features/org-groups/src/lib/__tests__/SalesforceOrgCardDraggable.spec.tsx @@ -0,0 +1,87 @@ +import { DragDropProvider } from '@dnd-kit/react'; +import { SalesforceOrgUi } from '@jetstream/types'; +import { render, screen, waitFor } from '@testing-library/react'; +import { atom } from 'jotai'; +import { describe, expect, it, vi } from 'vitest'; +import { ORG_GROUP_DRAG_INSTRUCTIONS, ORG_GROUP_DRAG_PLUGINS } from '../org-group-drag-announcements'; + +// The org info popover lives in ui-core; the card's own controls are what this spec inspects +vi.doMock('@jetstream/ui-core', () => ({ + AddOrg: ({ label }: { label: string }) => , + OrgInfoPopover: () => null, + useAmplitude: () => ({ trackEvent: vi.fn() }), + useOrgExpiration: () => ({ isExpiring: false, isExpired: false }), + useUpdateOrgs: () => ({ + actionInProgress: false, + orgLoading: false, + handleAddOrg: vi.fn(), + handleRemoveOrg: vi.fn(), + handleUpdateOrg: vi.fn(), + }), +})); + +vi.doMock('@jetstream/ui/app-state', () => ({ + fromAppState: { salesforceOrgsAsyncState: atom([]) }, +})); + +vi.doMock('@jetstream/shared/data', () => ({ + checkOrgHealth: vi.fn(), + getOrgs: vi.fn(), +})); + +const { SalesforceOrgCardDraggable } = await import('../SalesforceOrgCardDraggable'); + +const org = { + uniqueId: '00D000000000001-005000000000001', + label: 'Acme Sandbox', + username: 'admin@acme.com', + organizationId: '00D000000000001', + instanceUrl: 'https://acme.my.salesforce.com', + jetstreamOrganizationId: null, +} as SalesforceOrgUi; + +function renderCard() { + return render( + + + , + ); +} + +function getAccessibleName(element: Element) { + return element.getAttribute('aria-label') ?? element.textContent ?? ''; +} + +describe('SalesforceOrgCardDraggable', () => { + it('names the drag handle after the action and the org, and describes it with the move instructions', async () => { + renderCard(); + const handle = screen.getByRole('button', { name: 'Move Acme Sandbox' }); + + // dnd-kit applies its attributes in a scheduled effect after mount + await waitFor(() => expect(handle.getAttribute('aria-describedby')).toBeTruthy()); + expect(handle.getAttribute('aria-roledescription')).toBe('draggable'); + const description = document.getElementById(handle.getAttribute('aria-describedby') as string); + expect(description?.textContent).toBe(ORG_GROUP_DRAG_INSTRUCTIONS.draggable); + }); + + it('keeps the org id and instance url as plain content, outside every control name', async () => { + renderCard(); + const handle = screen.getByRole('button', { name: 'Move Acme Sandbox' }); + await waitFor(() => expect(handle.getAttribute('aria-describedby')).toBeTruthy()); + + const controls = Array.from(document.querySelectorAll('button, a[href], input, [tabindex]')); + expect(controls.length).toBeGreaterThan(0); + for (const control of controls) { + const name = getAccessibleName(control); + const description = document.getElementById(control.getAttribute('aria-describedby') ?? '')?.textContent ?? ''; + expect(`${name} ${description}`).not.toContain(org.organizationId); + expect(`${name} ${description}`).not.toContain(org.instanceUrl); + } + + // The details remain readable in browse mode + expect(screen.getByText(org.organizationId)).toBeTruthy(); + expect(screen.getByText(org.instanceUrl)).toBeTruthy(); + // The heading is a programmatic focus target only (tabindex -1), named by the org label alone + expect(screen.getByRole('heading', { name: 'Acme Sandbox' }).getAttribute('tabindex')).toBe('-1'); + }); +}); diff --git a/libs/features/org-groups/src/lib/__tests__/org-group-drag-announcements.spec.ts b/libs/features/org-groups/src/lib/__tests__/org-group-drag-announcements.spec.ts new file mode 100644 index 000000000..33b8b8377 --- /dev/null +++ b/libs/features/org-groups/src/lib/__tests__/org-group-drag-announcements.spec.ts @@ -0,0 +1,63 @@ +import { describe, expect, it } from 'vitest'; +import { ORG_GROUP_DRAG_ANNOUNCEMENTS } from '../org-group-drag-announcements'; +import { DraggableSfdcCard, SfdcCardDropTarget } from '../organization-group.types'; + +const orgCard: DraggableSfdcCard = { uniqueId: '00D1-0051', organizationId: null, label: 'Acme Sandbox' }; +const productionGroup: SfdcCardDropTarget = { action: 'add', orgGroupId: '3f2a9c1e', label: 'Production Orgs' }; +const unassigned: SfdcCardDropTarget = { action: 'remove', label: 'Orgs without a group' }; + +// The plugin hands the announcement functions the live drag operation; only `data` (and `canceled`) is read +function buildEvent({ source, target, canceled = false }: { source?: DraggableSfdcCard; target?: SfdcCardDropTarget; canceled?: boolean }) { + return { + canceled, + operation: { + source: source ? { id: source.uniqueId, data: source } : null, + target: target ? { id: 'droppable-id', data: target } : null, + }, + } as never; +} + +const manager = {} as never; + +describe('ORG_GROUP_DRAG_ANNOUNCEMENTS', () => { + it('announces the org label instead of its id when picked up', () => { + expect(ORG_GROUP_DRAG_ANNOUNCEMENTS.dragstart(buildEvent({ source: orgCard }), manager)).toBe('Picked up Acme Sandbox.'); + }); + + it('announces the group name instead of its id while hovering', () => { + expect(ORG_GROUP_DRAG_ANNOUNCEMENTS.dragover?.(buildEvent({ source: orgCard, target: productionGroup }), manager)).toBe( + 'Acme Sandbox is over Production Orgs.', + ); + expect(ORG_GROUP_DRAG_ANNOUNCEMENTS.dragover?.(buildEvent({ source: orgCard, target: unassigned }), manager)).toBe( + 'Acme Sandbox is over Orgs without a group.', + ); + expect(ORG_GROUP_DRAG_ANNOUNCEMENTS.dragover?.(buildEvent({ source: orgCard }), manager)).toBe('Acme Sandbox is not over a group.'); + }); + + it('announces the outcome of the drop', () => { + expect(ORG_GROUP_DRAG_ANNOUNCEMENTS.dragend(buildEvent({ source: orgCard, target: productionGroup }), manager)).toBe( + 'Acme Sandbox was moved to Production Orgs.', + ); + expect(ORG_GROUP_DRAG_ANNOUNCEMENTS.dragend(buildEvent({ source: orgCard }), manager)).toBe( + 'Acme Sandbox was dropped outside a group and was not moved.', + ); + expect(ORG_GROUP_DRAG_ANNOUNCEMENTS.dragend(buildEvent({ source: orgCard, target: productionGroup, canceled: true }), manager)).toBe( + 'Move cancelled. Acme Sandbox was not moved.', + ); + }); + + it('never falls back to the ids when a label is missing', () => { + const unlabeledSource = { ...orgCard, label: '' }; + const unlabeledTarget = { ...productionGroup, label: '' }; + const announcement = ORG_GROUP_DRAG_ANNOUNCEMENTS.dragend(buildEvent({ source: unlabeledSource, target: unlabeledTarget }), manager); + expect(announcement).toBe('Salesforce org was moved to group.'); + expect(announcement).not.toContain(orgCard.uniqueId); + expect(announcement).not.toContain(productionGroup.orgGroupId); + }); + + it('stays silent without a drag source', () => { + expect(ORG_GROUP_DRAG_ANNOUNCEMENTS.dragstart(buildEvent({}), manager)).toBeUndefined(); + expect(ORG_GROUP_DRAG_ANNOUNCEMENTS.dragover?.(buildEvent({}), manager)).toBeUndefined(); + expect(ORG_GROUP_DRAG_ANNOUNCEMENTS.dragend(buildEvent({}), manager)).toBeUndefined(); + }); +}); diff --git a/libs/features/org-groups/src/lib/org-group-drag-announcements.ts b/libs/features/org-groups/src/lib/org-group-drag-announcements.ts new file mode 100644 index 000000000..432a9bc31 --- /dev/null +++ b/libs/features/org-groups/src/lib/org-group-drag-announcements.ts @@ -0,0 +1,72 @@ +import { Accessibility, type DragDropManagerInput, defaultPreset } from '@dnd-kit/dom'; +import type { DragOverEvent, DragStartEvent } from '@dnd-kit/react'; +import { DraggableSfdcCard, SfdcCardDropTarget } from './organization-group.types'; + +type AccessibilityOptions = NonNullable[1]>; +type DragAnnouncements = NonNullable; + +type DragSource = DragStartEvent['operation']['source']; +type DropTarget = DragOverEvent['operation']['target']; + +function getSourceLabel(source: DragSource) { + return (source?.data as DraggableSfdcCard | undefined)?.label || 'Salesforce org'; +} + +function getTargetLabel(target: DropTarget) { + return (target?.data as SfdcCardDropTarget | undefined)?.label || 'group'; +} + +/** + * dnd-kit's default announcements interpolate the draggable and droppable ids, which are UUIDs on this page, + * so a screen reader user heard "Picked up draggable item 3f2a…". These resolve to the org label and the group + * name that the cards carry in their drag/drop `data`. + */ +export const ORG_GROUP_DRAG_ANNOUNCEMENTS: DragAnnouncements = { + dragstart: ({ operation: { source } }) => { + if (!source) { + return undefined; + } + return `Picked up ${getSourceLabel(source)}.`; + }, + dragover: ({ operation: { source, target } }) => { + if (!source) { + return undefined; + } + if (!target) { + return `${getSourceLabel(source)} is not over a group.`; + } + return `${getSourceLabel(source)} is over ${getTargetLabel(target)}.`; + }, + dragend: ({ operation: { source, target }, canceled }) => { + if (!source) { + return undefined; + } + const sourceLabel = getSourceLabel(source); + if (canceled) { + return `Move cancelled. ${sourceLabel} was not moved.`; + } + if (!target) { + return `${sourceLabel} was dropped outside a group and was not moved.`; + } + return `${sourceLabel} was moved to ${getTargetLabel(target)}.`; + }, +}; + +/** Read (via aria-describedby) when a drag handle receives focus */ +export const ORG_GROUP_DRAG_INSTRUCTIONS = { + draggable: + 'To move this org to another group, press Space or Enter to pick it up, use the arrow keys to move it over the destination group, then press Space or Enter to drop it. Press Escape to cancel.', +} satisfies AccessibilityOptions['screenReaderInstructions']; + +/** + * dnd-kit's default plugin set with its Accessibility plugin configured for this page. + * Module-level so DragDropProvider, which compares the list by reference, never re-installs the plugins. + * (Annotated because the inferred type reaches into @dnd-kit/abstract, which this repo does not depend on directly.) + */ +export const ORG_GROUP_DRAG_PLUGINS: NonNullable = [ + Accessibility.configure({ + announcements: ORG_GROUP_DRAG_ANNOUNCEMENTS, + screenReaderInstructions: ORG_GROUP_DRAG_INSTRUCTIONS, + } satisfies AccessibilityOptions), + ...defaultPreset.plugins.filter((plugin) => plugin !== Accessibility), +]; diff --git a/libs/features/org-groups/src/lib/organization-group.types.ts b/libs/features/org-groups/src/lib/organization-group.types.ts index 6d4a709fa..90528d134 100644 --- a/libs/features/org-groups/src/lib/organization-group.types.ts +++ b/libs/features/org-groups/src/lib/organization-group.types.ts @@ -1,10 +1,16 @@ export interface DraggableSfdcCard { uniqueId: string; organizationId: string | null; + /** Announced to screen readers in place of the uniqueId while the card is dragged */ + label: string; } +/** Drop-target label for the card that holds orgs outside any group */ +export const UNASSIGNED_ORGS_DROP_LABEL = 'Orgs without a group'; + /** * Data attached to org-group drop targets, read in the DragDropProvider onDragEnd handler. * `add` targets carry the destination group id; `remove` targets clear the org's group. + * `label` is announced to screen readers in place of the droppable id. */ -export type SfdcCardDropTarget = { action: 'add'; orgGroupId: string } | { action: 'remove' }; +export type SfdcCardDropTarget = ({ action: 'add'; orgGroupId: string } | { action: 'remove' }) & { label: string }; diff --git a/libs/features/permission-analysis/src/PermissionAnalysisExportGrid.tsx b/libs/features/permission-analysis/src/PermissionAnalysisExportGrid.tsx index 4beebbc16..581b333bf 100644 --- a/libs/features/permission-analysis/src/PermissionAnalysisExportGrid.tsx +++ b/libs/features/permission-analysis/src/PermissionAnalysisExportGrid.tsx @@ -28,6 +28,7 @@ import { pickAssignmentExportClickableColumnKeys, pickPermissionSetExportClickableColumnKeys, pickTabVisibilityExportClickableColumnKeys, + withFindingDetailsCell, type PermissionAnalysisFinding, type PermissionExportRow, type SobjectExportDetail, @@ -308,22 +309,6 @@ type ContainerModalState = { type ExportFindingsModalState = FieldCellModalState | ContainerModalState | null; -function mergeFindingCellClass( - column: ColumnWithFilter, - extraClass: (row: T) => string | undefined, -): ColumnWithFilter { - const prior = column.cellClass; - return { - ...column, - cellClass: (row: T) => { - const a = typeof prior === 'function' ? prior(row) : prior; - const b = extraClass(row); - const merged = [a, b].filter(Boolean).join(' '); - return merged.length > 0 ? merged : undefined; - }, - } as ColumnWithFilter; -} - /** * Read-only SOQL export rows with dynamic columns and quick filter. * Optional issue highlights for field permissions, permission set / profile rows, and assignments. @@ -404,21 +389,14 @@ export const PermissionAnalysisExportGrid: FunctionComponent { const key = typeof col.key === 'string' ? col.key : ''; - return mergeFindingCellClass(col, (row: RowWithKey) => { + return withFindingDetailsCell(col, (row: RowWithKey) => { const parentId = typeof row.ParentId === 'string' ? row.ParentId.trim() : ''; const objectApi = typeof row.SobjectType === 'string' ? row.SobjectType.trim() : ''; const fieldApi = typeof row.Field === 'string' ? row.Field.trim() : ''; if (!parentId || !objectApi || !fieldApi) { return undefined; } - const severity = fieldPermissionCellSeverity(fieldHighlights, parentId, objectApi, fieldApi, key); - if (severity === 'error') { - return 'permission-finding-cell--error permission-finding-cell--clickable'; - } - if (severity === 'warning') { - return 'permission-finding-severity-cell--warning permission-finding-cell--clickable'; - } - return undefined; + return fieldPermissionCellSeverity(fieldHighlights, parentId, objectApi, fieldApi, key); }); }); } @@ -427,7 +405,7 @@ export const PermissionAnalysisExportGrid: FunctionComponent { const key = typeof col.key === 'string' ? col.key : ''; const isClickColumn = permissionSetClickColumns.includes(key); - return mergeFindingCellClass(col, (row: RowWithKey) => { + return withFindingDetailsCell(col, (row: RowWithKey) => { if (!isClickColumn) { return undefined; } @@ -435,14 +413,7 @@ export const PermissionAnalysisExportGrid: FunctionComponent { const key = typeof col.key === 'string' ? col.key : ''; const isClickColumn = assignmentClickColumns.includes(key); - return mergeFindingCellClass(col, (row: RowWithKey) => { + return withFindingDetailsCell(col, (row: RowWithKey) => { if (!isClickColumn) { return undefined; } @@ -459,14 +430,7 @@ export const PermissionAnalysisExportGrid: FunctionComponent { const key = typeof col.key === 'string' ? col.key : ''; const isClickColumn = tabVisibilityClickColumns.includes(key); - return mergeFindingCellClass(col, (row: RowWithKey) => { + return withFindingDetailsCell(col, (row: RowWithKey) => { if (!isClickColumn) { return undefined; } @@ -483,14 +447,7 @@ export const PermissionAnalysisExportGrid: FunctionComponent(key, fieldType), - name: headerLabel, - key, - field: key, - resizable: true, - width: TREE_COL_PERMISSION_BOOL, - minWidth: TREE_MIN_PERMISSION_BOOL, - cellClass: (row: FieldPermissionTreeRow) => { - if (!isFieldPermissionLeafRow(row)) { - return undefined; - } - if (key !== 'PermissionsRead' && key !== 'PermissionsEdit') { - return undefined; - } - const parentId = typeof row.ParentId === 'string' ? row.ParentId.trim() : ''; - const sobjectType = typeof row.SobjectType === 'string' ? row.SobjectType.trim() : ''; - const fieldFull = typeof row.Field === 'string' ? row.Field.trim() : ''; - if (!parentId || !sobjectType || !fieldFull) { - return undefined; - } - const severity = fieldPermissionCellSeverity(findingCellHighlightsRef.current, parentId, sobjectType, fieldFull, key); - if (severity === 'error') { - return 'permission-finding-cell--error permission-finding-cell--clickable'; - } - if (severity === 'warning') { - return 'permission-finding-severity-cell--warning permission-finding-cell--clickable'; - } + const baseColumn = setColumnFromType(key, fieldType); + // Reads through findingCellHighlightsRef (resolved per render inside the finding cell wrapper) + // so the columns memo stays stable + const severityForRow = (row: FieldPermissionTreeRow) => { + if (!isFieldPermissionLeafRow(row)) { return undefined; - }, - } as ColumnWithFilter); + } + if (key !== 'PermissionsRead' && key !== 'PermissionsEdit') { + return undefined; + } + const parentId = typeof row.ParentId === 'string' ? row.ParentId.trim() : ''; + const sobjectType = typeof row.SobjectType === 'string' ? row.SobjectType.trim() : ''; + const fieldFull = typeof row.Field === 'string' ? row.Field.trim() : ''; + if (!parentId || !sobjectType || !fieldFull) { + return undefined; + } + return fieldPermissionCellSeverity(findingCellHighlightsRef.current, parentId, sobjectType, fieldFull, key); + }; + permissionCols.push( + withFindingDetailsCell( + { + ...baseColumn, + name: headerLabel, + key, + field: key, + resizable: true, + width: TREE_COL_PERMISSION_BOOL, + minWidth: TREE_MIN_PERMISSION_BOOL, + } as ColumnWithFilter, + severityForRow, + { columnLabel: headerLabel }, + ), + ); } return [groupPermSetCol, groupObjectCol, fieldCol, ...permissionCols]; diff --git a/libs/features/permission-analysis/src/PermissionAnalysisFindingsFiltersBar.tsx b/libs/features/permission-analysis/src/PermissionAnalysisFindingsFiltersBar.tsx index 0fb40b2d7..488f84058 100644 --- a/libs/features/permission-analysis/src/PermissionAnalysisFindingsFiltersBar.tsx +++ b/libs/features/permission-analysis/src/PermissionAnalysisFindingsFiltersBar.tsx @@ -109,7 +109,13 @@ export const PermissionAnalysisFindingsFiltersBar: FunctionComponent
-
ev.stopPropagation()} onPointerDown={(ev) => ev.stopPropagation()} onKeyDown={(ev) => ev.stopPropagation()}> + {/* Event fence only (keeps the toolbar's handlers from seeing the popover's events) — not a control */} +
ev.stopPropagation()} + onPointerDown={(ev) => ev.stopPropagation()} + onKeyDown={(ev) => ev.stopPropagation()} + > ev.stopPropagation(), - 'aria-label': 'Filters', + // No aria-label: the visible "Filters (N)" text names the trigger, count included title: 'Filters', }} > diff --git a/libs/features/permission-analysis/src/PermissionAnalysisFindingsModal.tsx b/libs/features/permission-analysis/src/PermissionAnalysisFindingsModal.tsx index a8d8f9cf0..ec41da6b2 100644 --- a/libs/features/permission-analysis/src/PermissionAnalysisFindingsModal.tsx +++ b/libs/features/permission-analysis/src/PermissionAnalysisFindingsModal.tsx @@ -1,8 +1,8 @@ import { css } from '@emotion/react'; import { APP_ROUTES } from '@jetstream/shared/ui-router'; -import { ColumnWithFilter, Grid, Modal, ViewDocsLink } from '@jetstream/ui'; -import { GridDownloadButton } from '@jetstream/ui-core'; -import { FunctionComponent, ReactNode } from 'react'; +import { ColumnWithFilter, Grid, Icon, Modal, ViewDocsLink } from '@jetstream/ui'; +import { GridDownloadModal } from '@jetstream/ui-core'; +import { FunctionComponent, ReactNode, useState } from 'react'; import { getFindingCodeDisplayParts, getFindingLabelForCode, type PermissionAnalysisFinding } from './permission-export-result-view'; /** Deep link to the issue code reference table in the docs. Undefined only if the route ever loses its DOCS entry. */ @@ -48,7 +48,7 @@ function trimmedFindingField(finding: PermissionAnalysisFinding, key: keyof Perm return typeof value === 'string' ? value.trim() : ''; } -/** Flat column set for {@link GridDownloadButton}; `getValue` mirrors what the modal shows per finding. */ +/** Flat column set for {@link GridDownloadModal}; `getValue` mirrors what the modal shows per finding. */ const FINDINGS_DOWNLOAD_COLUMNS: ColumnWithFilter[] = [ { key: 'severity', name: 'Severity', getValue: ({ row }) => severityLabelForFinding(row) }, { @@ -115,176 +115,194 @@ export const PermissionAnalysisFindingsModal: FunctionComponent { + // Modal `hide` UNMOUNTS its content, so the download modal's state and rendering must live HERE + // (the component that stays mounted) — not inside the footer, where hiding would destroy it + const [isDownloadModalOpen, setIsDownloadModalOpen] = useState(false); if (!open) { return null; } return ( - - - - - } - onClose={onClose} - className="slds-p-around_small" - > -
+ + + + + } + onClose={onClose} + className="slds-p-around_small" > -
- {summaryLine} - -
- {findings.map((finding, index) => { - const code = typeof finding.code === 'string' ? finding.code.trim() : ''; - const codeParts = getFindingCodeDisplayParts(code || undefined); - const summaryTitle = codeParts.title.trim(); - const detailText = findingDetailText(finding, summaryTitle); - const { accent, tint } = findingBlockChrome(finding); - return ( -
-
+
+ {summaryLine} + +
+
+ {findings.map((finding, index) => { + const code = typeof finding.code === 'string' ? finding.code.trim() : ''; + const codeParts = getFindingCodeDisplayParts(code || undefined); + const summaryTitle = codeParts.title.trim(); + const detailText = findingDetailText(finding, summaryTitle); + const { accent, tint } = findingBlockChrome(finding); + return ( +
- {code ? ( - <> - {severityLabelForFinding(finding)} - - - - {codeParts.technicalCode ? ( - <> - {summaryTitle}{' '} +
+ {code ? ( + <> + {severityLabelForFinding(finding)} + + + + {codeParts.technicalCode ? ( + <> + {summaryTitle}{' '} + + {codeParts.technicalCode} + + + ) : ( - {codeParts.technicalCode} + {summaryTitle} - - ) : ( - - {summaryTitle} - - )} - - ) : ( - {severityLabelForFinding(finding)} - )} + )} + + ) : ( + {severityLabelForFinding(finding)} + )} +
+ {detailText ? ( +

+ {detailText} +

+ ) : null} + {typeof finding.objectApiName === 'string' && finding.objectApiName.trim().length > 0 ? ( +

+ Object: {finding.objectApiName.trim()} +

+ ) : null} + {typeof finding.fieldApiName === 'string' && finding.fieldApiName.trim().length > 0 ? ( +

+ Field: {finding.fieldApiName.trim()} +

+ ) : null} + {typeof finding.permissionSetId === 'string' && finding.permissionSetId.trim().length > 0 ? ( +

+ Permission set Id: {finding.permissionSetId.trim()} +

+ ) : null}
- {detailText ? ( -

- {detailText} -

- ) : null} - {typeof finding.objectApiName === 'string' && finding.objectApiName.trim().length > 0 ? ( -

- Object: {finding.objectApiName.trim()} -

- ) : null} - {typeof finding.fieldApiName === 'string' && finding.fieldApiName.trim().length > 0 ? ( -

- Field: {finding.fieldApiName.trim()} -

- ) : null} - {typeof finding.permissionSetId === 'string' && finding.permissionSetId.trim().length > 0 ? ( -

- Permission set Id: {finding.permissionSetId.trim()} -

- ) : null}
-
- ); - })} + ); + })} +
-
-
+ + {isDownloadModalOpen && ( + setIsDownloadModalOpen(false)} + /> + )} + ); }; diff --git a/libs/features/permission-analysis/src/PermissionAnalysisIssuesTab.tsx b/libs/features/permission-analysis/src/PermissionAnalysisIssuesTab.tsx index 7581749e9..5be3e6eee 100644 --- a/libs/features/permission-analysis/src/PermissionAnalysisIssuesTab.tsx +++ b/libs/features/permission-analysis/src/PermissionAnalysisIssuesTab.tsx @@ -363,6 +363,7 @@ const AggregatedIssueCodeRollupCard: FunctionComponent<{ tabIndex={0} css={aggregatedRollupRowInteractiveCss} title="View issue details" + aria-label={`View issue details for ${row.label.trim() || row.code}: ${row.count} ${row.count === 1 ? 'issue' : 'issues'}, ${row.errorCount} ${row.errorCount === 1 ? 'error' : 'errors'}, ${row.warningCount} ${row.warningCount === 1 ? 'warning' : 'warnings'}`} onClick={onOpen} onKeyDown={aggregatedRollupRowKeyDown(onOpen)} > @@ -448,6 +449,7 @@ const AggregatedObjectRollupCard: FunctionComponent<{ tabIndex={0} css={aggregatedRollupRowInteractiveCss} title="View issue details" + aria-label={`View issue details for ${row.objectApiName}: ${row.count} ${row.count === 1 ? 'issue' : 'issues'}, ${row.errorCount} ${row.errorCount === 1 ? 'error' : 'errors'}, ${row.warningCount} ${row.warningCount === 1 ? 'warning' : 'warnings'}`} onClick={onOpen} onKeyDown={aggregatedRollupRowKeyDown(onOpen)} > diff --git a/libs/features/permission-analysis/src/PermissionAnalysisObjectPermissionsTree.tsx b/libs/features/permission-analysis/src/PermissionAnalysisObjectPermissionsTree.tsx index 1fa6a4f7b..ab8d1c822 100644 --- a/libs/features/permission-analysis/src/PermissionAnalysisObjectPermissionsTree.tsx +++ b/libs/features/permission-analysis/src/PermissionAnalysisObjectPermissionsTree.tsx @@ -24,6 +24,7 @@ import { objectPermissionFindingRowKey, sortObjectPermissionExportRowsForAnalysisTree, sortedObjectPermissionBooleanKeys, + withFindingDetailsCell, type PermissionAnalysisFinding, type PermissionExportRow, type SobjectExportDetail, @@ -123,7 +124,9 @@ function renderPermissionSetGroupCell( `} onClick={toggleGroup} title={exportLabel} + aria-expanded={isExpanded} > + {/* Decorative: the button's visible text names it and aria-expanded carries the state */} (key, fieldType), - name: headerLabel, - key, - field: key, - resizable: true, - width: TREE_COL_PERMISSION_BOOL, - minWidth: TREE_MIN_PERMISSION_BOOL, - cellClass: (row: ObjectPermissionTreeRow) => { - if (!isObjectPermissionLeafRow(row)) { - return undefined; - } - const parentId = typeof row.ParentId === 'string' ? row.ParentId.trim() : ''; - const sobjectType = typeof row.SobjectType === 'string' ? row.SobjectType.trim() : ''; - if (!parentId || !sobjectType) { - return undefined; - } - const rowKey = objectPermissionFindingRowKey(parentId, sobjectType); - const severity = findingCellHighlights.get(rowKey)?.get(columnKey); - if (severity === 'error') { - return 'permission-finding-cell--error permission-finding-cell--clickable'; - } - if (severity === 'warning') { - return 'permission-finding-severity-cell--warning permission-finding-cell--clickable'; - } + const baseColumn = setColumnFromType(key, fieldType); + const severityForRow = (row: ObjectPermissionTreeRow) => { + if (!isObjectPermissionLeafRow(row)) { return undefined; - }, - } as ColumnWithFilter); + } + const parentId = typeof row.ParentId === 'string' ? row.ParentId.trim() : ''; + const sobjectType = typeof row.SobjectType === 'string' ? row.SobjectType.trim() : ''; + if (!parentId || !sobjectType) { + return undefined; + } + const rowKey = objectPermissionFindingRowKey(parentId, sobjectType); + return findingCellHighlights.get(rowKey)?.get(columnKey); + }; + permissionCols.push( + withFindingDetailsCell( + { + ...baseColumn, + name: headerLabel, + key, + field: key, + resizable: true, + width: TREE_COL_PERMISSION_BOOL, + minWidth: TREE_MIN_PERMISSION_BOOL, + } as ColumnWithFilter, + severityForRow, + { columnLabel: headerLabel }, + ), + ); } return [groupPermSetCol, objectCol, ...permissionCols]; diff --git a/libs/features/permission-analysis/src/PermissionAnalysisPermissionSetsTree.tsx b/libs/features/permission-analysis/src/PermissionAnalysisPermissionSetsTree.tsx index 841bb5a28..7435cc4d9 100644 --- a/libs/features/permission-analysis/src/PermissionAnalysisPermissionSetsTree.tsx +++ b/libs/features/permission-analysis/src/PermissionAnalysisPermissionSetsTree.tsx @@ -291,8 +291,8 @@ function renderPermissionSetGroupCell( setupLogin: { org: SalesforceOrgUi; serverUrl: string; skipFrontDoorAuth: boolean }, onOpenFindings: (permissionSetId: string) => void, resolveSetupTarget: (permissionSetId: string) => { recordType: ProfileOrPermSetRecordType; recordId: string }, - openInSetupTitle: string, - findingsForContainerButtonTitle: string, + /** "profile" or "permission set" — names the per-group controls, which otherwise repeat identically for every group */ + containerNoun: string, { groupKey, childRows, isExpanded, toggleGroup }: RenderGroupCellProps, ) { const permissionSetId = String(groupKey); @@ -347,6 +347,8 @@ function renderPermissionSetGroupCell( type="button" className="slds-button slds-button_reset slds-p-around_xx-small" title={isExpanded ? 'Collapse' : 'Expand'} + // Every group repeats this control — the name carries which group it expands + aria-label={`${isExpanded ? 'Collapse' : 'Expand'} ${titleLabel}`} aria-expanded={isExpanded} css={css` flex-shrink: 0; @@ -360,7 +362,6 @@ function renderPermissionSetGroupCell( icon={isExpanded ? 'chevrondown' : 'chevronright'} className="slds-icon slds-icon-text-default slds-icon_x-small" omitContainer - description={isExpanded ? 'Collapse' : 'Expand'} /> ) => { if (event.shiftKey || event.ctrlKey || event.metaKey) { if (!canDeepLink) { @@ -453,7 +456,7 @@ function renderPermissionSetGroupCell( serverUrl={setupLogin.serverUrl} skipFrontDoorAuth={setupLogin.skipFrontDoorAuth} returnUrl={returnUrl} - title={openInSetupTitle} + title={`Open ${containerNoun} ${titleLabel} in Salesforce Setup`} omitIcon className={OBJECT_TYPE_ACTION_BUTTON_CLASSNAME} onClick={(event) => { @@ -466,7 +469,8 @@ function renderPermissionSetGroupCell( )} @@ -522,8 +525,7 @@ export const PermissionAnalysisPermissionSetsTree: FunctionComponent { const isProfilesTree = treePresentation === 'profiles'; const groupColumnName = isProfilesTree ? 'Profile' : 'Permission Set'; - const openInSetupTitle = isProfilesTree ? 'Open this profile in Salesforce Setup' : 'Open this permission set in Salesforce Setup'; - const findingsForContainerButtonTitle = isProfilesTree ? 'View issues for this profile' : 'View issues for this permission set'; + const containerNoun = isProfilesTree ? 'profile' : 'permission set'; const rowByPermissionSetId = useMemo(() => { const map = new Map(); @@ -684,8 +686,7 @@ export const PermissionAnalysisPermissionSetsTree: FunctionComponent { @@ -729,13 +730,14 @@ export const PermissionAnalysisPermissionSetsTree: FunctionComponent { @@ -852,8 +854,7 @@ export const PermissionAnalysisPermissionSetsTree: FunctionComponent + {/* Decorative: the button's visible text names it and aria-expanded carries the state */} + {/* Decorative: the button's visible text names it and aria-expanded carries the state */} { @@ -579,7 +580,7 @@ export const PermissionAnalysisUserAssignmentsTree: FunctionComponent { @@ -627,7 +628,8 @@ export const PermissionAnalysisUserAssignmentsTree: FunctionComponent { event.stopPropagation(); openFindingsForPermissionSet(permissionSetId); @@ -640,7 +642,6 @@ export const PermissionAnalysisUserAssignmentsTree: FunctionComponent )} @@ -681,7 +682,7 @@ export const PermissionAnalysisUserAssignmentsTree: FunctionComponent { @@ -732,7 +733,7 @@ export const PermissionAnalysisUserAssignmentsTree: FunctionComponent { @@ -779,7 +780,7 @@ export const PermissionAnalysisUserAssignmentsTree: FunctionComponent { diff --git a/libs/features/permission-analysis/src/PermissionAnalysisView.tsx b/libs/features/permission-analysis/src/PermissionAnalysisView.tsx index 7f4624a4c..564576266 100644 --- a/libs/features/permission-analysis/src/PermissionAnalysisView.tsx +++ b/libs/features/permission-analysis/src/PermissionAnalysisView.tsx @@ -6,6 +6,7 @@ import { escapeSoqlString, formatNumber } from '@jetstream/shared/ui-utils'; import { getErrorMessage, gzipDecode, pluralizeIfMultiple } from '@jetstream/shared/utils'; import type { AsyncJob, PermissionExportAnalysisJob, PermissionExportFullResult } from '@jetstream/types'; import { + AssistiveStatus, AutoFullHeightContainer, Icon, ProgressIndicator, @@ -17,13 +18,14 @@ import { ToolbarItemGroup, Tooltip, ViewDocsLink, + useAnnouncer, } from '@jetstream/ui'; import { PermissionAnalysisHistoryModal, RequireMetadataApiBanner, jobsState } from '@jetstream/ui-core'; import { applicationCookieState, selectSkipFrontdoorAuth, selectedOrgState } from '@jetstream/ui/app-state'; import { getDexieDb } from '@jetstream/ui/db'; import { useLiveQuery } from 'dexie-react-hooks'; import { useAtomValue } from 'jotai'; -import { Fragment, FunctionComponent, useEffect, useMemo, useState } from 'react'; +import { Fragment, FunctionComponent, useEffect, useMemo, useRef, useState } from 'react'; import { Link, useSearchParams } from 'react-router'; import { PermissionAnalysisExportGrid } from './PermissionAnalysisExportGrid'; import { PermissionAnalysisFieldPermissionsTree } from './PermissionAnalysisFieldPermissionsTree'; @@ -229,6 +231,17 @@ export const PermissionAnalysisView: FunctionComponent = () => { const isTerminal = jobStatusNormalized === 'completed' || jobStatusNormalized === 'failed'; const fetchError = decodeError; + + // The progress block is replaced by the results silently (failures already toast) — announce the + // completion to the user who watched the analysis run + const { announce, announcer } = useAnnouncer(); + const previousJobStatusRef = useRef(jobStatusNormalized); + useEffect(() => { + if (previousJobStatusRef.current === 'running' && jobStatusNormalized === 'completed') { + announce('Permission analysis complete. Results are ready.'); + } + previousJobStatusRef.current = jobStatusNormalized; + }, [jobStatusNormalized, announce]); const terminalErrorMessage = historyRow?.errorMessage ?? inFlightJob?.statusMessage ?? null; const liveProgress = inFlightJob?.progress; @@ -1031,6 +1044,7 @@ export const PermissionAnalysisView: FunctionComponent = () => { className="slds-scrollable_none" bufferIfNotRendered={HEIGHT_BUFFER} > + {announcer} {!jobId && (
@@ -1047,6 +1061,8 @@ export const PermissionAnalysisView: FunctionComponent = () => { {jobId && !fetchError && !isTerminal && (

Permission analysis in progress…

+ {/* Announce the phase only: a live region on the step counter would speak every tick of a long analysis */} +

{isJobRunning && liveProgress?.label ? liveProgress.label : 'Preparing'} {isJobRunning && liveProgress && liveProgress.total > 0 @@ -1054,6 +1070,7 @@ export const PermissionAnalysisView: FunctionComponent = () => { : ''}

diff --git a/libs/features/permission-analysis/src/permission-export-result-view-modules/__tests__/export-result-findings.spec.tsx b/libs/features/permission-analysis/src/permission-export-result-view-modules/__tests__/export-result-findings.spec.tsx new file mode 100644 index 000000000..0505de8c3 --- /dev/null +++ b/libs/features/permission-analysis/src/permission-export-result-view-modules/__tests__/export-result-findings.spec.tsx @@ -0,0 +1,45 @@ +import type { ColumnWithFilter } from '@jetstream/ui'; +import { render, screen } from '@testing-library/react'; +import type { ReactNode } from 'react'; +import { describe, expect, test } from 'vitest'; +import { withFindingDetailsCell } from '../export-result-findings'; + +type Row = { Id: string; severity?: 'error' | 'warning' }; + +function renderCellOf(column: ColumnWithFilter, row: Row) { + const renderCell = column.renderCell as unknown as (props: { row: Row; column: unknown }) => ReactNode; + return render(
{renderCell({ row, column })}
); +} + +describe('withFindingDetailsCell', () => { + const baseColumn = { + key: 'Id', + renderCell: ({ row }: { row: Row }) => {row.Id}, + } as unknown as ColumnWithFilter; + + test('adds a details button, hidden until focused, next to the cell content when the row has a finding', () => { + const column = withFindingDetailsCell(baseColumn, (row) => row.severity, { columnLabel: 'Id' }); + renderCellOf(column, { Id: '001', severity: 'error' }); + + expect(screen.getByRole('link', { name: '001' })).toBeTruthy(); + const button = screen.getByRole('button', { name: 'View error details for Id' }); + expect(button.className).toContain('slds-assistive-text'); + expect(button.className).toContain('slds-assistive-text_focus'); + expect(button.getAttribute('type')).toBe('button'); + }); + + test('renders only the cell content when the row has no finding', () => { + const column = withFindingDetailsCell(baseColumn, (row) => row.severity); + renderCellOf(column, { Id: '002' }); + + expect(screen.getByRole('link', { name: '002' })).toBeTruthy(); + expect(screen.queryByRole('button')).toBeNull(); + }); + + test("merges the severity cell class with the column's own class", () => { + const column = withFindingDetailsCell({ ...baseColumn, cellClass: 'my-cell' } as ColumnWithFilter, (row) => row.severity); + const cellClass = column.cellClass as (row: Row) => string | undefined; + expect(cellClass({ Id: '003', severity: 'warning' })).toContain('my-cell'); + expect(cellClass({ Id: '003' })).toBe('my-cell'); + }); +}); diff --git a/libs/features/permission-analysis/src/permission-export-result-view-modules/export-result-findings.ts b/libs/features/permission-analysis/src/permission-export-result-view-modules/export-result-findings.tsx similarity index 87% rename from libs/features/permission-analysis/src/permission-export-result-view-modules/export-result-findings.ts rename to libs/features/permission-analysis/src/permission-export-result-view-modules/export-result-findings.tsx index 7b15fc8dc..565d160f2 100644 --- a/libs/features/permission-analysis/src/permission-export-result-view-modules/export-result-findings.ts +++ b/libs/features/permission-analysis/src/permission-export-result-view-modules/export-result-findings.tsx @@ -1,4 +1,6 @@ import { getPermissionExportFindingDefinition, PermissionExportFindingCode } from '@jetstream/shared/constants'; +import type { ColumnWithFilter } from '@jetstream/ui'; +import type { ReactNode } from 'react'; import { FIELD_PERMISSION_BOOLEAN_COLUMN_KEYS, @@ -298,6 +300,64 @@ export function fieldPermissionCellSeverity( return fromSpecific ?? fromScope; } +/** `cellClass` for a highlighted issue cell; `--clickable` also opts the cell into the click delegate that opens the modal. */ +const FINDING_CELL_CLASS_BY_SEVERITY: Record = { + error: 'permission-finding-cell--error permission-finding-cell--clickable', + warning: 'permission-finding-severity-cell--warning permission-finding-cell--clickable', +}; + +interface WithFindingDetailsCellOptions { + /** Column header label appended to the sr-only button text (`View error details for {columnLabel}`). */ + columnLabel?: string; +} + +/** + * Decorates a grid column so rows with an issue get the severity highlight `cellClass` (merged with any + * class the column already computes) plus an accessible path to the issue details. + * + * Finding cells were mouse-only (a DOM click delegate opens the modal): an sr-only button gives + * keyboard/SR users the same path — grid Enter clicks it, the click bubbles to the same delegate, + * and the cell announces that details are available. + */ +export function withFindingDetailsCell( + column: ColumnWithFilter, + severityForRow: (row: TRow) => PermissionObjectFindingCellSeverity | undefined, + { columnLabel }: WithFindingDetailsCellOptions = {}, +): ColumnWithFilter { + const priorCellClass = column.cellClass; + const priorRenderCell = column.renderCell; + return { + ...column, + cellClass: (row: TRow) => { + const prior = typeof priorCellClass === 'function' ? priorCellClass(row) : priorCellClass; + const severity = severityForRow(row); + const merged = [prior, severity ? FINDING_CELL_CLASS_BY_SEVERITY[severity] : undefined].filter(Boolean).join(' '); + return merged.length > 0 ? merged : undefined; + }, + renderCell: (props) => { + const base = priorRenderCell + ? priorRenderCell(props) + : (column.getValue?.({ row: props.row, column: props.column }) ?? + ((props.row as Record)[String(column.key)] as ReactNode) ?? + null); + const severity = severityForRow(props.row); + if (!severity) { + return base; + } + return ( + <> + {base} + {/* Hidden until focused: Id cells also hold the record lookup trigger, so a keyboard user + stepping through the cell's controls must be able to see this one */} + + + ); + }, + }; +} + /** * Max severity per permission-set container Id for profile / permission-set / assignment export rows. */ diff --git a/libs/features/platform-event-monitor/src/PlatformEventMonitorEvent.tsx b/libs/features/platform-event-monitor/src/PlatformEventMonitorEvent.tsx index 523bb57d6..cb5cae5bf 100644 --- a/libs/features/platform-event-monitor/src/PlatformEventMonitorEvent.tsx +++ b/libs/features/platform-event-monitor/src/PlatformEventMonitorEvent.tsx @@ -12,7 +12,11 @@ export const PlatformEventMonitorEvent: FunctionComponent
- + UUID: {event.EventUuid} - Replay Id: {event.replayId}
diff --git a/libs/features/platform-event-monitor/src/PlatformEventMonitorEvents.tsx b/libs/features/platform-event-monitor/src/PlatformEventMonitorEvents.tsx
index 436d9bf01..487b8b052 100644
--- a/libs/features/platform-event-monitor/src/PlatformEventMonitorEvents.tsx
+++ b/libs/features/platform-event-monitor/src/PlatformEventMonitorEvents.tsx
@@ -1,10 +1,10 @@
 import { css } from '@emotion/react';
 import { logger } from '@jetstream/shared/client-logger';
-import { setItemInLocalStorage } from '@jetstream/shared/ui-utils';
-import { orderValues } from '@jetstream/shared/utils';
+import { formatNumber, setItemInLocalStorage } from '@jetstream/shared/ui-utils';
+import { orderValues, pluralizeIfMultiple } from '@jetstream/shared/utils';
 import { ContextMenuItem } from '@jetstream/types';
 import type { RenderCellProps, RowHeightArgs, SortColumn } from '@jetstream/ui';
-import { AutoFullHeightContainer, ColumnWithFilter, ContextMenuActionData, DataTree } from '@jetstream/ui';
+import { AssistiveStatus, AutoFullHeightContainer, ColumnWithFilter, ContextMenuActionData, DataTree } from '@jetstream/ui';
 import { STORAGE_KEYS } from '@jetstream/ui/app-state';
 import copyToClipboard from 'copy-to-clipboard';
 import groupBy from 'lodash/groupBy';
@@ -70,6 +70,9 @@ const columns: ColumnWithFilter[] = [
 
 const groupedRows = ['event'] as const;
 
+/** Long enough that a burst of events announces once instead of per message */
+const EVENT_COUNT_ANNOUNCE_DEBOUNCE_MS = 1000;
+
 function getRowId(data: PlatformEventRow): string {
   return data.uuid || `${data.replayId}` || JSON.stringify(data);
 }
@@ -151,6 +154,12 @@ export const PlatformEventMonitorEvents: FunctionComponent
+      {/* Events stream in continuously; announcing each one would be unusable, so announce the
+          running total once a burst settles. The grid itself carries the row count via aria-rowcount. */}
+      
       
-          
+          
+            
+          
           
             
           
           
@@ -70,7 +82,13 @@ export const PlatformEventMonitorListenerCard = ({
               disabled={!hasSubscriptions}
               onClick={() => onClearEvents()}
             >
-              
+              
             
           
           
+                    
+                  
+                )}
+                {!sobjectDescribeData.describe.fields.length && (
+                  
+ This platform event does not have any custom fields. + + + +
+ )} + + )}
- {publishEventResponse && publishEventResponse.success && ( -
- Event Id: {publishEventResponse.eventId} -
- )} - {publishEventResponse && !publishEventResponse.success && ( -
- - There was an error publishing your event: -

{publishEventResponse.errorMessage}

-
-
- )} - {sobjectDescribeError && ( -
- - There was a problem loading the fields for the event: -

{sobjectDescribeErrorMsg}

-
-
- )} -
- {sobjectDescribeLoaded && sobjectDescribeData && ( - - {!!sobjectDescribeData.describe.fields.length && ( - - - - - - - )} - {!sobjectDescribeData.describe.fields.length && ( -
- This platform event does not have any custom fields. - - - -
- )} -
- )} -
- + ); }; diff --git a/libs/features/platform-event-monitor/src/PlatformEventMonitorSubscribe.tsx b/libs/features/platform-event-monitor/src/PlatformEventMonitorSubscribe.tsx index 2f94944cc..3fc36eb45 100644 --- a/libs/features/platform-event-monitor/src/PlatformEventMonitorSubscribe.tsx +++ b/libs/features/platform-event-monitor/src/PlatformEventMonitorSubscribe.tsx @@ -1,8 +1,8 @@ import { css } from '@emotion/react'; import { isEnterKey } from '@jetstream/shared/ui-utils'; import { ListItemGroup, Maybe } from '@jetstream/types'; -import { ComboboxWithGroupedItems, Grid, Input } from '@jetstream/ui'; -import React, { FunctionComponent, KeyboardEvent, useEffect, useState } from 'react'; +import { AssistiveStatus, ComboboxWithGroupedItems, Grid, Input } from '@jetstream/ui'; +import React, { FunctionComponent, KeyboardEvent, useEffect, useRef, useState } from 'react'; import { PlatformEventObject } from './platform-event-monitor.types'; import { MessagesByChannel } from './usePlatformEvent'; @@ -29,9 +29,19 @@ export const PlatformEventMonitorSubscribe: FunctionComponent { const [replayId, setReplayId] = useState('-1'); const [currentEventSubscribed, setCurrentEventSubscribed] = useState(false); + const [subscriptionStatusMessage, setSubscriptionStatusMessage] = useState(''); + const previousSubscriptionRef = useRef<{ channel: Maybe; subscribed: boolean } | null>(null); + // Subscribe / Unsubscribe only swap the button label, which is not announced — announce the outcome + // when the selected event's subscription state flips (selecting a different event is not an outcome) useEffect(() => { - setCurrentEventSubscribed(!!selectedSubscribeEvent && !!messagesByChannel[selectedSubscribeEvent]); + const subscribed = !!selectedSubscribeEvent && !!messagesByChannel[selectedSubscribeEvent]; + setCurrentEventSubscribed(subscribed); + const previous = previousSubscriptionRef.current; + if (previous && selectedSubscribeEvent && previous.channel === selectedSubscribeEvent && previous.subscribed !== subscribed) { + setSubscriptionStatusMessage(subscribed ? `Subscribed to ${selectedSubscribeEvent}` : `Unsubscribed from ${selectedSubscribeEvent}`); + } + previousSubscriptionRef.current = { channel: selectedSubscribeEvent, subscribed }; }, [selectedSubscribeEvent, messagesByChannel]); useEffect(() => { @@ -104,20 +114,22 @@ export const PlatformEventMonitorSubscribe: FunctionComponent - {currentEventSubscribed && ( - - )} - {!currentEventSubscribed && ( - - )} + {/* One stable element for both states — swapping two buttons dropped keyboard focus to + the moment the subscription state flipped */} + +
); diff --git a/libs/features/query/src/QueryBuilder/ExecuteQueryButton.tsx b/libs/features/query/src/QueryBuilder/ExecuteQueryButton.tsx index 6f911e55a..4396bc1a2 100644 --- a/libs/features/query/src/QueryBuilder/ExecuteQueryButton.tsx +++ b/libs/features/query/src/QueryBuilder/ExecuteQueryButton.tsx @@ -1,5 +1,5 @@ import { DescribeGlobalSObjectResult, Maybe, SalesforceOrgUi } from '@jetstream/types'; -import { Icon, KeyboardShortcut, Tooltip, getModifierKey } from '@jetstream/ui'; +import { getAriaKeyshortcuts, getModifierKey, Icon, KeyboardShortcut, Tooltip } from '@jetstream/ui'; import { recentHistoryItemsDb } from '@jetstream/ui/db'; import { FunctionComponent } from 'react'; import { Link } from 'react-router'; @@ -41,6 +41,7 @@ export const ExecuteQueryButton: FunctionComponent = ({ name: selectedSObject.name, }, }} + aria-keyshortcuts={getAriaKeyshortcuts([getModifierKey(), 'enter'])} data-testid="execute-query-button" onClick={handleClick} > diff --git a/libs/features/query/src/QueryBuilder/QuerySubqueryConfigPanel.tsx b/libs/features/query/src/QueryBuilder/QuerySubqueryConfigPanel.tsx index e88cda746..ae6eab9e6 100644 --- a/libs/features/query/src/QueryBuilder/QuerySubqueryConfigPanel.tsx +++ b/libs/features/query/src/QueryBuilder/QuerySubqueryConfigPanel.tsx @@ -3,7 +3,7 @@ import { logger } from '@jetstream/shared/client-logger'; import { fetchFields, getListItemsFromFieldWithRelatedItems, sortQueryFields, unFlattenedListItemsById } from '@jetstream/shared/ui-utils'; import { groupByFlat } from '@jetstream/shared/utils'; import { ExpressionType, Field, ListItem, QueryFields, QueryOrderByClause, SalesforceOrgUi } from '@jetstream/types'; -import { Panel, Spinner } from '@jetstream/ui'; +import { ariaDisabledButtonProps, Panel, Spinner } from '@jetstream/ui'; import { fromQueryState } from '@jetstream/ui-core'; import { getSubqueryFieldBaseKey, removeInFlightQueryFields } from '@jetstream/ui-core/shared'; import { useAtom, useAtomValue, useSetAtom } from 'jotai'; @@ -300,12 +300,12 @@ export const QuerySubqueryConfigPanel: FunctionComponent + {/* Stays focusable while its own click disables it — native disabled would drop focus to */} diff --git a/libs/features/query/src/QueryBuilder/QuerySubqueryLevel.tsx b/libs/features/query/src/QueryBuilder/QuerySubqueryLevel.tsx index f6be8eca5..bb4a9127c 100644 --- a/libs/features/query/src/QueryBuilder/QuerySubqueryLevel.tsx +++ b/libs/features/query/src/QueryBuilder/QuerySubqueryLevel.tsx @@ -439,6 +439,9 @@ export const QuerySubqueryLevel: FunctionComponent = ({ initOpenIds={focusedSectionId ? [focusedSectionId] : []} scrollInitOpenIdIntoView allowMultiple={false} + // An object can have dozens of child relationships — one tab stop with ArrowUp/ArrowDown + // between headers keeps the rest of the page reachable by keyboard + singleTabStop sections={visibleChildRelationships.map((childRelationship) => ({ id: getSectionId(currentRelationshipPath, childRelationship), testId: childRelationship.relationshipName, diff --git a/libs/features/query/src/QueryBuilder/QuerySubquerySObjects.tsx b/libs/features/query/src/QueryBuilder/QuerySubquerySObjects.tsx index fd68fff8b..61d16e63b 100644 --- a/libs/features/query/src/QueryBuilder/QuerySubquerySObjects.tsx +++ b/libs/features/query/src/QueryBuilder/QuerySubquerySObjects.tsx @@ -6,7 +6,7 @@ import { fromQueryState } from '@jetstream/ui-core'; import { getSubqueryFieldBaseKey } from '@jetstream/ui-core/shared'; import { useAtomValue, useSetAtom } from 'jotai'; import isNumber from 'lodash/isNumber'; -import { Fragment, FunctionComponent, ReactNode, useCallback, useEffect, useMemo, useRef, useState } from 'react'; +import { FunctionComponent, ReactNode, useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { buildSelectedSubqueryTree, countSubqueriesBelow, @@ -109,11 +109,16 @@ export const QuerySubquerySObjects: FunctionComponent. */ + const containerRef = useRef(null); function handleClearAll() { clearSubqueries(currentRelationshipPath); goToLevels((currentLevels) => currentLevels.slice(0, -1)); + window.setTimeout(() => { + containerRef.current?.querySelector('button, a[href], input, [tabindex="0"]')?.focus(); + }); } /** @@ -159,7 +164,8 @@ export const QuerySubquerySObjects: FunctionComponent + // display: contents keeps the wrapper out of the layout; it only scopes the focus fallback below +
{levels.length > 0 && (
goToLevels((currentLevels) => [...currentLevels, level])} /> )} - +
); }; diff --git a/libs/features/query/src/QueryOptions/ManualSoql.tsx b/libs/features/query/src/QueryOptions/ManualSoql.tsx index ffabdfbdd..49c13cf51 100644 --- a/libs/features/query/src/QueryOptions/ManualSoql.tsx +++ b/libs/features/query/src/QueryOptions/ManualSoql.tsx @@ -188,6 +188,8 @@ export const ManualSoql: FunctionComponent = ({ className, isTo > [ ...prevItems, @@ -59,7 +61,15 @@ export const QueryFieldFunction = ({ hasGroupByClause, selectedFields }: QueryFi ]); } + /** + * The delete button unmounts with its row, which would drop keyboard focus to — land on + * the previous row's delete button (or the row that slides into slot 0, which is always present + * because an emptied list is refilled with one blank row) + */ function handleDeleteRow(index: number) { + window.setTimeout(() => { + document.getElementById(`${DELETE_ROW_BUTTON_ID_PREFIX}${Math.max(index - 1, 0)}`)?.focus(); + }); setFieldFilterFunctions((prevItems) => { const output = prevItems.filter((_, i) => i !== index); if (!output.length) { @@ -89,7 +99,12 @@ export const QueryFieldFunction = ({ hasGroupByClause, selectedFields }: QueryFi onChange={(selectedField, selectedFunction, alias) => handleChange(i, selectedField, selectedFunction, alias)} /> - diff --git a/libs/features/query/src/QueryOptions/QueryFieldFunctionRow.tsx b/libs/features/query/src/QueryOptions/QueryFieldFunctionRow.tsx index ccf78372a..55393b5d8 100644 --- a/libs/features/query/src/QueryOptions/QueryFieldFunctionRow.tsx +++ b/libs/features/query/src/QueryOptions/QueryFieldFunctionRow.tsx @@ -105,6 +105,7 @@ export const QueryFieldFunctionRow = ({ } buttonProps={{ className: classNames('slds-button slds-button_icon slds-button_icon-border-filled', { 'slds-is-selected': !!alias }), + 'aria-label': 'Set field alias', disabled: !selectedField, }} > diff --git a/libs/features/query/src/QueryOptions/QueryGroupBy.tsx b/libs/features/query/src/QueryOptions/QueryGroupBy.tsx index 0b36598ba..8fdf843bc 100644 --- a/libs/features/query/src/QueryOptions/QueryGroupBy.tsx +++ b/libs/features/query/src/QueryOptions/QueryGroupBy.tsx @@ -1,8 +1,8 @@ import { ListItem, QueryGroupByClause } from '@jetstream/types'; -import { Icon } from '@jetstream/ui'; +import { Icon, ariaDisabledButtonProps } from '@jetstream/ui'; import { fromQueryState } from '@jetstream/ui-core'; import { useAtom } from 'jotai'; -import { Fragment, useState } from 'react'; +import { useRef, useState } from 'react'; import QueryGroupByRow from './QueryGroupByRow'; export interface QueryGroupByContainerProps { @@ -14,6 +14,8 @@ export interface QueryGroupByContainerProps { export const QueryGroupByContainer = ({ sobject, fields, onLoadRelatedFields }: QueryGroupByContainerProps) => { const [groupByClauses, setGroupByClauses] = useAtom(fromQueryState.queryGroupByState); const [nextKey, setNextKey] = useState(1); + // Scoped to this instance so a second mount of the component can never receive the focus + const containerRef = useRef(null); function handleUpdate(groupBy: QueryGroupByClause) { setGroupByClauses(groupByClauses.map((currItem) => (currItem.key === groupBy.key ? groupBy : currItem))); @@ -25,6 +27,15 @@ export const QueryGroupByContainer = ({ sobject, fields, onLoadRelatedFields }: } function handleDelete(deletedGroupBy: QueryGroupByClause) { + // The delete button unmounts with its row, which would drop keyboard focus to — land on + // the previous row's delete button (row 0 always exists: an emptied list is refilled with one row) + const deletedIndex = groupByClauses.findIndex((groupBy) => groupBy.key === deletedGroupBy.key); + window.setTimeout(() => { + const deleteButtons = containerRef.current?.querySelectorAll( + '[role="group"][aria-label^="Group by row "] button[title="Delete Condition"]', + ); + deleteButtons?.[Math.max(deletedIndex - 1, 0)]?.focus(); + }); const tempGroupByClauses = groupByClauses.filter((groupBy) => groupBy.key !== deletedGroupBy.key); // ensure there is always at least one group by if (tempGroupByClauses.length === 0) { @@ -35,7 +46,7 @@ export const QueryGroupByContainer = ({ sobject, fields, onLoadRelatedFields }: } return ( - +
{groupByClauses.map((groupBy, i) => ( ))}
-
- +
); }; diff --git a/libs/features/query/src/QueryOptions/QueryGroupByRow.tsx b/libs/features/query/src/QueryOptions/QueryGroupByRow.tsx index 208b16a9f..d01f78a34 100644 --- a/libs/features/query/src/QueryOptions/QueryGroupByRow.tsx +++ b/libs/features/query/src/QueryOptions/QueryGroupByRow.tsx @@ -32,7 +32,7 @@ export const QueryGroupByRow: FunctionComponent = ({ ); return ( -
+
{/* Resource */} [] = [ export const QueryOrderByContainer: FunctionComponent = React.memo( ({ sobject, fields, orderByClauses, setOrderByClauses, onLoadRelatedFields }) => { + // The same component is mounted in the query builder AND the subquery panel, so the focus target + // must be looked up inside this instance, not the document + const containerRef = useRef(null); function getNextKey(clauses: QueryOrderByClause[]) { return clauses.reduce((max, clause) => Math.max(max, clause.key), -1) + 1; } @@ -38,6 +41,15 @@ export const QueryOrderByContainer: FunctionComponent — land on + // the previous row's delete button (row 0 always exists: an emptied list is refilled with one row) + const deletedIndex = orderByClauses.findIndex((orderBy) => orderBy.key === deletedOrderby.key); + window.setTimeout(() => { + const deleteButtons = containerRef.current?.querySelectorAll( + '[role="group"][aria-label^="Order by row "] button[title="Delete Condition"]', + ); + deleteButtons?.[Math.max(deletedIndex - 1, 0)]?.focus(); + }); const tempOrderByClauses = orderByClauses.filter((orderBy) => orderBy.key !== deletedOrderby.key); // ensure there is always at least one order by if (tempOrderByClauses.length === 0) { @@ -47,7 +59,7 @@ export const QueryOrderByContainer: FunctionComponent +
{orderByClauses.map((orderBy, i) => ( ))}
-
- +
); }, ); diff --git a/libs/features/query/src/QueryOptions/QueryOrderByRow.tsx b/libs/features/query/src/QueryOptions/QueryOrderByRow.tsx index 5a5821519..9dada4404 100644 --- a/libs/features/query/src/QueryOptions/QueryOrderByRow.tsx +++ b/libs/features/query/src/QueryOptions/QueryOrderByRow.tsx @@ -34,7 +34,7 @@ export const QueryOrderByRow: FunctionComponent = ({ const [initialSelectedNulls] = useState(nulls.find((item) => item.value === orderBy.nulls) || nulls[0]); return ( -
+
{/* Resource */}
= ({ cla return ( + {/* The form's onSubmit owns the save (a click here submits the form) — the guarded click + only blocks the submit while disabled, otherwise the save would fire twice per click */} + {/* Both stay focusable while their own click disables them — native disabled would drop focus to */} {mode === 'configure' && ( @@ -392,8 +395,7 @@ export const BulkUpdateFromQueryModal: FunctionComponent handleCommit())} > Update {formatNumber(impactedRecordCount)} {pluralizeFromNumber('Record', impactedRecordCount)} @@ -431,6 +433,7 @@ export const BulkUpdateFromQueryModal: FunctionComponent diff --git a/libs/features/query/src/QueryResults/QueryResults.tsx b/libs/features/query/src/QueryResults/QueryResults.tsx index 3448245fd..b450f7d5c 100644 --- a/libs/features/query/src/QueryResults/QueryResults.tsx +++ b/libs/features/query/src/QueryResults/QueryResults.tsx @@ -54,6 +54,7 @@ import { ToolbarItemGroup, Tooltip, buildResultsExport, + getAriaKeyshortcuts, getModifierKey, useConfirmation, } from '@jetstream/ui'; @@ -125,6 +126,7 @@ export const QueryResults = React.memo(() => { sobject?: { name: string; label: string }; }>(); const [soqlPanelOpen, setSoqlPanelOpen] = useState(false); + const soqlPanelButtonRef = useRef(null); const [soql, setSoql] = useState(''); const [sobject, setSobject] = useState>(null); const [parsedQuery, setParsedQuery] = useState>(null); @@ -726,6 +728,7 @@ export const QueryResults = React.memo(() => { className="slds-button slds-button_brand slds-m-right_x-small" to={{ pathname: APP_ROUTES.QUERY.ROUTE, search: APP_ROUTES.QUERY.SEARCH_PARAM }} state={{ soql }} + aria-keyshortcuts={getAriaKeyshortcuts([getModifierKey(), 'shift', 'enter'])} > Back @@ -733,6 +736,7 @@ export const QueryResults = React.memo(() => { -
+ } > - - Reload - + + @@ -806,6 +820,7 @@ export const QueryResults = React.memo(() => { isOpen={soqlPanelOpen} selectedOrg={selectedOrg} sObject={allowContentDownload.sobjectName || ''} + returnFocusTo={soqlPanelButtonRef} onClosed={() => setSoqlPanelOpen(false)} executeQuery={(soql, tooling) => executeQuery(soql, SOURCE_MANUAL, { isTooling: tooling })} onOpenHistory={handleOpenHistory} diff --git a/libs/features/query/src/QueryResults/QueryResultsAttachmentDownload.tsx b/libs/features/query/src/QueryResults/QueryResultsAttachmentDownload.tsx index f2a91310d..eb5c7f7fd 100644 --- a/libs/features/query/src/QueryResults/QueryResultsAttachmentDownload.tsx +++ b/libs/features/query/src/QueryResults/QueryResultsAttachmentDownload.tsx @@ -18,7 +18,7 @@ import { } from '@jetstream/shared/ui-utils'; import { getErrorMessage, getRecordIdFromAttributes, pluralizeIfMultiple } from '@jetstream/shared/utils'; import { AsyncJobNew, BinaryDownloadCompatibleObjectsSchema, Maybe, SalesforceOrgUi, SalesforceRecord } from '@jetstream/types'; -import { Icon, Modal, Radio, RadioGroup, ScopedNotification, Tooltip } from '@jetstream/ui'; +import { ariaDisabledButtonProps, Icon, Modal, Radio, RadioGroup, ScopedNotification, Tooltip } from '@jetstream/ui'; import { fromJetstreamEvents, useAmplitude } from '@jetstream/ui-core'; import { Fragment, FunctionComponent, useEffect, useState } from 'react'; import z from 'zod'; @@ -264,7 +264,8 @@ export const QueryResultsAttachmentDownload: FunctionComponent handleModalClose(true)} disabled={isDownloading}> Cancel - diff --git a/libs/features/query/src/QueryResults/QueryResultsGetRecAsApexGenerateOptions.tsx b/libs/features/query/src/QueryResults/QueryResultsGetRecAsApexGenerateOptions.tsx index 5d17684da..6d156f682 100644 --- a/libs/features/query/src/QueryResults/QueryResultsGetRecAsApexGenerateOptions.tsx +++ b/libs/features/query/src/QueryResults/QueryResultsGetRecAsApexGenerateOptions.tsx @@ -90,6 +90,7 @@ export const QueryResultsGetRecAsApexGenerateOptions: FunctionComponent
+ after + `); + + focusNextTabbableAfter(editor); + expect(document.activeElement?.id).toBe('after'); + }); + + it('ignores disabled and tabindex=-1 candidates', () => { + const editor = setup(` +
+ +
nope
+ + `); + + focusNextTabbableAfter(editor); + expect(document.activeElement?.id).toBe('after'); + }); + + it('reports when there is nothing after it rather than dropping focus to the body', () => { + const editor = setup(` + +
+ `); + const before = document.getElementById('before') as HTMLElement; + before.focus(); + + expect(focusNextTabbableAfter(editor)).toBe(false); + expect(document.activeElement).toBe(before); + }); + + it('is a no-op without an element', () => { + expect(focusNextTabbableAfter(null)).toBe(false); + }); +}); + +describe('focusPreviousTabbableBefore', () => { + afterEach(() => { + document.body.innerHTML = ''; + }); + + it('moves focus to the NEAREST tabbable before the widget, not the first on the page', () => { + document.body.innerHTML = ` + + +
+ + `; + + expect(focusPreviousTabbableBefore(document.getElementById('editor'))).toBe(true); + expect(document.activeElement?.id).toBe('near'); + }); + + it('skips tabbables inside the widget', () => { + document.body.innerHTML = ` + +
+ `; + + focusPreviousTabbableBefore(document.getElementById('editor')); + expect(document.activeElement?.id).toBe('before'); + }); + + it('reports when there is nothing before it', () => { + document.body.innerHTML = `
`; + expect(focusPreviousTabbableBefore(document.getElementById('editor'))).toBe(false); + }); +}); diff --git a/libs/shared/ui-utils/src/lib/hooks/__tests__/useKeyboardShortcuts.spec.ts b/libs/shared/ui-utils/src/lib/hooks/__tests__/useKeyboardShortcuts.spec.ts index ff5a9a6a1..fc5638626 100644 --- a/libs/shared/ui-utils/src/lib/hooks/__tests__/useKeyboardShortcuts.spec.ts +++ b/libs/shared/ui-utils/src/lib/hooks/__tests__/useKeyboardShortcuts.spec.ts @@ -100,3 +100,49 @@ describe('useGoBackShortcut', () => { expect(handler).not.toHaveBeenCalled(); }); }); + +describe('scope', () => { + function openModalDialog() { + const dialog = document.createElement('div'); + dialog.setAttribute('role', 'dialog'); + dialog.setAttribute('aria-modal', 'true'); + document.body.appendChild(dialog); + return dialog; + } + + test('a page action stays quiet while a modal dialog is open and fires again once it closes', () => { + const dialog = openModalDialog(); + const handler = vi.fn(); + renderHook(() => usePrimaryActionShortcut(handler)); + dispatchKeydown({ key: 'Enter', metaKey: true }); + expect(handler).not.toHaveBeenCalled(); + + dialog.remove(); + dispatchKeydown({ key: 'Enter', metaKey: true }); + expect(handler).toHaveBeenCalledTimes(1); + }); + + test('a dialog action keeps firing while its modal is open, and the page action behind it does not', () => { + const dialog = openModalDialog(); + const pageHandler = vi.fn(); + const dialogHandler = vi.fn(); + renderHook(() => usePrimaryActionShortcut(pageHandler)); + renderHook(() => usePrimaryActionShortcut(dialogHandler, { scope: 'dialog' })); + dispatchKeydown({ key: 'Enter', metaKey: true }); + expect(pageHandler).not.toHaveBeenCalled(); + expect(dialogHandler).toHaveBeenCalledTimes(1); + dialog.remove(); + }); + + test('the go-back shortcut applies the same scoping', () => { + const dialog = openModalDialog(); + const pageHandler = vi.fn(); + const dialogHandler = vi.fn(); + renderHook(() => useGoBackShortcut(pageHandler)); + renderHook(() => useGoBackShortcut(dialogHandler, { scope: 'dialog' })); + dispatchKeydown({ key: 'Enter', metaKey: true, shiftKey: true }); + expect(pageHandler).not.toHaveBeenCalled(); + expect(dialogHandler).toHaveBeenCalledTimes(1); + dialog.remove(); + }); +}); diff --git a/libs/shared/ui-utils/src/lib/hooks/useFuzzySearchFilter.ts b/libs/shared/ui-utils/src/lib/hooks/useFuzzySearchFilter.ts index c4948ae62..06805d9a5 100644 --- a/libs/shared/ui-utils/src/lib/hooks/useFuzzySearchFilter.ts +++ b/libs/shared/ui-utils/src/lib/hooks/useFuzzySearchFilter.ts @@ -13,7 +13,10 @@ const DEFAULT_OPTIONS: IFuseOptions = { export function useFuzzySearchFilter(items: T[], filter: string, options: IFuseOptions = DEFAULT_OPTIONS) { const fuse = useMemo(() => new Fuse(items, { ...options }), [items, options]); - const filterText = useDebounce(filter, 300); + const debouncedFilter = useDebounce(filter, 300); + // Clearing must apply immediately: a debounced reset made lists visibly "pop in" 300ms after + // the input was cleared or a previously-filtered dropdown was reopened + const filterText = filter ? debouncedFilter : ''; const [visibleItems, setVisibleItems] = useState(items); useEffect(() => { diff --git a/libs/shared/ui-utils/src/lib/hooks/useKeyboardShortcuts.ts b/libs/shared/ui-utils/src/lib/hooks/useKeyboardShortcuts.ts index a935409f3..21cec3b14 100644 --- a/libs/shared/ui-utils/src/lib/hooks/useKeyboardShortcuts.ts +++ b/libs/shared/ui-utils/src/lib/hooks/useKeyboardShortcuts.ts @@ -5,6 +5,17 @@ import { useGlobalEventHandler } from './useGlobalEventHandler'; interface KeyboardActionOptions { /** When true the shortcut is ignored — wire this to the same condition that disables the button */ disabled?: boolean; + /** + * Where the action lives. A `page` action (the default) stays quiet while a modal dialog is open: the + * page is inert behind it and its primary button is out of reach, so firing it from inside the modal + * (Cmd+Enter while reading a test result) would act on the page unseen. A `dialog` action belongs to + * the modal itself and keeps firing. + */ + scope?: 'page' | 'dialog'; +} + +function isModalDialogOpen() { + return !!document.querySelector('[role="dialog"][aria-modal="true"]'); } /** @@ -12,7 +23,7 @@ interface KeyboardActionOptions { * Ignores Shift so it never collides with the go-back shortcut, and skips events already handled by * a focused Monaco editor (which binds Cmd+Enter itself and stops propagation). */ -export function usePrimaryActionShortcut(handler: () => void, { disabled }: KeyboardActionOptions = {}) { +export function usePrimaryActionShortcut(handler: () => void, { disabled, scope = 'page' }: KeyboardActionOptions = {}) { const onKeydown = useCallback( (event: KeyboardEvent) => { if (disabled || event.defaultPrevented) { @@ -20,12 +31,15 @@ export function usePrimaryActionShortcut(handler: () => void, { disabled }: Keyb } const keyboardEvent = event as unknown as ReactKeyboardEvent; if (hasCtrlOrMeta(keyboardEvent) && !hasShiftModifierKey(keyboardEvent) && isEnterKey(keyboardEvent)) { + if (scope === 'page' && isModalDialogOpen()) { + return; + } event.stopPropagation(); event.preventDefault(); handler(); } }, - [disabled, handler], + [disabled, handler, scope], ); useGlobalEventHandler('keydown', onKeydown); } @@ -33,7 +47,7 @@ export function usePrimaryActionShortcut(handler: () => void, { disabled }: Keyb /** * Cmd+Shift+Enter (mac) / Ctrl+Shift+Enter — navigates back one step in a multi-step (wizard) flow. */ -export function useGoBackShortcut(handler: () => void, { disabled }: KeyboardActionOptions = {}) { +export function useGoBackShortcut(handler: () => void, { disabled, scope = 'page' }: KeyboardActionOptions = {}) { const onKeydown = useCallback( (event: KeyboardEvent) => { if (disabled || event.defaultPrevented) { @@ -41,12 +55,15 @@ export function useGoBackShortcut(handler: () => void, { disabled }: KeyboardAct } const keyboardEvent = event as unknown as ReactKeyboardEvent; if (hasCtrlOrMeta(keyboardEvent) && hasShiftModifierKey(keyboardEvent) && isEnterKey(keyboardEvent)) { + if (scope === 'page' && isModalDialogOpen()) { + return; + } event.stopPropagation(); event.preventDefault(); handler(); } }, - [disabled, handler], + [disabled, handler, scope], ); useGlobalEventHandler('keydown', onKeydown); } diff --git a/libs/shared/ui-utils/src/lib/shared-ui-keyboard.ts b/libs/shared/ui-utils/src/lib/shared-ui-keyboard.ts index 08ef259d2..219832a6d 100644 --- a/libs/shared/ui-utils/src/lib/shared-ui-keyboard.ts +++ b/libs/shared/ui-utils/src/lib/shared-ui-keyboard.ts @@ -180,7 +180,7 @@ export function isEnterKey(event: KeyboardEvent): boolean { } export function isSpaceKey(event: KeyboardEvent): boolean { - return event.keyCode === 32; + return event.key === ' ' || event.key === 'Spacebar' || event.keyCode === 32; } export function isEnterOrSpace(event: KeyboardEvent): boolean { diff --git a/libs/shared/ui-utils/src/lib/shared-ui-utils.ts b/libs/shared/ui-utils/src/lib/shared-ui-utils.ts index b5bc1f615..d4e8b9ca8 100644 --- a/libs/shared/ui-utils/src/lib/shared-ui-utils.ts +++ b/libs/shared/ui-utils/src/lib/shared-ui-utils.ts @@ -1936,3 +1936,58 @@ export function disposeEditorRefs(disposables: Maybe= 0; +} + +/** + * Moves focus to the nearest tabbable element outside `element`, forwards or backwards in document + * order, skipping anything inside it. Returns whether focus actually moved. + * + * For widgets that swallow Tab (a code editor), so they can offer an explicit "leave me" key in both + * directions without dropping focus to ``. Both directions matter: an editor that can only be + * left forwards is a one-way valve — everything before it becomes unreachable without cycling the + * whole page, because shift-tabbing back lands inside the editor again. + * + * Visibility is decided by attempting the focus and checking it took, rather than by a heuristic: + * `offsetParent` is also null for `position: fixed` elements, which are perfectly focusable. + */ +function focusAdjacentTabbable(element: HTMLElement | null | undefined, direction: 'forward' | 'backward'): boolean { + if (!element) { + return false; + } + const wanted = direction === 'forward' ? Node.DOCUMENT_POSITION_FOLLOWING : Node.DOCUMENT_POSITION_PRECEDING; + const candidates = Array.from(document.querySelectorAll(TABBABLE_SELECTOR)).filter( + // eslint-disable-next-line no-bitwise + (candidate) => !element.contains(candidate) && !!(element.compareDocumentPosition(candidate) & wanted), + ); + // Walking outwards from the element means the nearest candidate first, which for `backward` is the + // end of the list + if (direction === 'backward') { + candidates.reverse(); + } + for (const candidate of candidates) { + if (!isTabbable(candidate)) { + continue; + } + candidate.focus(); + if (document.activeElement === candidate) { + return true; + } + } + return false; +} + +/** Moves focus to the first tabbable element AFTER `element`. See {@link focusAdjacentTabbable}. */ +export function focusNextTabbableAfter(element: HTMLElement | null | undefined): boolean { + return focusAdjacentTabbable(element, 'forward'); +} + +/** Moves focus to the last tabbable element BEFORE `element`. See {@link focusAdjacentTabbable}. */ +export function focusPreviousTabbableBefore(element: HTMLElement | null | undefined): boolean { + return focusAdjacentTabbable(element, 'backward'); +} diff --git a/libs/test-utils/src/index.ts b/libs/test-utils/src/index.ts index 11410dec0..7cc65bf34 100644 --- a/libs/test-utils/src/index.ts +++ b/libs/test-utils/src/index.ts @@ -1 +1,2 @@ +export { axeScan } from './lib/a11y-test-utils'; export * as sfdcFieldsFactory from './lib/sfdc-fields.data-factory'; diff --git a/libs/test-utils/src/lib/a11y-test-utils.ts b/libs/test-utils/src/lib/a11y-test-utils.ts new file mode 100644 index 000000000..eafae3638 --- /dev/null +++ b/libs/test-utils/src/lib/a11y-test-utils.ts @@ -0,0 +1,68 @@ +import type { AxeResults, NodeResult } from 'axe-core'; +import { expect } from 'vitest'; +import { axe } from 'vitest-axe'; + +// Same scope as the Playwright sweep (apps/jetstream-e2e/src/tests/a11y/a11y.utils.ts) and +// scripts/a11y-scan-urls.mjs: WCAG 2.1 A/AA rules only, no axe best-practice rules. +const WCAG_21_AA_TAGS = ['wcag2a', 'wcag2aa', 'wcag21a', 'wcag21aa']; +const FOCUS_GUARD_ATTRIBUTE = 'data-floating-ui-focus-guard'; + +/** + * True only when the flagged node itself is a Floating UI focus guard. + * + * axe's `html` is the node's full outerHTML (for elements up to ~300 chars), so a substring test on + * it would also match a compact wrapper whose descendants include a guard and silently hide real + * violations on that wrapper (nested-interactive, aria-required-children, list, ...). Resolving the + * node's target selector back to the live element anchors the check to the node itself. + */ +function isFloatingUiFocusGuard(root: Element, { target }: NodeResult): boolean { + const [selector] = target; + if (typeof selector !== 'string') { + // Shadow DOM paths are arrays of selectors; nothing in the component library renders guards there. + return false; + } + try { + return root.ownerDocument.querySelector(selector)?.hasAttribute(FOCUS_GUARD_ATTRIBUTE) ?? false; + } catch { + // An unresolvable selector keeps the node — the filter must never hide a violation by accident. + return false; + } +} + +/** + * Run an axe-core scan for component tests, filtering out Floating UI's focus-guard sentinels. + * + * FloatingFocusManager renders visually-hidden `` focus guards + * with no accessible name, which trips axe's serious name-related rules (button-name / + * aria-command-name). The guards exist only to redirect focus at the edges of a floating element + * (focus never rests on them), so this is accepted library-internal noise, not a user-facing + * defect. vitest-axe's `axe()` does not expose axe's context exclude, so the guard nodes are + * filtered out of the results instead. + * + * Note: jsdom has no layout engine, so color-contrast checks come back `incomplete` rather than + * as violations — contrast is covered by the Playwright a11y sweep and manual audit instead. + * + * The scan asserts on its own: any remaining violation fails the test, so a bare `await axeScan(el)` + * is a complete assertion (the lint ratchet counts a spec as covered once it calls `axeScan(`). The + * filtered results are still returned for specs that want to inspect `incomplete` or `passes`. + * + * `knownViolations` names rule ids that are logged as open findings (e.g. `nested-interactive`, X4 in + * docs/accessibility/audit-2026/findings.md): they are left out of the assertion but stay in the + * returned `violations`, so a spec can still pin exactly which known rules it expects. + * + * Usage: + * await axeScan(baseElement); + * const results = await axeScan(baseElement, { knownViolations: ['nested-interactive'] }); + */ +export async function axeScan(element: Element, { knownViolations = [] }: { knownViolations?: string[] } = {}): Promise { + const results = await axe(element, { runOnly: { type: 'tag', values: WCAG_21_AA_TAGS } }); + const violations = results.violations + .map((violation) => ({ + ...violation, + nodes: violation.nodes.filter((node) => !isFloatingUiFocusGuard(element, node)), + })) + .filter(({ nodes }) => nodes.length > 0); + const unexpectedViolations = violations.filter(({ id }) => !knownViolations.includes(id)); + expect(unexpectedViolations, 'axe found WCAG 2.1 AA violations').toEqual([]); + return { ...results, violations }; +} diff --git a/libs/test-utils/src/test-setup-dom.ts b/libs/test-utils/src/test-setup-dom.ts index e79b08714..4040aa447 100644 --- a/libs/test-utils/src/test-setup-dom.ts +++ b/libs/test-utils/src/test-setup-dom.ts @@ -1,5 +1,16 @@ /* eslint-disable @typescript-eslint/no-empty-function */ /* eslint-disable @typescript-eslint/no-useless-constructor */ +import { vi } from 'vitest'; + +/** + * Component specs render real widgets (floating-ui popovers, 96-option comboboxes, virtualized grids) + * and many finish with an axe scan, which alone costs seconds. CI runs every project's suite + * concurrently via `nx run-many -t test`, so those renders contend for CPU and routinely blow + * Vitest's 5s default — as intermittent, unrelated-looking timeouts. Raise the floor once here rather + * than per test; it is still short enough to catch a genuine hang. + */ +vi.setConfig({ testTimeout: 20_000, hookTimeout: 20_000 }); + /** * Vitest setup for jsdom test environments that load `@dnd-kit/dom` (via `@dnd-kit/react`). * diff --git a/libs/test/e2e-utils/src/lib/pageObjectModels/DataHistoryPage.model.ts b/libs/test/e2e-utils/src/lib/pageObjectModels/DataHistoryPage.model.ts index 95f1f6c19..a51732d39 100644 --- a/libs/test/e2e-utils/src/lib/pageObjectModels/DataHistoryPage.model.ts +++ b/libs/test/e2e-utils/src/lib/pageObjectModels/DataHistoryPage.model.ts @@ -46,11 +46,15 @@ export class DataHistoryPage { /** * Download the first saved payload from the open detail modal. Each file kind renders its own - * `Download` button, which opens the shared file-download modal (`Download
)}
    - {sections.map((item) => { + {sections.map((item, index) => { const isOpen = openIds.has(item.id); let content = item.content; if (isFunction(item.content)) { @@ -133,10 +202,27 @@ export const Accordion: FunctionComponent = ({
-
+
setFocusedIndex(index) : undefined}> {content as ReactNode}
diff --git a/libs/ui/src/lib/accordion/__tests__/Accordion.spec.tsx b/libs/ui/src/lib/accordion/__tests__/Accordion.spec.tsx index 61e04bcbb..e3bae9373 100644 --- a/libs/ui/src/lib/accordion/__tests__/Accordion.spec.tsx +++ b/libs/ui/src/lib/accordion/__tests__/Accordion.spec.tsx @@ -1,3 +1,4 @@ +import { axeScan } from '@jetstream/test-utils'; import { UiSection } from '@jetstream/types'; import { fireEvent, render, screen } from '@testing-library/react'; import { Accordion } from '../Accordion'; @@ -151,3 +152,102 @@ describe('Accordion', () => { expect(scrolledElements).toHaveLength(0); }); }); + +describe('Accordion singleTabStop composite', () => { + function renderComposite({ singleTabStop = true, disabledIds = [] as string[], initOpenIds = [] as string[] } = {}) { + return render( + ({ + id, + title: id, + titleText: id, + disabled: disabledIds.includes(id), + content: , + }))} + />, + ); + } + + test('default mode keeps a tab stop per header', () => { + renderComposite({ singleTabStop: false }); + const headers = screen.getAllByRole('button', { name: /Contacts|Cases|Opportunities/ }); + headers.forEach((header) => expect(header.getAttribute('tabindex')).toBeNull()); + }); + + test('exactly one header is in the page tab order', () => { + renderComposite(); + expect(screen.getByRole('button', { name: 'Contacts' }).tabIndex).toBe(0); + expect(screen.getByRole('button', { name: 'Cases' }).tabIndex).toBe(-1); + expect(screen.getByRole('button', { name: 'Opportunities' }).tabIndex).toBe(-1); + }); + + test('the initially open section is the tab stop, so tabbing in lands where the user left off', () => { + renderComposite({ initOpenIds: ['Cases'] }); + expect(screen.getByRole('button', { name: 'Cases' }).tabIndex).toBe(0); + }); + + test('ArrowDown/ArrowUp move focus between headers and wrap at the ends', () => { + renderComposite(); + const contacts = screen.getByRole('button', { name: 'Contacts' }); + const cases = screen.getByRole('button', { name: 'Cases' }); + const opportunities = screen.getByRole('button', { name: 'Opportunities' }); + + contacts.focus(); + fireEvent.keyDown(contacts, { key: 'ArrowDown' }); + expect(document.activeElement).toBe(cases); + expect(cases.tabIndex).toBe(0); + expect(contacts.tabIndex).toBe(-1); + + fireEvent.keyDown(cases, { key: 'ArrowUp' }); + fireEvent.keyDown(contacts, { key: 'ArrowUp' }); + expect(document.activeElement).toBe(opportunities); + + fireEvent.keyDown(opportunities, { key: 'ArrowDown' }); + expect(document.activeElement).toBe(contacts); + }); + + test('Home and End jump to the first and last enabled headers', () => { + renderComposite(); + const contacts = screen.getByRole('button', { name: 'Contacts' }); + contacts.focus(); + fireEvent.keyDown(contacts, { key: 'End' }); + expect(document.activeElement).toBe(screen.getByRole('button', { name: 'Opportunities' })); + fireEvent.keyDown(document.activeElement as Element, { key: 'Home' }); + expect(document.activeElement).toBe(contacts); + }); + + test('disabled sections are skipped by arrow navigation', () => { + renderComposite({ disabledIds: ['Cases'] }); + const contacts = screen.getByRole('button', { name: 'Contacts' }); + contacts.focus(); + fireEvent.keyDown(contacts, { key: 'ArrowDown' }); + expect(document.activeElement).toBe(screen.getByRole('button', { name: 'Opportunities' })); + }); + + test('clicking a header toggles it and moves the roving tab stop there', () => { + renderComposite(); + const cases = screen.getByRole('button', { name: 'Cases' }); + fireEvent.click(cases); + expect(cases.getAttribute('aria-expanded')).toBe('true'); + expect(cases.tabIndex).toBe(0); + expect(screen.getByRole('button', { name: 'Contacts' }).tabIndex).toBe(-1); + }); + + test('arrow keys inside an open section content are left to the content', () => { + renderComposite(); + fireEvent.click(screen.getByRole('button', { name: 'Contacts' })); + const embedded = screen.getByRole('button', { name: 'Contacts embedded control' }); + embedded.focus(); + fireEvent.keyDown(embedded, { key: 'ArrowDown' }); + expect(document.activeElement).toBe(embedded); + }); + + test('has no axe violations', async () => { + const { baseElement } = renderComposite({ initOpenIds: ['Contacts'] }); + const results = await axeScan(baseElement); + expect(results.violations).toEqual([]); + }); +}); diff --git a/libs/ui/src/lib/card/Card.tsx b/libs/ui/src/lib/card/Card.tsx index 7c79ade60..8eaa19b3a 100644 --- a/libs/ui/src/lib/card/Card.tsx +++ b/libs/ui/src/lib/card/Card.tsx @@ -9,11 +9,15 @@ import Icon from '../widgets/Icon'; export interface CardProps { testId?: string; + /** Accessible name for the card region — announced when focus enters, giving repeated per-card actions context */ + ariaLabel?: string; className?: string; bodyClassName?: Maybe; title?: string | ReactNode; icon?: IconObj; actions?: ReactNode; + /** 'start' top-aligns the actions with a multi-line title (default: vertically centered) */ + actionsAlignment?: 'center' | 'start'; footer?: ReactNode; nestedBorder?: boolean; children?: ReactNode; @@ -24,10 +28,30 @@ export interface CardProps { * (e.x. ExpressionConditionRow) */ export const Card = forwardRef( - ({ testId, className, bodyClassName = 'slds-card__body_inner', title, icon, actions, footer, nestedBorder, children }, ref) => { + ( + { + testId, + ariaLabel, + className, + bodyClassName = 'slds-card__body_inner', + title, + icon, + actions, + actionsAlignment, + footer, + nestedBorder, + children, + }, + ref, + ) => { const titleContent = isString(title) ? {title} : title; return ( -
+
{title && (
( diff --git a/libs/ui/src/lib/data-table/PreviewChangesModal.tsx b/libs/ui/src/lib/data-table/PreviewChangesModal.tsx index cba07029d..91897d9f3 100644 --- a/libs/ui/src/lib/data-table/PreviewChangesModal.tsx +++ b/libs/ui/src/lib/data-table/PreviewChangesModal.tsx @@ -2,6 +2,7 @@ import { css } from '@emotion/react'; import { formatNumber, hasCtrlOrMeta, isEnterKey, useGlobalEventHandler } from '@jetstream/shared/ui-utils'; import { Field, Maybe, SalesforceOrgUi, SobjectCollectionResponse } from '@jetstream/types'; import { ChangeEvent, Fragment, FunctionComponent, ReactNode, useCallback, useEffect, useMemo, useRef, useState } from 'react'; +import { ariaDisabledButtonProps } from '../form/button/aria-disabled-button.utils'; import Checkbox from '../form/checkbox/Checkbox'; import Input from '../form/input/Input'; import SearchInput from '../form/search-input/SearchInput'; @@ -179,6 +180,7 @@ function StatusRenderer({ row }: DataTableCellProps): ReactNode { icon="check" className="slds-icon slds-icon_xx-small slds-icon-text-success" containerClassname="slds-icon_container" + description="Saved" /> ); @@ -191,6 +193,7 @@ function StatusRenderer({ row }: DataTableCellProps): ReactNode { icon="success" className="slds-icon slds-icon_xx-small slds-icon-text-success" containerClassname="slds-icon_container" + description="Ready to save" /> ); @@ -212,6 +215,8 @@ function StatusRenderer({ row }: DataTableCellProps): ReactNode { icon={severity === 'warning' ? 'warning' : 'error'} className={`slds-icon slds-icon_xx-small ${severity === 'warning' ? 'slds-icon-text-warning' : 'slds-icon-text-error'}`} containerClassname="slds-icon_container" + // The icon is the whole Status cell, so it must carry the message a keyboard user cannot hover for + description={severity === 'warning' ? `Warnings: ${status}` : status} /> ); @@ -565,6 +570,15 @@ export const PreviewChangesModal: FunctionComponent = }); }; + // Save unmounts once everything is saved — hand focus to the (relabelled) Close button so it does + // not fall to + const closeButtonRef = useRef(null); + useEffect(() => { + if (allSaved && (document.activeElement === document.body || !document.activeElement)) { + closeButtonRef.current?.focus(); + } + }, [allSaved]); + function renderBanner() { if (allSaved) { return All changes were saved successfully.; @@ -664,18 +678,21 @@ export const PreviewChangesModal: FunctionComponent = Download Results )} - {!allSaved && ( - + // The reason rides on a Tooltip rather than a title: an aria-disabled button has + // pointer-events:none, so hover never reaches it, while the tooltip's own wrapper still does + + + )}
diff --git a/libs/ui/src/lib/data-table/SalesforceRecordDataTable.tsx b/libs/ui/src/lib/data-table/SalesforceRecordDataTable.tsx index 2e1f7bfdb..4f5328081 100644 --- a/libs/ui/src/lib/data-table/SalesforceRecordDataTable.tsx +++ b/libs/ui/src/lib/data-table/SalesforceRecordDataTable.tsx @@ -29,6 +29,7 @@ import { import uniqueId from 'lodash/uniqueId'; import { Fragment, ReactNode, memo, useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { FileDownloadModal } from '../file-download-modal/FileDownloadModal'; +import { ariaDisabledButtonProps } from '../form/button/aria-disabled-button.utils'; import SearchInput from '../form/search-input/SearchInput'; import Grid from '../grid/Grid'; import AutoFullHeightContainer from '../layout/AutoFullHeightContainer'; @@ -58,6 +59,7 @@ import { } from './data-table-utils'; import { DataTable } from './DataTable'; import { FieldMetadataModal } from './FieldMetadataModal'; +import { DEFAULT_ROW_HEIGHT } from './grid/grid-constants'; import { replaceSubqueryOnRecord } from './grid/grid-row-utils'; import { RowsChangeData } from './grid/rdg-compat'; import { getRowErrorMessages, mapSaveErrorsToRow, summarizeRowErrors, validateRow } from './grid/validate-cell-value'; @@ -825,6 +827,14 @@ export const SalesforceRecordDataTable = memo( // Salesforce reports the query as incomplete when child records were truncated, but the records are what say // what is actually missing - this also covers a complete set of parents whose related records were cut short. const hasRecordsToLoad = useMemo(() => hasMoreRecords || !!records?.some(hasIncompleteSubqueries), [hasMoreRecords, records]); + // "Load All Records" unmounts once everything is loaded; if focus was on it, hand it to the record filter + const wasLoadingMoreRef = useRef(false); + useEffect(() => { + if (wasLoadingMoreRef.current && !isLoadingMore && !hasRecordsToLoad && document.activeElement === document.body) { + document.getElementById('record-filter')?.focus(); + } + wasLoadingMoreRef.current = isLoadingMore; + }, [isLoadingMore, hasRecordsToLoad]); return records ? ( @@ -841,8 +851,7 @@ export const SalesforceRecordDataTable = memo( > }, + { + key: 'Url', + name: 'Url', + renderCell: ({ row }) => ( + + View in Salesforce + + ), + }, + { + key: 'Both', + name: 'Both', + renderCell: ({ row }) => ( + <> + + Open + + ), + }, +]; + +const data: Row[] = [ + { _key: '1', Name: 'Alpha', Amount: '10', Link: 'one', Url: 'https://example.com/1', Both: 'https://example.com/a' }, + { _key: '2', Name: 'Bravo', Amount: '20', Link: 'two', Url: 'https://example.com/2', Both: 'https://example.com/b' }, +]; + +// The virtualizers measure the scroll container, which jsdom reports as 0x0 — give every element a +// viewport-sized box so the grid mounts real rows. +beforeAll(() => { + for (const property of ['clientHeight', 'clientWidth', 'offsetHeight', 'offsetWidth'] as const) { + Object.defineProperty(HTMLElement.prototype, property, { configurable: true, value: 600 }); + } + HTMLElement.prototype.getBoundingClientRect = () => + ({ width: 800, height: 600, top: 0, left: 0, bottom: 600, right: 800, x: 0, y: 0, toJSON: () => ({}) }) as DOMRect; +}); + +function getCell(rowId: string, columnId: string): HTMLElement { + const cell = document.querySelector(`[data-row-id="${rowId}"][data-col-id="${columnId}"]`); + if (!cell) { + throw new Error(`No cell rendered for row ${rowId} / column ${columnId}`); + } + return cell; +} + +function describedByText(element: HTMLElement): string { + return (element.getAttribute('aria-describedby') ?? '') + .split(' ') + .filter(Boolean) + .map((id) => document.getElementById(id)?.textContent ?? ``) + .join(' '); +} + +/** Land keyboard focus on a cell: a click sets the active cell, an arrow key move then drives cell focus. */ +async function arrowTo(fromCell: HTMLElement, key: 'ArrowRight' | 'ArrowLeft', expectedCell: HTMLElement) { + fireEvent.keyDown(fromCell, { key }); + await waitFor(() => expect(document.activeElement).toBe(expectedCell)); +} + +describe('grid cell keyboard hints', () => { + test('a focused cell is described by what Enter does with it: edit, activate, open a link, or enter the cell', async () => { + const { baseElement } = render( row._key} />); + const nameCell = getCell('1', 'Name'); + const amountCell = getCell('1', 'Amount'); + const linkCell = getCell('1', 'Link'); + const urlCell = getCell('1', 'Url'); + const bothCell = getCell('1', 'Both'); + fireEvent.mouseDown(nameCell); + + await arrowTo(nameCell, 'ArrowRight', amountCell); + expect(describedByText(amountCell)).toMatch(/editable\. press enter to edit/i); + await axeScan(baseElement); + + await arrowTo(amountCell, 'ArrowRight', linkCell); + expect(describedByText(linkCell)).toMatch(/contains a control\. press enter to activate it/i); + // the hint moved with focus + expect(amountCell.hasAttribute('aria-describedby')).toBe(false); + + await arrowTo(linkCell, 'ArrowRight', urlCell); + expect(describedByText(urlCell)).toMatch(/contains a link\. press enter to open it/i); + + await arrowTo(urlCell, 'ArrowRight', bothCell); + expect(describedByText(bothCell)).toMatch(/contains controls\. press enter to move to them/i); + + await arrowTo(bothCell, 'ArrowLeft', urlCell); + await arrowTo(urlCell, 'ArrowLeft', linkCell); + await arrowTo(linkCell, 'ArrowLeft', amountCell); + await arrowTo(amountCell, 'ArrowLeft', nameCell); + // a plain read-only cell gets no hint at all + expect(nameCell.hasAttribute('aria-describedby')).toBe(false); + expect(linkCell.hasAttribute('aria-describedby')).toBe(false); + }); +}); + +describe('grid cell read-only state', () => { + test('a column is read-only unless it is editable AND has an editor, since Enter only opens an editor when both are set', () => { + const readOnlyColumns: ColumnWithFilter[] = [ + { key: 'Name', name: 'Name' }, + // editable for paste/clear eligibility but nothing to open on Enter + { key: 'Link', name: 'Link', editable: true }, + { key: 'Amount', name: 'Amount', editable: true, renderEditCell: () => null }, + // per-row predicate: only the first row opens an editor + { key: 'Url', name: 'Url', editable: (row) => row._key === '1', renderEditCell: () => null }, + ]; + render( row._key} />); + + expect(getCell('1', 'Name').getAttribute('aria-readonly')).toBe('true'); + expect(getCell('1', 'Link').getAttribute('aria-readonly')).toBe('true'); + expect(getCell('1', 'Amount').hasAttribute('aria-readonly')).toBe(false); + expect(getCell('1', 'Url').hasAttribute('aria-readonly')).toBe(false); + expect(getCell('2', 'Url').getAttribute('aria-readonly')).toBe('true'); + }); + + test('summary row cells are read-only even in editable columns, since they host controls rather than an editor', () => { + const summaryColumns: ColumnWithFilter[] = [ + { key: 'Name', name: 'Name', renderSummaryCell: () => Totals }, + { + key: 'Amount', + name: 'Amount', + editable: true, + renderEditCell: () => null, + renderSummaryCell: () => , + }, + ]; + render( row._key} topSummaryRows={[{ label: 'Totals' }]} />); + const summaryRowId = getSummaryRowId(0); + + const nameSummaryCell = getCell(summaryRowId, 'Name'); + const amountSummaryCell = getCell(summaryRowId, 'Amount'); + expect(nameSummaryCell.textContent).toBe('Totals'); + expect(nameSummaryCell.getAttribute('aria-readonly')).toBe('true'); + expect(amountSummaryCell.querySelector('button')?.textContent).toBe('Reset'); + expect(amountSummaryCell.getAttribute('aria-readonly')).toBe('true'); + // the body cell of the same editable column is still editable + expect(getCell('1', 'Amount').hasAttribute('aria-readonly')).toBe(false); + }); +}); diff --git a/libs/ui/src/lib/data-table/grid/__tests__/grid-in-cell-text-input.spec.tsx b/libs/ui/src/lib/data-table/grid/__tests__/grid-in-cell-text-input.spec.tsx index 0b0cedb6a..ffaeb9a53 100644 --- a/libs/ui/src/lib/data-table/grid/__tests__/grid-in-cell-text-input.spec.tsx +++ b/libs/ui/src/lib/data-table/grid/__tests__/grid-in-cell-text-input.spec.tsx @@ -1,3 +1,4 @@ +import { axeScan } from '@jetstream/test-utils'; import { fireEvent, render } from '@testing-library/react'; import { beforeAll, describe, expect, test, vi } from 'vitest'; import { DataTable, DataTableProps } from '../../DataTable'; @@ -56,6 +57,11 @@ function pasteInto(element: HTMLElement, text: string): boolean { } describe('in-cell text inputs keep their own keyboard and clipboard behavior', () => { + test('a grid whose summary row holds a filter input has no axe violations', async () => { + const { baseElement } = renderTable(); + await axeScan(baseElement); + }); + test('pasting into a summary row filter input is left to the input', () => { const onPaste = vi.fn(); const { getByLabelText } = renderTable({ onPaste }); @@ -83,6 +89,21 @@ describe('in-cell text inputs keep their own keyboard and clipboard behavior', ( } }); + // A cell whose only control is the filter input has nothing for Tab to cycle to, so Tab is the way + // back to the cell — focus must stay inside the grid instead of escaping to the next page tab stop. + test('Tab in actionable mode returns focus to the cell when there is only one control', () => { + const { getByLabelText } = renderTable({ onPaste: vi.fn() }); + const input = getByLabelText('Filter') as HTMLInputElement; + const cell = input.closest('[data-row-id]') as HTMLElement; + + fireEvent.mouseDown(cell); + fireEvent.keyDown(cell, { key: 'Enter' }); + input.focus(); + + expect(fireEvent.keyDown(input, { key: 'Tab' })).toBe(false); + expect(document.activeElement).toBe(cell); + }); + test('pasting into a data cell still reaches the grid', () => { const onPaste = vi.fn(); renderTable({ onPaste }); diff --git a/libs/ui/src/lib/data-table/grid/__tests__/useGridKeyboardNavigation.spec.tsx b/libs/ui/src/lib/data-table/grid/__tests__/useGridKeyboardNavigation.spec.tsx index a65833210..5dcf6e3c0 100644 --- a/libs/ui/src/lib/data-table/grid/__tests__/useGridKeyboardNavigation.spec.tsx +++ b/libs/ui/src/lib/data-table/grid/__tests__/useGridKeyboardNavigation.spec.tsx @@ -1,7 +1,8 @@ import { useTable } from '@tanstack/react-table'; import { act, renderHook } from '@testing-library/react'; +import type { KeyboardEvent as ReactKeyboardEvent } from 'react'; import { beforeEach, describe, expect, test, vi } from 'vitest'; -import { SELECT_COLUMN_KEY } from '../grid-constants'; +import { HEADER_ROW_ID, SELECT_COLUMN_KEY } from '../grid-constants'; import { jetstreamTableFeatures } from '../grid-features'; import { TanstackColumnDef, TanstackTable } from '../grid-types'; import { useGridKeyboardNavigation } from '../keyboard/useGridKeyboardNavigation'; @@ -32,21 +33,45 @@ const data: Row[] = [ { _key: '3', Name: 'Charlie', Amount: '30' }, ]; -function renderNav(options: { columns?: TanstackColumnDef[]; scrollLeft?: number } = {}) { +function renderNav(options: { columns?: TanstackColumnDef[]; scrollLeft?: number; data?: Row[]; rootElement?: HTMLElement } = {}) { const scroller = document.createElement('div'); scroller.scrollLeft = options.scrollLeft ?? 0; return renderHook(() => { const table = useTable({ features: jetstreamTableFeatures, - data, + data: options.data ?? data, columns: options.columns ?? columns, getRowId: (row) => row._key, }) as unknown as TanstackTable; - const keyboardNav = useGridKeyboardNavigation({ table, getRootElement: () => null, getScrollElement: () => scroller }); + const keyboardNav = useGridKeyboardNavigation({ + table, + getRootElement: () => options.rootElement ?? null, + getScrollElement: () => scroller, + }); return { table, keyboardNav }; }); } +function keyEvent(key: string): ReactKeyboardEvent { + return { + key, + ctrlKey: false, + metaKey: false, + shiftKey: false, + altKey: false, + preventDefault: vi.fn(), + stopPropagation: vi.fn(), + target: null, + } as unknown as ReactKeyboardEvent; +} + +/** Flush the hook's rAF-deferred focus work inside act. */ +async function flushFrame() { + await act(async () => { + await new Promise((resolve) => window.requestAnimationFrame(() => resolve(null))); + }); +} + /** The permission-manager shape: a frozen DATA column pinned at the far left, ahead of scrollable ones. */ const frozenDataColumns: TanstackColumnDef[] = [ { id: 'Name', accessorKey: 'Name', meta: { jetstream: { frozen: true } } as TanstackColumnDef['meta'] }, @@ -199,3 +224,106 @@ describe('useGridKeyboardNavigation — mouse range drag', () => { expect(getSelectionBounds(result.current.table)).toEqual([{ minRowIndex: 0, maxRowIndex: 0, minColumnIndex: 1, maxColumnIndex: 1 }]); }); }); + +describe('useGridKeyboardNavigation — empty body (a filter matched zero rows)', () => { + test('Tab-in seeds the header row so the column filters stay reachable', () => { + const { result } = renderNav({ data: [] }); + const rootEl = document.createElement('div'); + + act(() => result.current.keyboardNav.handleRootFocus({ target: rootEl, currentTarget: rootEl } as never)); + + expect(result.current.keyboardNav.activeCell).toEqual({ rowId: HEADER_ROW_ID, columnId: SELECT_COLUMN_KEY }); + }); + + test('header navigation still works: arrows move between header cells, Down stays on the header', () => { + const { result } = renderNav({ data: [] }); + act(() => result.current.keyboardNav.handleHeaderCellMouseDown('Name')); + + act(() => result.current.keyboardNav.handleKeyDown(keyEvent('ArrowRight'))); + expect(result.current.keyboardNav.activeCell).toEqual({ rowId: HEADER_ROW_ID, columnId: 'Amount' }); + + // There is no body row to step into — Down is swallowed and focus stays on the header. + act(() => result.current.keyboardNav.handleKeyDown(keyEvent('ArrowDown'))); + expect(result.current.keyboardNav.activeCell).toEqual({ rowId: HEADER_ROW_ID, columnId: 'Amount' }); + + act(() => result.current.keyboardNav.handleKeyDown(keyEvent('ArrowLeft'))); + expect(result.current.keyboardNav.activeCell).toEqual({ rowId: HEADER_ROW_ID, columnId: 'Name' }); + }); + + test('a stale active cell (its row was filtered away) snaps to the header row and keeps its column', () => { + const { result } = renderNav({ data: [] }); + // The user was on a body cell when the filter removed every row from under it. + act(() => result.current.keyboardNav.handleCellMouseDown('1', 'Amount', false)); + + act(() => result.current.keyboardNav.handleKeyDown(keyEvent('ArrowUp'))); + + expect(result.current.keyboardNav.activeCell).toEqual({ rowId: HEADER_ROW_ID, columnId: 'Amount' }); + }); + + test('arrow keys with no active cell at all land on the header row instead of dying', () => { + const { result } = renderNav({ data: [] }); + + act(() => result.current.keyboardNav.handleKeyDown(keyEvent('ArrowDown'))); + + expect(result.current.keyboardNav.activeCell).toEqual({ rowId: HEADER_ROW_ID, columnId: SELECT_COLUMN_KEY }); + }); +}); + +describe('useGridKeyboardNavigation — focus return after an overlay closes', () => { + function buildGridDom(cells: { rowId: string; columnId: string }[]) { + const root = document.createElement('div'); + const cellElements = cells.map(({ rowId, columnId }) => { + const cellEl = document.createElement('div'); + cellEl.setAttribute('data-row-id', rowId); + cellEl.setAttribute('data-col-id', columnId); + cellEl.tabIndex = -1; + root.appendChild(cellEl); + return cellEl; + }); + document.body.appendChild(root); + const overlay = document.createElement('div'); + overlay.className = 'slds-popover'; + const overlayInput = document.createElement('input'); + overlay.appendChild(overlayInput); + document.body.appendChild(overlay); + return { root, cellElements, overlay, overlayInput, cleanup: () => [root, overlay].forEach((el) => el.remove()) }; + } + + test('an overlay that unmounts with focus inside it (no focusout fires) still returns focus to the cell', async () => { + const { root, cellElements, overlay, overlayInput, cleanup } = buildGridDom([{ rowId: HEADER_ROW_ID, columnId: 'Name' }]); + const [headerCellEl] = cellElements; + const trigger = document.createElement('button'); + headerCellEl.appendChild(trigger); + const { result } = renderNav({ rootElement: root }); + + act(() => result.current.keyboardNav.handleHeaderCellMouseDown('Name')); + // Focus enters the overlay — arms the pending return-focus cell. + act(() => overlayInput.focus()); + // The overlay unmounts WITH focus inside it: the browser fires no focusout for the removed node; + // the overlay's own returnFocus then lands on the trigger, and that focusin is the only signal. + act(() => overlay.remove()); + act(() => trigger.focus()); + await flushFrame(); + + expect(document.activeElement).toBe(headerCellEl); + cleanup(); + }); + + test('when the originating row was filtered away, focus lands on the header cell of the same column', async () => { + // Only the header cell exists in the DOM — the body row the overlay was opened from is gone. + const { root, overlay, overlayInput, cleanup } = buildGridDom([{ rowId: HEADER_ROW_ID, columnId: 'Name' }]); + const { result } = renderNav({ data: [], rootElement: root }); + + act(() => result.current.keyboardNav.handleCellMouseDown('1', 'Name', false)); + act(() => overlayInput.focus()); + // Focus leaves the overlay for nowhere in particular (it closes), so focus falls to . + act(() => overlayInput.blur()); + await flushFrame(); + act(() => overlay.remove()); + act(() => document.dispatchEvent(new FocusEvent('focusout'))); + await flushFrame(); + + expect(result.current.keyboardNav.activeCell).toEqual({ rowId: HEADER_ROW_ID, columnId: 'Name' }); + cleanup(); + }); +}); diff --git a/libs/ui/src/lib/data-table/grid/__tests__/useGridTabOrderContainment.spec.tsx b/libs/ui/src/lib/data-table/grid/__tests__/useGridTabOrderContainment.spec.tsx new file mode 100644 index 000000000..d1fe7fb65 --- /dev/null +++ b/libs/ui/src/lib/data-table/grid/__tests__/useGridTabOrderContainment.spec.tsx @@ -0,0 +1,101 @@ +import { fireEvent, render, waitFor } from '@testing-library/react'; +import { useEffect, useState } from 'react'; +import { beforeAll, describe, expect, test } from 'vitest'; +import { DataTable } from '../../DataTable'; +import { ColumnWithFilter } from '../grid-types'; + +interface Row { + _key: string; + Id: string; + Name: string; +} + +/** Mirrors SalesforceLogin: renders nothing until an async value resolves, then mounts a link. */ +function LateMountedLink({ id }: { id: string }) { + const [href, setHref] = useState(null); + useEffect(() => { + const timeout = setTimeout(() => setHref(`https://example.com/${id}`), 0); + return () => clearTimeout(timeout); + }, [id]); + if (!href) { + return null; + } + return ( + + {id} + + ); +} + +const columns: ColumnWithFilter[] = [ + { + key: 'Id', + name: 'Id', + renderCell: ({ row }) => ( + + + {row.Id} + + + + ), + }, + { + key: 'Name', + name: 'Name', + renderCell: ({ row }) => , + }, +]; + +const data: Row[] = [ + { _key: '1', Id: '001', Name: 'One' }, + { _key: '2', Id: '002', Name: 'Two' }, +]; + +// The row/column virtualizers measure the scroll container, which jsdom reports as 0x0 — nothing would +// render. Give every element a viewport-sized box so the grid mounts real rows. +beforeAll(() => { + for (const property of ['clientHeight', 'clientWidth', 'offsetHeight', 'offsetWidth'] as const) { + Object.defineProperty(HTMLElement.prototype, property, { configurable: true, value: 600 }); + } + HTMLElement.prototype.getBoundingClientRect = () => + ({ width: 800, height: 600, top: 0, left: 0, bottom: 600, right: 800, x: 0, y: 0, toJSON: () => ({}) }) as DOMRect; +}); + +function renderTable() { + return render( row._key} />); +} + +describe('grid tab-order containment (single page tab stop)', () => { + test('links and buttons rendered by consumer cell renderers are removed from the tab order', async () => { + const { getByTestId } = renderTable(); + + await waitFor(() => { + expect((getByTestId('link-001') as HTMLElement).tabIndex).toBe(-1); + expect((getByTestId('link-002') as HTMLElement).tabIndex).toBe(-1); + expect((getByTestId('button-001') as HTMLElement).tabIndex).toBe(-1); + }); + }); + + test('a link that mounts after the cell (async href, like SalesforceLogin) is also removed', async () => { + const { findByTestId } = renderTable(); + + const lateLink = (await findByTestId('late-link-001')) as HTMLElement; + await waitFor(() => expect(lateLink.tabIndex).toBe(-1)); + }); + + test('the roving tabindex on the active cell is left alone', async () => { + renderTable(); + + const cell = document.querySelector('[data-row-id="1"][data-col-id="Id"]') as HTMLElement; + expect(cell).toBeTruthy(); + fireEvent.mouseDown(cell); + expect(cell.tabIndex).toBe(0); + + // Give the MutationObserver's microtask a chance to (incorrectly) sweep it. + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(cell.tabIndex).toBe(0); + }); +}); diff --git a/libs/ui/src/lib/data-table/grid/components/GridBody.tsx b/libs/ui/src/lib/data-table/grid/components/GridBody.tsx index f99901f61..c2e9f5dac 100644 --- a/libs/ui/src/lib/data-table/grid/components/GridBody.tsx +++ b/libs/ui/src/lib/data-table/grid/components/GridBody.tsx @@ -3,10 +3,11 @@ import { shallow, useSelector } from '@tanstack/react-store'; import type { CellSelectionBounds } from '@tanstack/react-table'; import { useVirtualizer } from '@tanstack/react-virtual'; import { CSSProperties, RefObject, useEffect, useMemo, useRef } from 'react'; +import { CellHintKind, getCellHintId } from '../grid-cell-hints'; import { DEFAULT_ROW_HEIGHT, HEADER_ROW_ID, isSummaryRowId } from '../grid-constants'; import { selectRowModelInputs } from '../grid-context'; import { TanstackTable } from '../grid-types'; -import { GridInteractionSource, GridMode } from '../keyboard/useGridKeyboardNavigation'; +import { GRID_OVERLAY_SELECTOR, GridInteractionSource, GridMode } from '../keyboard/useGridKeyboardNavigation'; import { getRowSelectionSpans } from '../selection/grid-selection'; import { GridGroupRow } from './GridGroupRow'; import { ActiveCell, GridRow } from './GridRow'; @@ -22,6 +23,8 @@ export interface GridBodyProps { gridTemplateColumns: string; /** Visible leaf-column indexes to render (windowed + always-on frozen), passed through to each row. */ visibleColumnIndexes: number[]; + /** The grid is a treegrid: every row (not just nested ones) exposes aria-level. */ + treeGrid?: boolean; /** Fixed numeric height per row, or a per-row callback. This is the authoritative, deterministic row * height — each row's box is pinned to it. Rows are NOT DOM-measured: with column virtualization the * mounted-cell set changes during horizontal scroll, so a measured height would oscillate and reflow @@ -41,6 +44,8 @@ export interface GridBodyProps { getLastInteractionSource?: () => GridInteractionSource; /** The cell currently being edited (its editor owns focus, so the body must not steal it). */ editingCell?: ActiveCell | null; + /** Prefix of the hidden cell-hint elements rendered by GridContainer (see grid-cell-hints.ts). */ + gridId: string; /** Resolved cell-selection rectangles (inclusive display-index bounds; empty when collapsed). */ selectionBounds?: CellSelectionBounds[]; onCellMouseDown?: (rowId: string, columnId: string, shiftKey: boolean, button?: number, ctrlOrMetaKey?: boolean) => void; @@ -74,6 +79,7 @@ export function GridBody({ scrollRef, gridTemplateColumns, visibleColumnIndexes, + treeGrid, rowHeight, overscan = 8, summaryRowCount = 0, @@ -81,6 +87,7 @@ export function GridBody({ mode = 'navigation', getLastInteractionSource, editingCell, + gridId, selectionBounds, onCellMouseDown, onCellMouseEnter, @@ -122,28 +129,92 @@ export function GridBody({ const rowVirtualizer = useVirtualizer({ count: rows.length, getScrollElement: () => scrollRef.current, + // Heights are rounded up to whole pixels: a fractional row height lands alternate rows on half-pixel + // offsets, where their 1px borders render faint and uneven on 1x displays. estimateSize: (index) => { const current = rowHeightRef.current; if (typeof current === 'function') { const row = rows[index]; if (row) { - return current({ type: row.getIsGrouped() ? 'GROUP' : 'ROW', row: row.original, columnWidths: columnWidthsRef.current }); + return Math.ceil( + current({ type: row.getIsGrouped() ? 'GROUP' : 'ROW', row: row.original, columnWidths: columnWidthsRef.current }), + ); } return DEFAULT_ROW_HEIGHT; } - return current ?? DEFAULT_ROW_HEIGHT; + return Math.ceil(current ?? DEFAULT_ROW_HEIGHT); }, overscan, getItemKey: (index) => rows[index].id, // In auto-height mode the estimate above is only the initial guess; the virtualizer measures each // rendered row's real height (rows wrap to content) and corrects the offsets, keeping virtualization. - ...(autoRowHeight ? { measureElement: (el: Element | null) => el?.getBoundingClientRect().height ?? DEFAULT_ROW_HEIGHT } : {}), + ...(autoRowHeight + ? { measureElement: (el: Element | null) => Math.ceil(el?.getBoundingClientRect().height ?? DEFAULT_ROW_HEIGHT) } + : {}), }); const measureRowRef = autoRowHeight ? rowVirtualizer.measureElement : undefined; // Resolve the active cell to a DOM node: scroll its row into view, then focus the cell (navigation) // or the first focusable inside it (actionable). Runs only when the coordinate/mode changes — NOT on // manual scroll — so scrolling away from the active cell never yanks the viewport back. + // The cell currently carrying the keyboard hint, so it can be cleared when focus moves on. + const hintedCellRef = useRef(null); + + /** + * Which hint describes the cell — the same decision tree as the keyboard hook's Enter handling + * (edit first, then activateCell). Header cells get none: their sort/filter controls are announced directly. + */ + const getCellHintKind = (cellEl: HTMLElement): CellHintKind | null => { + const role = cellEl.getAttribute('role'); + if (role !== 'gridcell' && role !== 'rowheader') { + return null; + } + if (cellEl.getAttribute('aria-readonly') !== 'true') { + return 'editable'; + } + if (cellEl.querySelectorAll('input[type="checkbox"]:not([disabled])').length === 1) { + return 'checkbox'; + } + const controls = Array.from(cellEl.querySelectorAll(ACTIONABLE_FOCUSABLE_SELECTOR)); + if (controls.length === 0) { + return null; + } + if (controls.length > 1) { + return 'controls'; + } + const [control] = controls; + if (control.matches('.jgrid-tree-toggle')) { + return 'expand'; + } + if (control.matches('a[href]')) { + return 'link'; + } + if (control.matches('input:not([type="checkbox"]):not([type="radio"]):not([type="button"]), textarea, select')) { + return 'input'; + } + return 'control'; + }; + + /** + * Screen readers announce a gridcell's content but nothing about what Enter would do with it, so the + * cell is described before it takes focus. Applied imperatively to the single focused cell rather than + * rendered on every cell, and cleared again when focus moves on. + */ + const applyCellKeyboardHints = (cellEl: HTMLElement) => { + const previousHintedCell = hintedCellRef.current; + if (previousHintedCell && previousHintedCell !== cellEl) { + previousHintedCell.removeAttribute('aria-describedby'); + } + const hintKind = getCellHintKind(cellEl); + if (hintKind) { + cellEl.setAttribute('aria-describedby', getCellHintId(gridId, hintKind)); + hintedCellRef.current = cellEl; + } else { + cellEl.removeAttribute('aria-describedby'); + hintedCellRef.current = null; + } + }; + // True when the active cell is the one being edited — its editor input owns focus, so skip cell focus. const isEditingActiveCell = !!editingCell && !!activeCell && editingCell.rowId === activeCell.rowId && editingCell.columnId === activeCell.columnId; @@ -192,10 +263,36 @@ export function GridBody({ if (cancelled) { return; } + // Never steal focus from an overlay opened FROM the grid (record popover, filter popover, + // dropdown menu): keyboard activation re-applies the active cell, and without this guard the + // effect's refocus raced the overlay's own focus management and yanked focus back to the cell. + // An overlay that CONTAINS the grid (a modal hosting it) doesn't count. + const overlayWithFocus = (document.activeElement as HTMLElement | null)?.closest?.(GRID_OVERLAY_SELECTOR); + if (overlayWithFocus && (!scrollRef.current || !overlayWithFocus.contains(scrollRef.current))) { + return; + } const cellEl = scrollRef.current?.querySelector( `[data-row-id="${CSS.escape(activeCell.rowId)}"][data-col-id="${CSS.escape(activeCell.columnId)}"]`, ); if (cellEl) { + // Native focus scrolling only ensures the cell is inside the viewport — sticky frozen + // columns overlay the left edge, so a cell can land hidden UNDERNEATH the frozen band. + // Nudge the scroller left by the overlap so the cell is actually visible. + const scroller = scrollRef.current; + // Body/header cells carry the frozen class; summary cells are made sticky through their inline style + const isFrozenCell = cellEl.classList.contains('jgrid-cell-frozen') || cellEl.style.position === 'sticky'; + if (scroller && !isFrozenCell) { + const frozenBandWidth = leafColumns.reduce( + (width, column) => (column.columnDef.meta?.jetstream?.frozen ? width + column.getSize() : width), + 0, + ); + if (frozenBandWidth > 0) { + const overlap = frozenBandWidth - (cellEl.getBoundingClientRect().left - scroller.getBoundingClientRect().left); + if (overlap > 0) { + scroller.scrollLeft -= overlap; + } + } + } if (mode === 'actionable') { // Move focus to the first interactive control. The cell DIV holding focus (the navigation-mode // state) must NOT count as "already inside" — otherwise entering actionable mode from the cell @@ -207,7 +304,14 @@ export function GridBody({ (focusable ?? cellEl).focus(); } } else if (!cellEl.contains(document.activeElement)) { - cellEl.focus(); + // APG grid pattern: a cell whose only content is a single interactive widget marks it with + // data-grid-inner-focus — arrow navigation focuses the widget itself so its role and state + // are announced (e.g. "Pin entry, toggle button, pressed") and Enter/Space activate natively + const innerFocusTarget = cellEl.querySelector('[data-grid-inner-focus]:not(:disabled)'); + if (!innerFocusTarget) { + applyCellKeyboardHints(cellEl); + } + (innerFocusTarget ?? cellEl).focus(); } return; } @@ -293,6 +397,7 @@ export function GridBody({ return ( ({ const dynamicClass = typeof meta?.cellClass === 'function' ? meta.cellClass(row) : meta?.cellClass; - // `editable` may be a per-row predicate, so resolve it against this row before announcing read-only. + // A cell is announced editable only when Enter really opens an editor — `editable` alone also covers + // paste/clear eligibility (checkbox columns), where Enter toggles the control instead. `editable` may + // be a per-row predicate, so resolve it against this row. const editable = meta?.editable; - const isEditable = typeof editable === 'function' ? editable(row) : !!editable; + const isEditable = !!meta?.editor && (typeof editable === 'function' ? editable(row) : !!editable); const style: CSSProperties = { ...(colSpan > 1 ? { gridColumn: `${colIndex + 1} / span ${colSpan}` } : { gridColumnStart: colIndex + 1 }), diff --git a/libs/ui/src/lib/data-table/grid/components/GridContainer.tsx b/libs/ui/src/lib/data-table/grid/components/GridContainer.tsx index 28d09201c..cc90c4fdb 100644 --- a/libs/ui/src/lib/data-table/grid/components/GridContainer.tsx +++ b/libs/ui/src/lib/data-table/grid/components/GridContainer.tsx @@ -5,8 +5,10 @@ import { useVirtualizer } from '@tanstack/react-virtual'; import classNames from 'classnames'; import { CSSProperties, ReactNode, useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { ContextMenu } from '../../../form/context-menu/ContextMenu'; +import { FILTER_COUNT_ANNOUNCE_DEBOUNCE_MS } from '../../../widgets/AssistiveStatus'; import { EditorHost } from '../editors/EditorHost'; import { computeEdgeScrollVelocity, createEdgeAutoScroller } from '../grid-auto-scroll'; +import { CELL_HINT_KINDS, CELL_HINT_TEXT, getCellHintId } from '../grid-cell-hints'; import { copyGridDataToClipboard, copyGridGroupRowsToClipboard, GridCopyResult } from '../grid-clipboard'; import { reorderColumnOrder } from '../grid-column-utils'; import { HEADER_ROW_ID, isSummaryRowId, isTextEntryElement, NON_DATA_COLUMN_KEYS, TABLE_CONTEXT_MENU_ITEMS } from '../grid-constants'; @@ -35,7 +37,8 @@ import { RowWithKey, TanstackTable, } from '../grid-types'; -import { useGridKeyboardNavigation } from '../keyboard/useGridKeyboardNavigation'; +import { GRID_OVERLAY_SELECTOR, useGridKeyboardNavigation } from '../keyboard/useGridKeyboardNavigation'; +import { useGridTabOrderContainment } from '../keyboard/useGridTabOrderContainment'; import { getActiveRangeRect, getSelectionBounds, hasMultiCellSelection } from '../selection/grid-selection'; import { GridBody, RowHeightFn } from './GridBody'; import { GridHeader } from './GridHeader'; @@ -159,9 +162,10 @@ export function GridContainer({ if (editingCellRef.current || contextMenuRef.current) { return true; } - // A popover/modal opened from a cell (via Space/Enter or click) moves focus into its portaled panel; - // keep the active cell so closing the overlay returns to a live grid coordinate. - return relatedTarget instanceof HTMLElement && !!relatedTarget.closest('.jgrid-editor, .slds-popover, .slds-modal'); + // A popover/modal/dropdown menu opened from a cell (via Space/Enter or click) moves focus into its + // portaled panel; keep the active cell so closing the overlay returns to a live grid coordinate. + // `.jgrid-editor` (the in-grid popup editor) is specific to this blur guard. + return relatedTarget instanceof HTMLElement && !!relatedTarget.closest(`.jgrid-editor, ${GRID_OVERLAY_SELECTOR}`); }, []); // Paste/clear eligibility: `editable` alone (no `renderEditCell` required). Checkbox-style tables @@ -282,6 +286,9 @@ export function GridContainer({ onClearSelection: onPaste ? stableClearSelection : undefined, }); + // The grid is a single page tab stop — strip consumer-rendered in-cell controls from the tab order. + useGridTabOrderContainment(useCallback(() => gridRef.current, [])); + // Announce the matching row count after the filter set changes (the filtered model is pre-grouping, so // this counts data rows and isn't perturbed by expanding/collapsing groups or sorting). Skips the // initial render so the grid doesn't announce on mount. @@ -293,10 +300,17 @@ export function GridContainer({ const filteredRowCount = table.getFilteredRowModel().rows.length; const previousFilteredRowCountRef = useRef(null); useEffect(() => { - if (previousFilteredRowCountRef.current !== null && previousFilteredRowCountRef.current !== filteredRowCount) { - announce(`${filteredRowCount} ${filteredRowCount === 1 ? 'row' : 'rows'}`); - } + const previousCount = previousFilteredRowCountRef.current; previousFilteredRowCountRef.current = filteredRowCount; + if (previousCount === null || previousCount === filteredRowCount) { + return; + } + // Debounced: quick-filter typing changes the count every keystroke, and screen readers drop + // polite live-region churn during typing — announce once after the count settles + const timeout = window.setTimeout(() => { + announce(`${filteredRowCount} ${filteredRowCount === 1 ? 'row' : 'rows'}`); + }, FILTER_COUNT_ANNOUNCE_DEBOUNCE_MS); + return () => window.clearTimeout(timeout); }, [filteredRowCount, announce]); const leafColumns = table.getVisibleLeafColumns(); @@ -849,7 +863,8 @@ export function GridContainer({ role={role} data-id={gridId} aria-label={ariaLabel || 'Data table'} - aria-rowcount={rowCount + 1 + (summaryRows?.length ?? 0)} + // Header + summary rows + body rows; with no data the "No data available" placeholder is a row + aria-rowcount={Math.max(rowCount, 1) + 1 + (summaryRows?.length ?? 0)} aria-colcount={leafColumns.length} aria-multiselectable={table.options.enableRowSelection ? true : undefined} className="jgrid" @@ -878,6 +893,7 @@ export function GridContainer({ /> ({ mode={keyboardNav.mode} getLastInteractionSource={keyboardNav.getLastInteractionSource} editingCell={editingCell} + gridId={gridId} selectionBounds={selectionBounds} onCellMouseDown={keyboardNav.handleCellMouseDown} onCellMouseEnter={keyboardNav.handleCellMouseEnter} @@ -901,6 +918,13 @@ export function GridContainer({
+ {/* Keyboard hints for the focused cell — GridBody points the cell's aria-describedby at one of these. */} + {CELL_HINT_KINDS.map((kind) => ( + + {CELL_HINT_TEXT[kind]} + + ))} + {/* Screen-reader announcement of the current navigation/actionable mode. */} {keyboardNav.mode === 'actionable' ? 'Actionable mode' : 'Navigation mode'} diff --git a/libs/ui/src/lib/data-table/grid/components/GridGroupRow.tsx b/libs/ui/src/lib/data-table/grid/components/GridGroupRow.tsx index 93bbd23c9..33a5d62d8 100644 --- a/libs/ui/src/lib/data-table/grid/components/GridGroupRow.tsx +++ b/libs/ui/src/lib/data-table/grid/components/GridGroupRow.tsx @@ -106,6 +106,9 @@ export function GridGroupRow({
({
{ isExpanded: boolean; /** True for the last data row — lets its corner cells round to match the table's bottom corners. */ isLastRow: boolean; + /** In a treegrid every row exposes its level (1-based); a plain grid has no levels. */ + treeGrid?: boolean; /** Selection-rectangle spans intersecting this row (identity-stable per bounds), or null. */ selectionSpans?: RowSelectionSpan[] | null; rowClass?: (row: TRow) => string | undefined; @@ -60,6 +62,7 @@ function GridRowComponent({ activeCell, isExpanded, isLastRow, + treeGrid, selectionSpans, rowClass, onCellMouseDown, @@ -142,7 +145,7 @@ function GridRowComponent({ ref={autoHeight ? measureRef : undefined} role="row" aria-rowindex={ariaRowIndex} - aria-level={row.depth > 0 ? row.depth + 1 : undefined} + aria-level={treeGrid ? row.depth + 1 : undefined} aria-expanded={row.getCanExpand() ? isExpanded : undefined} aria-selected={row.getCanSelect() ? isSelected : undefined} data-row-id={row.id} diff --git a/libs/ui/src/lib/data-table/grid/components/GridSummaryRow.tsx b/libs/ui/src/lib/data-table/grid/components/GridSummaryRow.tsx index 36920f788..bbc02ff4e 100644 --- a/libs/ui/src/lib/data-table/grid/components/GridSummaryRow.tsx +++ b/libs/ui/src/lib/data-table/grid/components/GridSummaryRow.tsx @@ -51,10 +51,14 @@ export function GridSummaryRow({ const summaryCellClass = meta?.column?.summaryCellClass; const dynamicClass = typeof summaryCellClass === 'function' ? summaryCellClass(summaryRow as any) : summaryCellClass; const isActive = activeColumnId === column.id; + // Summary cells host filter and bulk-action controls, never an editor: read-only for the cell hints return (
({ editingCell, table, getRootEle className="slds-p-around_x-small" onOutsideClick={() => handleClose(draftRow !== null ? true : (authorColumn.editorOptions?.commitOnOutsideClick ?? false), false)} > - + {/* The editor's popup (date picker, picklist, combobox list) IS the editing surface: one Escape + must discard the draft, not merely close the popup and demand a second press */} + + +
diff --git a/libs/ui/src/lib/data-table/grid/filters/HeaderFilters.tsx b/libs/ui/src/lib/data-table/grid/filters/HeaderFilters.tsx index 4774157ed..039b24f2f 100644 --- a/libs/ui/src/lib/data-table/grid/filters/HeaderFilters.tsx +++ b/libs/ui/src/lib/data-table/grid/filters/HeaderFilters.tsx @@ -102,9 +102,21 @@ export const HeaderFilterButton = memo(({ columnKey, columnName }: { columnKey: } return ( -
ev.stopPropagation()} onPointerDown={(ev) => ev.stopPropagation()} onKeyDown={(ev) => ev.stopPropagation()}> +
ev.stopPropagation()} + onPointerDown={(ev) => ev.stopPropagation()} + onKeyDown={(ev) => { + // Only swallow activation keys (the grid would ALSO sort/filter the header cell) — arrows and + // Escape must keep bubbling to the grid, or focus landing back on this trigger after the + // popover closes leaves the user with dead table navigation + if (ev.key === 'Enter' || ev.key === ' ') { + ev.stopPropagation(); + } + }} + > ev.stopPropagation()}>

Filter

@@ -315,7 +327,13 @@ export const HeaderSetFilter = memo(({ columnKey, filter, values, updateFilter } max-height: 25vh; `} > - + {!hasVisibleItems &&
No items
} {hasVisibleItems && ( <> diff --git a/libs/ui/src/lib/data-table/grid/grid-cell-hints.ts b/libs/ui/src/lib/data-table/grid/grid-cell-hints.ts new file mode 100644 index 000000000..3630a09eb --- /dev/null +++ b/libs/ui/src/lib/data-table/grid/grid-cell-hints.ts @@ -0,0 +1,25 @@ +/** + * What Enter does on a focused body cell, spelled out for screen readers. Screen readers announce a + * gridcell's content but nothing about the editor, control or link behind it, so GridBody points the + * focused cell's aria-describedby at one of these (rendered once per grid by GridContainer as hidden + * text). The kinds mirror the branches of the grid's Enter handling: an editable cell opens its editor; + * otherwise a lone checkbox/control is activated, a lone text field is entered, and a cell with several + * controls switches to Actionable mode. + */ +export type CellHintKind = 'editable' | 'checkbox' | 'expand' | 'link' | 'input' | 'control' | 'controls'; + +export const CELL_HINT_TEXT: Record = { + editable: 'Editable. Press Enter to edit.', + checkbox: 'Press Enter to toggle the checkbox.', + expand: 'Expandable row. Press Enter to expand or collapse it.', + link: 'Contains a link. Press Enter to open it.', + input: 'Press Enter to type in the field and Escape to return to the cell.', + control: 'Contains a control. Press Enter to activate it.', + controls: 'Contains controls. Press Enter to move to them and Escape to return to the cell.', +}; + +export const CELL_HINT_KINDS = Object.keys(CELL_HINT_TEXT) as CellHintKind[]; + +export function getCellHintId(gridId: string, kind: CellHintKind) { + return `${gridId}-hint-${kind}`; +} diff --git a/libs/ui/src/lib/data-table/grid/grid-column-utils.tsx b/libs/ui/src/lib/data-table/grid/grid-column-utils.tsx index d41f287bd..0fcca486a 100644 --- a/libs/ui/src/lib/data-table/grid/grid-column-utils.tsx +++ b/libs/ui/src/lib/data-table/grid/grid-column-utils.tsx @@ -92,7 +92,7 @@ export function getColumnDefinitions( if (includeRecordActions) { parentColumns.unshift({ key: ACTION_COLUMN_KEY, - name: '', + name: 'Actions', resizable: true, width: 116, minWidth: 100, diff --git a/libs/ui/src/lib/data-table/grid/grid-constants.ts b/libs/ui/src/lib/data-table/grid/grid-constants.ts index b483e1de2..b9fd55257 100644 --- a/libs/ui/src/lib/data-table/grid/grid-constants.ts +++ b/libs/ui/src/lib/data-table/grid/grid-constants.ts @@ -47,7 +47,9 @@ export const TABLE_CONTEXT_MENU_ITEMS: ContextMenuItem[] = [ ]; /** Default fixed row height (px) for non-wrapped rows; also the virtualizer seed estimate. */ -export const DEFAULT_ROW_HEIGHT = 28.5; +// Whole pixels only: rows are placed with translateY(start), and a fractional height puts every other +// row's 1px border across two device pixels on 1x displays (faint, uneven lines). +export const DEFAULT_ROW_HEIGHT = 29; export const DEFAULT_HEADER_ROW_HEIGHT = 35; export const DEFAULT_SUMMARY_ROW_HEIGHT = 34; export const DEFAULT_COLUMN_WIDTH = 200; diff --git a/libs/ui/src/lib/data-table/grid/keyboard/useGridKeyboardNavigation.ts b/libs/ui/src/lib/data-table/grid/keyboard/useGridKeyboardNavigation.ts index e7823ef92..9fb360f85 100644 --- a/libs/ui/src/lib/data-table/grid/keyboard/useGridKeyboardNavigation.ts +++ b/libs/ui/src/lib/data-table/grid/keyboard/useGridKeyboardNavigation.ts @@ -26,6 +26,14 @@ import { useRangeDragAutoScroll } from './useRangeDragAutoScroll'; export type GridMode = 'navigation' | 'actionable'; +/** + * Portaled overlays that can hold focus on behalf of the grid — popovers, modals, dropdown menus, + * dialogs. Shared by every "is focus inside an overlay?" check (keyboard navigation, GridBody's + * refocus guard, GridContainer's blur guard) so the list cannot drift between copies again — a + * missing `.slds-dropdown` in one copy was a real bug. + */ +export const GRID_OVERLAY_SELECTOR = '.slds-popover, .slds-modal, .slds-dropdown, [role="dialog"]'; + /** * What drove the most recent active-cell change. Consumers use it to decide whether to move DOM focus * and whether to scroll the cell into view: @@ -90,6 +98,54 @@ function getRowSegmentStarts(row: TanstackRow, column return starts; } +/** + * Column indexes at which the HEADER row starts a rendered header cell — honors HEADER colSpans + * (column-group headers like the permission manager's profile name spanning its sub-columns), so + * header navigation steps between rendered cells instead of walking every spanned-over track. + */ +function getHeaderSegmentStarts(columns: TanstackColumn[]): number[] { + const starts: number[] = []; + let index = 0; + while (index < columns.length) { + starts.push(index); + const span = Math.max(1, columns[index].columnDef.meta?.jetstream?.colSpan?.({ type: 'HEADER' }) ?? 1); + index += span; + } + return starts; +} + +/** + * The header cell that OWNS `targetColIndex`: grouped headers span their sub-columns (only the span + * owner renders a header cell), so a vertical move from a body cell in a spanned sub-column must land + * on the owner or focus has no element to go to. + */ +function resolveHeaderColumnStart(columns: TanstackColumn[], targetColIndex: number): number { + let owner = 0; + for (const start of getHeaderSegmentStarts(columns)) { + if (start <= targetColIndex) { + owner = start; + } else { + break; + } + } + return owner; +} + +/** Id of the header cell that owns `columnId` (see resolveHeaderColumnStart). */ +function headerColumnIdFor(columns: TanstackColumn[], columnId: string): string { + const colIndex = Math.max( + 0, + columns.findIndex((column) => column.id === columnId), + ); + return columns[resolveHeaderColumnStart(columns, colIndex)]?.id ?? columnId; +} + +/** The next/previous segment start relative to `colIndex` within `starts` (clamped at the ends). */ +function stepSegment(starts: number[], colIndex: number, direction: 1 | -1): number { + const segmentIndex = Math.max(0, starts.filter((start) => start <= colIndex).length - 1); + return starts[clamp(segmentIndex + direction, 0, starts.length - 1)]; +} + /** * True when the row renders at least one cell wider than a single column. Consumers use a row-level * colSpan for message/placeholder rows ("No metadata found", "no rows found") — the tell that a row @@ -353,7 +409,9 @@ export function useGridKeyboardNavigation({ const root = getRootElement(); let attempts = 0; const tryFocus = () => { - const panel = Array.from(document.querySelectorAll('.slds-popover')).find((el) => !root || !el.contains(root)); + const panel = Array.from(document.querySelectorAll('.slds-popover:not(.slds-popover_tooltip)')).find( + (el) => !root || !el.contains(root), + ); if (panel) { if (!panel.contains(document.activeElement)) { const body = panel.querySelector('.slds-popover__body'); @@ -470,26 +528,21 @@ export function useGridKeyboardNavigation({ // overlay; when it closes and focus would fall to , pull it back to the cell so arrow navigation // continues. Works for popovers/modals opened by mouse OR keyboard, from a body cell or the header. useEffect(() => { - // Returns overlays (portaled popovers/modals) that are NOT an ancestor of this grid — i.e. a popover - // opened FROM the grid, excluding a modal that merely hosts the grid. + // True while focus is inside an overlay (portaled popover/modal/dropdown menu) that is NOT an ancestor + // of this grid — i.e. an overlay opened FROM the grid, excluding a modal that merely hosts the grid. + // Focus-based on purpose: a document-wide query was permanently true in the app (the navbar's + // CSS-toggled `.slds-dropdown` menus are always mounted, and any visible tooltip is a `.slds-popover`), + // which silently disabled every return-focus path below. const hasForeignOverlayOpen = () => { const root = getRootElement(); - return Array.from(document.querySelectorAll('.slds-popover, .slds-modal, [role="dialog"]')).some( - (overlay) => !root || !overlay.contains(root), - ); + const active = document.activeElement; + const overlay = active instanceof Element ? active.closest(GRID_OVERLAY_SELECTOR) : null; + return !!overlay && (!root || !overlay.contains(root)); }; - // When focus moves into such an overlay, remember the active cell so we can restore it on close. - const handleFocusIn = (event: FocusEvent) => { - const target = event.target as HTMLElement | null; - const overlay = target?.closest?.('.slds-popover, .slds-modal, [role="dialog"]'); - const root = getRootElement(); - if (overlay && (!root || !overlay.contains(root)) && activeCellRef.current) { - pendingReturnFocusCellRef.current = activeCellRef.current; - } - }; - - const handleFocusOut = () => { + // Deferred a frame so an overlay close/unmount settles first; while the overlay is still up + // (e.g. tabbing within it) the check is a no-op and the pending return stays armed. + const scheduleReturnFocusCheck = () => { if (!pendingReturnFocusCellRef.current) { return; } @@ -498,7 +551,6 @@ export function useGridKeyboardNavigation({ if (!cell) { return; } - // Wait while the overlay is still up (e.g. tabbing within it). if (hasForeignOverlayOpen()) { return; } @@ -509,19 +561,50 @@ export function useGridKeyboardNavigation({ // it back on a control INSIDE that cell (e.g. a header filter icon after Escape). Otherwise DOM // focus sits on the in-cell control and arrow navigation can't resume — the cell is the rover. const focusReturnedInsideCell = !!cellEl && !!active && active !== cellEl && cellEl.contains(active); - if (!active || active === document.body || focusReturnedInsideCell) { - cellEl?.focus(); + // ...unless that control is the cell's designated inner-focus widget, which IS the rover for that cell + const focusReturnedToInnerFocusWidget = focusReturnedInsideCell && !!active?.closest('[data-grid-inner-focus]'); + if (!active || active === document.body || (focusReturnedInsideCell && !focusReturnedToInnerFocusWidget)) { + if (cellEl) { + // The overlay was opened from actionable mode in many cases; the cell is the rover again, so + // navigation mode must be restored or Up/Down stay swallowed and the live region says "Actionable" + setMode('navigation'); + cellEl.focus(); + } else { + // The originating row is gone (the overlay's filter excluded it) — land on the header cell + // of the same column so navigation continues from a live coordinate. + interactionSourceRef.current = 'keyboard'; + applySelection(HEADER_ROW_ID, headerColumnIdFor(table.getVisibleLeafColumns(), cell.columnId), false); + } } }); }; + // When focus moves into such an overlay, remember the active cell so we can restore it on close. + const handleFocusIn = (event: FocusEvent) => { + const target = event.target as HTMLElement | null; + const overlay = target?.closest?.(GRID_OVERLAY_SELECTOR); + const root = getRootElement(); + if (overlay && (!root || !overlay.contains(root)) && activeCellRef.current) { + pendingReturnFocusCellRef.current = activeCellRef.current; + return; + } + // Focus landed OUTSIDE any overlay while a return is armed. When an overlay unmounts with focus + // still inside it, the browser fires no focusout for the removed node — the overlay's own + // returnFocus then lands on the trigger and this focusin is the only close signal we get. + scheduleReturnFocusCheck(); + }; + + const handleFocusOut = () => { + scheduleReturnFocusCheck(); + }; + document.addEventListener('focusin', handleFocusIn); document.addEventListener('focusout', handleFocusOut); return () => { document.removeEventListener('focusin', handleFocusIn); document.removeEventListener('focusout', handleFocusOut); }; - }, [getCellElement, getRootElement]); + }, [applySelection, getCellElement, getRootElement, table]); const handleRootFocus = useCallback( (event: ReactFocusEvent) => { @@ -533,9 +616,16 @@ export function useGridKeyboardNavigation({ } const rows = table.getRowModel().rows; const columns = table.getVisibleLeafColumns(); - if (rows.length && columns.length) { - interactionSourceRef.current = 'keyboard'; + if (!columns.length) { + return; + } + interactionSourceRef.current = 'keyboard'; + if (rows.length) { applySelection(rows[0].id, columns[0].id, false); + } else { + // Empty body (e.g. a filter excluded every row): seed the header row instead — the column + // filters are the only interactive surface left and must stay keyboard-reachable. + applySelection(HEADER_ROW_ID, columns[0].id, false); } }, [activeCell, table, applySelection], @@ -733,6 +823,14 @@ export function useGridKeyboardNavigation({ const handleKeyDown = useCallback( (event: ReactKeyboardEvent) => { + // React synthetic events bubble through PORTALS following the React tree, so keys pressed + // inside an overlay a cell renderer opened (record lookup popover) arrive here even though + // the overlay's DOM lives outside the grid. Those keys belong to the overlay — handling them + // as grid navigation re-activated the cell's control and toggle-closed the popover mid-press. + const rootElement = getRootElement(); + if (rootElement && event.target instanceof Node && !rootElement.contains(event.target)) { + return; + } // A focused text-entry control (the summary row's column filter input, a header filter's search // box, an open cell editor) owns every key it receives — caret movement, text selection and the // clipboard shortcuts. Without this the grid treated them as navigation and moved the active cell @@ -743,19 +841,20 @@ export function useGridKeyboardNavigation({ } const rows = table.getRowModel().rows; const columns = table.getVisibleLeafColumns(); - if (!rows.length || !columns.length) { + if (!columns.length) { return; } interactionSourceRef.current = 'keyboard'; - const current: ActiveCell = activeCell ?? { rowId: rows[0].id, columnId: columns[0].id }; + // Null only while the body is empty — the empty-body branch below returns before it is used. + const current: ActiveCell | null = activeCell ?? (rows.length ? { rowId: rows[0].id, columnId: columns[0].id } : null); const rowIndex = Math.max( 0, - rows.findIndex((row) => row.id === current.rowId), + rows.findIndex((row) => row.id === current?.rowId), ); const colIndex = Math.max( 0, - columns.findIndex((column) => column.id === current.columnId), + columns.findIndex((column) => column.id === current?.columnId), ); const ctrlOrMeta = event.ctrlKey || event.metaKey; const extend = event.shiftKey; @@ -783,6 +882,14 @@ export function useGridKeyboardNavigation({ } return; } + // Backstop for the same rule as the `event.target` guard at the top of this handler: a text-entry + // control owns its arrow keys (caret movement) and Up/Down (textarea lines, select options), so + // only Tab/Shift+Tab cycle the cell's controls from one. The guard above already covers every + // press whose target is the focused control — this catches the case where focus and the event + // target disagree, and must not be removed without re-checking R15 in the findings log. + if (isTextEntryElement(document.activeElement) && event.key !== 'Tab') { + return; + } const forward = event.key === 'Tab' ? !event.shiftKey : event.key === 'ArrowRight'; const backward = event.key === 'Tab' ? event.shiftKey : event.key === 'ArrowLeft'; if (forward || backward) { @@ -798,6 +905,14 @@ export function useGridKeyboardNavigation({ ); const nextIndex = (currentIndex + (backward ? -1 : 1) + controls.length) % controls.length; controls[nextIndex].focus(); + } else if (event.key === 'Tab') { + // A lone control (e.g. a summary-row filter input) has nothing to cycle to — treat Tab + // like Escape and return to the cell so focus stays inside the grid instead of exiting it. + // Only Tab: an arrow press with nothing to cycle is left alone rather than silently + // dropping the user out of Actionable mode without moving the active cell. + consume(); + setMode('navigation'); + cellEl?.focus(); } return; } @@ -835,22 +950,27 @@ export function useGridKeyboardNavigation({ case 'ArrowUp': consume(); break; + // Header cells honor HEADER colSpans (e.g. a profile-name group header spanning its + // Read/Edit sub-columns) — arrows step between RENDERED cells, not underlying tracks, + // otherwise a spanned header needs one press per covered column to cross. case 'ArrowRight': consume(); - applySelection(HEADER_ROW_ID, columns[clamp(headerColIndex + 1, 0, columns.length - 1)].id, false); + applySelection(HEADER_ROW_ID, columns[stepSegment(getHeaderSegmentStarts(columns), headerColIndex, 1)].id, false); break; case 'ArrowLeft': consume(); - applySelection(HEADER_ROW_ID, columns[clamp(headerColIndex - 1, 0, columns.length - 1)].id, false); + applySelection(HEADER_ROW_ID, columns[stepSegment(getHeaderSegmentStarts(columns), headerColIndex, -1)].id, false); break; case 'Home': consume(); applySelection(HEADER_ROW_ID, columns[0].id, false); break; - case 'End': + case 'End': { consume(); - applySelection(HEADER_ROW_ID, columns[columns.length - 1].id, false); + const headerStarts = getHeaderSegmentStarts(columns); + applySelection(HEADER_ROW_ID, columns[headerStarts[headerStarts.length - 1]].id, false); break; + } case 'Enter': case 'F2': // Let Cmd/Ctrl+Enter bubble to app-level handlers (e.g. save). @@ -913,7 +1033,7 @@ export function useGridKeyboardNavigation({ if (summaryIndex > 0) { applySelection(getSummaryRowId(summaryIndex - 1), columns[summaryColIndex].id, false, true); } else { - applySelection(HEADER_ROW_ID, columns[summaryColIndex].id, false, true); + applySelection(HEADER_ROW_ID, headerColumnIdFor(columns, columns[summaryColIndex].id), false, true); } break; case 'Escape': @@ -937,6 +1057,76 @@ export function useGridKeyboardNavigation({ return; } + // ── Empty body (a filter excluded every row, or no data has loaded) ── + // The header and summary rows are the only live surface left — route navigation keys there so + // the column filters stay reachable and the user can broaden the filter again. Without this the + // grid went completely dead (every key returned early) the moment a filter matched zero rows. + if (!rows.length || !current) { + switch (event.key) { + case 'ArrowUp': + case 'ArrowDown': + case 'ArrowLeft': + case 'ArrowRight': + case 'Home': + case 'End': + case 'PageUp': + case 'PageDown': { + consume(); + // Keep the column when the active cell references a row the filter just removed. + const columnId = current && columns.some((column) => column.id === current.columnId) ? current.columnId : columns[0].id; + applySelection( + summaryRowCount > 0 ? getSummaryRowId(summaryRowCount - 1) : HEADER_ROW_ID, + summaryRowCount > 0 ? columnId : headerColumnIdFor(columns, columnId), + false, + ); + break; + } + default: + break; + } + return; + } + + // Shift+Tab from an inner-focus widget: the widget (tabIndex -1) sits inside the active cell + // (tabIndex 0), so sequential navigation would stop on the cell before leaving the grid. Hand + // focus to the cell during the keydown and let the default action continue from there, so one + // press leaves the grid exactly as it does from a plain cell. + if ( + event.key === 'Tab' && + event.shiftKey && + activeCell && + event.target instanceof HTMLElement && + event.target.closest('[data-grid-inner-focus]') + ) { + getCellElement(activeCell)?.focus(); + return; + } + + // A cell marked for inner-widget focus (data-grid-inner-focus) keeps NATIVE activation: focus + // sits on the widget itself, so Enter/Space fire its click through the browser — the grid must + // neither consume the key (that would block the native click) nor also activate the cell + // (that would double-fire the action). + if ((event.key === 'Enter' || event.key === ' ') && !ctrlOrMeta && event.target instanceof HTMLElement) { + const innerFocusControl = event.target.closest('[data-grid-inner-focus]'); + if (innerFocusControl) { + // A focusable-but-not-activatable target (a tooltip trigger span) has no native Space action, so + // the browser would scroll the virtualized body — swallow it (the target's own keydown ran first). + const nativelyActivatable = innerFocusControl.matches('button, a[href], input, select, textarea, summary'); + if (!nativelyActivatable) { + if (event.key === ' ') { + consume(); + } + return; + } + // Space activates every control natively; Enter is native for buttons/links but a NO-OP on + // checkboxes — for those, fall through so the grid's activate path clicks the checkbox. + const enterIsNative = !(innerFocusControl instanceof HTMLInputElement && innerFocusControl.type === 'checkbox'); + if (event.key === ' ' || enterIsNative) { + return; + } + } + } + // ── Navigation mode ── // Vertical moves target the sticky desired column (not the possibly-snapped current column), so // passing through a group header or a spanned "no rows" row doesn't drag the user sideways. @@ -952,7 +1142,12 @@ export function useGridKeyboardNavigation({ // otherwise the column header row — so the keyboard can reach both. A range-extend (Shift) // stays in the body. if (rowIndex === 0 && !extend) { - applySelection(summaryRowCount > 0 ? getSummaryRowId(summaryRowCount - 1) : HEADER_ROW_ID, columns[desiredCol].id, false, true); + applySelection( + summaryRowCount > 0 ? getSummaryRowId(summaryRowCount - 1) : HEADER_ROW_ID, + summaryRowCount > 0 ? columns[desiredCol].id : headerColumnIdFor(columns, columns[desiredCol].id), + false, + true, + ); } else { moveTo(rowIndex - 1, desiredCol, extend, true); } @@ -970,7 +1165,10 @@ export function useGridKeyboardNavigation({ // Tree (real data row with children): Right expands a collapsed row. currentRow.toggleExpanded(); } else { - moveTo(rowIndex, colIndex + 1, extend); + // Segment-aware: from a cell that spans several columns (e.g. a full-width message row), + // +1 lands inside the same span and snaps back to its owner — step to the next rendered + // cell instead. Rows without spans get plain +1 (every column is a segment start). + moveTo(rowIndex, currentRow ? stepSegment(getRowSegmentStarts(currentRow, columns), colIndex, 1) : colIndex + 1, extend); } break; } @@ -991,7 +1189,7 @@ export function useGridKeyboardNavigation({ const parentIndex = parent ? rows.findIndex((row) => row.id === parent.id) : -1; moveTo(parentIndex >= 0 ? parentIndex : rowIndex, parentIndex >= 0 ? colIndex : colIndex - 1, extend); } else { - moveTo(rowIndex, colIndex - 1, extend); + moveTo(rowIndex, currentRow ? stepSegment(getRowSegmentStarts(currentRow, columns), colIndex, -1) : colIndex - 1, extend); } break; } @@ -1115,6 +1313,7 @@ export function useGridKeyboardNavigation({ onUndo, onRedo, onClearSelection, + getRootElement, ], ); diff --git a/libs/ui/src/lib/data-table/grid/keyboard/useGridTabOrderContainment.ts b/libs/ui/src/lib/data-table/grid/keyboard/useGridTabOrderContainment.ts new file mode 100644 index 000000000..c11d0e4ec --- /dev/null +++ b/libs/ui/src/lib/data-table/grid/keyboard/useGridTabOrderContainment.ts @@ -0,0 +1,66 @@ +import { useEffect } from 'react'; + +/** Cells manage their own roving tabindex (0 on the active cell, -1 otherwise) — never touch them. */ +const CELL_SELECTOR = '[role="gridcell"], [role="rowheader"], [role="columnheader"]'; + +/** Anything the browser would put in the page tab order. */ +const FOCUSABLE_SELECTOR = 'a[href], area[href], button, input, select, textarea, [tabindex]'; + +function removeFromTabOrder(element: Element) { + if (!(element instanceof HTMLElement) || element.tabIndex === -1) { + return; + } + // Escape hatch for controls OUTSIDE the cell navigation model (e.g. the permission manager's + // column-group-header popover trigger) — grid keyboard navigation never visits group headers, + // so Tab is the only way to reach them. + if (element.hasAttribute('data-grid-keep-tab-stop')) { + return; + } + // Only sweep content INSIDE a cell: the cells themselves (and the grid root) own the roving tabindex. + if (element.matches(CELL_SELECTOR) || !element.closest(CELL_SELECTOR)) { + return; + } + element.tabIndex = -1; +} + +function sweep(scope: Element) { + removeFromTabOrder(scope); + scope.querySelectorAll(FOCUSABLE_SELECTOR).forEach(removeFromTabOrder); +} + +/** + * Keeps the grid a single page tab stop by removing focusable elements that consumer cell renderers + * mount inside cells (links, buttons, inputs) from the tab order. The keyboard model reaches them + * through Enter/Space activation and Actionable mode instead (see useGridKeyboardNavigation) — + * `.focus()`/`.click()` work regardless of tabindex. + * + * The grid's built-in renderers already set `tabIndex={-1}` on their controls; this is the safety net + * for the arbitrary content consumers render (e.g. record links), which would otherwise turn a large + * table into hundreds of tab stops. A MutationObserver handles content that mounts after the cell — + * e.g. a link that appears once an async URL resolves — and virtualized rows scrolling into view. + */ +export function useGridTabOrderContainment(getRootElement: () => HTMLElement | null) { + useEffect(() => { + const root = getRootElement(); + if (!root || typeof MutationObserver === 'undefined') { + return; + } + sweep(root); + const observer = new MutationObserver((mutations) => { + for (const mutation of mutations) { + if (mutation.type === 'childList') { + mutation.addedNodes.forEach((node) => { + if (node instanceof Element) { + sweep(node); + } + }); + } else if (mutation.target instanceof Element) { + // An existing element became focusable (gained an href, or a renderer set tabindex >= 0). + removeFromTabOrder(mutation.target); + } + } + }); + observer.observe(root, { childList: true, subtree: true, attributes: true, attributeFilter: ['href', 'tabindex'] }); + return () => observer.disconnect(); + }, [getRootElement]); +} diff --git a/libs/ui/src/lib/data-table/grid/renderers/CellRenderers.tsx b/libs/ui/src/lib/data-table/grid/renderers/CellRenderers.tsx index a75bc5605..7e6f68516 100644 --- a/libs/ui/src/lib/data-table/grid/renderers/CellRenderers.tsx +++ b/libs/ui/src/lib/data-table/grid/renderers/CellRenderers.tsx @@ -52,7 +52,8 @@ export function BooleanRenderer({ column, row }: DataTableCellProps): React className="slds-align_absolute-center" id={`${column.key}-${getRowId(row)}`} checked={value} - label="value" + // Named after the column so screen readers hear e.g. "Read, checkbox, ticked" instead of "value" + label={typeof column.name === 'string' && column.name.trim() ? column.name : 'value'} hideLabel readOnly /> @@ -165,35 +166,60 @@ export function ActionRenderer({ row }: DataTableCellProps): ReactNode { - - - {isDeleted ? ( - ) : ( - )} - diff --git a/libs/ui/src/lib/data-table/grid/renderers/SubqueryRenderer.tsx b/libs/ui/src/lib/data-table/grid/renderers/SubqueryRenderer.tsx index 95ebdd4ac..0d9a4d636 100644 --- a/libs/ui/src/lib/data-table/grid/renderers/SubqueryRenderer.tsx +++ b/libs/ui/src/lib/data-table/grid/renderers/SubqueryRenderer.tsx @@ -18,7 +18,7 @@ import Spinner from '../../../widgets/Spinner'; import Tooltip from '../../../widgets/Tooltip'; import { DataTableV2 } from '../DataTableV2'; import { copySalesforceRecordTableDataToClipboard } from '../grid-clipboard'; -import { NON_DATA_COLUMN_KEYS, TABLE_CONTEXT_MENU_ITEMS } from '../grid-constants'; +import { DEFAULT_ROW_HEIGHT, NON_DATA_COLUMN_KEYS, TABLE_CONTEXT_MENU_ITEMS } from '../grid-constants'; import { GridSubqueryContext } from '../grid-context'; import { getRowId, getSubqueryModalTagline, replaceSubqueryOnRecord } from '../grid-row-utils'; import { ContextAction, ContextMenuActionData, DataTableCellProps, RowWithKey, SubqueryContext, SubqueryLevel } from '../grid-types'; @@ -501,7 +501,7 @@ function SubqueryModal({ data={rows} columns={columns} getRowKey={getRowId} - rowHeight={28.5} + rowHeight={DEFAULT_ROW_HEIGHT} enableRowSelection rowSelection={rowSelection} onRowSelectionChange={setRowSelection} diff --git a/libs/ui/src/lib/docked-composer/DockedComposer.tsx b/libs/ui/src/lib/docked-composer/DockedComposer.tsx index 8436a5145..75cece4dc 100644 --- a/libs/ui/src/lib/docked-composer/DockedComposer.tsx +++ b/libs/ui/src/lib/docked-composer/DockedComposer.tsx @@ -72,30 +72,45 @@ export const DockedComposer = forwardRef( className={classNames('slds-docked-composer slds-grid slds-grid_vertical', { 'slds-is-open': isOpen, 'slds-is-closed': !isOpen })} role="dialog" aria-labelledby={id} - aria-describedby={`${id}-content`} + // The body only exists once the composer has been opened; a dangling describedby is an axe violation + aria-describedby={isOpen || hasOpened ? `${id}-content` : undefined} > -
+ {/* Pointer-only convenience: clicking the title bar toggles the composer. The keyboard path is the + always-rendered expand/minimize button below. */} + {/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-noninteractive-element-interactions, jsx-a11y/no-static-element-interactions */} +
{!iconOverride && } {!!iconOverride && iconOverride}
- {!isOpen && Minimized} + {/* Persistent status region (not the whole header) so only the state change is announced */} + + {isOpen ? '' : 'Minimized'} +

{label}

- {allowMinimize && isOpen && ( - )} diff --git a/libs/ui/src/lib/expression-group/ExpressionActionDropDown.tsx b/libs/ui/src/lib/expression-group/ExpressionActionDropDown.tsx index 6694ad5bf..01a964411 100644 --- a/libs/ui/src/lib/expression-group/ExpressionActionDropDown.tsx +++ b/libs/ui/src/lib/expression-group/ExpressionActionDropDown.tsx @@ -6,6 +6,8 @@ export interface ExpressionActionDropDownProps { label: string; helpText?: string; value: AndOr; + /** Keep the label for assistive technology only (condition groups show their number in a legend instead) */ + hideLabel?: boolean; ancillaryOptions?: React.ReactNode; onChange: (value: AndOr) => void; } @@ -26,6 +28,7 @@ function getInitSelected(value: AndOr) { export const ExpressionActionDropDown: FunctionComponent = ({ label, + hideLabel, helpText, value, ancillaryOptions, @@ -37,6 +40,7 @@ export const ExpressionActionDropDown: FunctionComponent = React.me {parentAction} {`Condition Group ${group}`} - + {children}
diff --git a/libs/ui/src/lib/file-download-modal/FileFauxDownloadModal.tsx b/libs/ui/src/lib/file-download-modal/FileFauxDownloadModal.tsx index 27896c61e..bed6dd6e0 100644 --- a/libs/ui/src/lib/file-download-modal/FileFauxDownloadModal.tsx +++ b/libs/ui/src/lib/file-download-modal/FileFauxDownloadModal.tsx @@ -169,8 +169,12 @@ export const FileFauxDownloadModal: FunctionComponent) { + // Enter in the filename input downloads — on keydown, never keyup: the modal opens with this input + // focused, so the keyup of the Enter that activated the opening button lands here and used to + // download and close the modal before it was ever seen + function handleFilenameKeyDown(event: KeyboardEvent) { if (isEnterKey(event) && !filenameEmpty) { + event.preventDefault(); handleDownload(); } } @@ -285,7 +289,7 @@ export const FileFauxDownloadModal: FunctionComponent setFileName(event.target.value)} - onKeyUp={handleKeyUp} + onKeyDown={handleFilenameKeyDown} />
diff --git a/libs/ui/src/lib/file-download-modal/RecordDownloadModal.tsx b/libs/ui/src/lib/file-download-modal/RecordDownloadModal.tsx index 643b217aa..e9b13ebe7 100644 --- a/libs/ui/src/lib/file-download-modal/RecordDownloadModal.tsx +++ b/libs/ui/src/lib/file-download-modal/RecordDownloadModal.tsx @@ -561,8 +561,12 @@ export const RecordDownloadModal: FunctionComponent = return hasSelectableSubset(selectedRecords, records); } - function handleKeyUp(event: KeyboardEvent) { + // Enter in the filename input downloads — on keydown, never keyup: the modal opens with this input + // focused, so the keyup of the Enter that activated the opening button lands here and used to + // download and close the modal before it was ever seen + function handleFilenameKeyDown(event: KeyboardEvent) { if (isEnterKey(event) && !invalidConfig && !isLoadingChildRelationships) { + event.preventDefault(); handleDownload(); } } @@ -817,6 +821,8 @@ export const RecordDownloadModal: FunctionComponent = label="Filename" isRequired rightAddon={fileFormat !== RADIO_FORMAT_GDRIVE ? `.${fileExtension}` : undefined} + // Without hasError the required message could never render — an empty name only silently disabled Download + hasError={!fileName} errorMessage="This field is required" errorMessageId="filename-error" > @@ -828,7 +834,7 @@ export const RecordDownloadModal: FunctionComponent = minLength={1} maxLength={250} onChange={(event) => setFileName(event.target.value)} - onKeyUp={handleKeyUp} + onKeyDown={handleFilenameKeyDown} />
diff --git a/libs/ui/src/lib/file-download-modal/__tests__/RecordDownloadModal.spec.tsx b/libs/ui/src/lib/file-download-modal/__tests__/RecordDownloadModal.spec.tsx index 4840e718f..eff25afe4 100644 --- a/libs/ui/src/lib/file-download-modal/__tests__/RecordDownloadModal.spec.tsx +++ b/libs/ui/src/lib/file-download-modal/__tests__/RecordDownloadModal.spec.tsx @@ -1,5 +1,5 @@ import { SalesforceOrgUi } from '@jetstream/types'; -import { render, screen } from '@testing-library/react'; +import { fireEvent, render, screen } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { MemoryRouter } from 'react-router'; import { beforeEach, describe, expect, test, vi } from 'vitest'; @@ -134,3 +134,23 @@ describe('RecordDownloadModal bulk API requirement', () => { expect(screen.getByLabelText('Standard').disabled).toBe(false); }); }); + +describe('RecordDownloadModal filename Enter', () => { + beforeEach(() => { + localStorage.clear(); + vi.clearAllMocks(); + }); + + // The modal opens with the filename input focused, so the keyup of the Enter that activated the + // opening button lands in it — that keyup must not download + test('downloads on Enter keydown in the filename input, never on the keyup alone', () => { + const { onDownload } = setup(); + const filename = document.getElementById('download-filename') as HTMLInputElement; + + fireEvent.keyUp(filename, { key: 'Enter' }); + expect(onDownload).not.toHaveBeenCalled(); + + expect(fireEvent.keyDown(filename, { key: 'Enter' })).toBe(false); + expect(onDownload).toHaveBeenCalledTimes(1); + }); +}); diff --git a/libs/ui/src/lib/form/__tests__/form-error-semantics.spec.tsx b/libs/ui/src/lib/form/__tests__/form-error-semantics.spec.tsx new file mode 100644 index 000000000..a45ee4d33 --- /dev/null +++ b/libs/ui/src/lib/form/__tests__/form-error-semantics.spec.tsx @@ -0,0 +1,64 @@ +import { axeScan } from '@jetstream/test-utils'; +import { render, screen } from '@testing-library/react'; +import { describe, expect, test } from 'vitest'; +import { Checkbox } from '../checkbox/Checkbox'; +import { Combobox } from '../combobox/Combobox'; +import { DatePicker } from '../date/DatePicker'; +import { Picklist } from '../picklist/Picklist'; +import { Radio } from '../radio/Radio'; +import { RadioGroup } from '../radio/RadioGroup'; +import { Slider } from '../slider/Slider'; + +const NOOP = () => undefined; + +/** + * Every control that renders an error message must also flag itself invalid: linking the message + * with aria-describedby alone reads the text but never tells the user the field has a problem. + */ +describe('form controls in an error state', () => { + test.each([ + { + name: 'Combobox', + render: () => ( + + ), + role: 'combobox', + }, + { + name: 'Picklist', + render: () => ( + + ), + role: 'combobox', + }, + { + name: 'Checkbox', + render: () => , + role: 'checkbox', + }, + { + name: 'DatePicker', + render: () => , + role: 'textbox', + }, + { + name: 'RadioGroup', + render: () => ( + + + + ), + role: 'radiogroup', + }, + { + name: 'Slider', + render: () => , + role: 'slider', + }, + ])('$name exposes aria-invalid and its error message', async ({ render: renderControl, role }) => { + const { baseElement } = render(renderControl()); + const control = screen.getByRole(role); + expect(control.getAttribute('aria-invalid')).toBe('true'); + await axeScan(baseElement); + }); +}); diff --git a/libs/ui/src/lib/form/button/FormRowButton.tsx b/libs/ui/src/lib/form/button/FormRowButton.tsx index 163793037..d336e2a46 100644 --- a/libs/ui/src/lib/form/button/FormRowButton.tsx +++ b/libs/ui/src/lib/form/button/FormRowButton.tsx @@ -16,9 +16,18 @@ export interface FormRowButtonProps { export const FormRowButton: FunctionComponent = ({ title, icon, onClick }) => { return (
- + {/* Invisible label mirrors the real labels' exact height so the button aligns with sibling + inputs — the old fixed 15px margin sat a couple px short of a real label row */} +
-
diff --git a/libs/ui/src/lib/form/button/UpgradeToProButton.tsx b/libs/ui/src/lib/form/button/UpgradeToProButton.tsx index bffe8625e..fb0ad8f1c 100644 --- a/libs/ui/src/lib/form/button/UpgradeToProButton.tsx +++ b/libs/ui/src/lib/form/button/UpgradeToProButton.tsx @@ -25,7 +25,8 @@ export const UpgradeToProButton = ({ showOpenInNewTabIcon, source, trackEvent }: { + afterEach(() => { + document.body.innerHTML = ''; + }); + + it('runs the handler and lets the click reach ancestors while enabled', async () => { + const onClick = vi.fn(); + const { container, onAncestorClick } = listenAboveReact(); + const { baseElement } = render( + , + { container }, + ); + + fireEvent.click(screen.getByRole('button', { name: 'Save' })); + + expect(onClick).toHaveBeenCalledTimes(1); + expect(onAncestorClick).toHaveBeenCalledTimes(1); + expect(screen.getByRole('button', { name: 'Save' }).getAttribute('aria-disabled')).toBeNull(); + await axeScan(baseElement); + }); + + it('while disabled, blocks the handler and hides the click from ancestors, like a native disabled button', async () => { + const onClick = vi.fn(); + const { container, onAncestorClick } = listenAboveReact(); + const { baseElement } = render( + , + { container }, + ); + const button = screen.getByRole('button', { name: 'Save' }); + button.focus(); + + const notPrevented = fireEvent.click(button); + + expect(onClick).not.toHaveBeenCalled(); + expect(onAncestorClick).not.toHaveBeenCalled(); + expect(notPrevented).toBe(false); + expect(button.getAttribute('aria-disabled')).toBe('true'); + expect(document.activeElement).toBe(button); + await axeScan(baseElement); + }); +}); diff --git a/libs/ui/src/lib/form/button/aria-disabled-button.utils.ts b/libs/ui/src/lib/form/button/aria-disabled-button.utils.ts new file mode 100644 index 000000000..08a32df7f --- /dev/null +++ b/libs/ui/src/lib/form/button/aria-disabled-button.utils.ts @@ -0,0 +1,36 @@ +import { MouseEvent } from 'react'; + +interface AriaDisabledButtonProps { + 'aria-disabled': true | undefined; + onClick: (event: MouseEvent) => void; +} + +/** + * Props for an action control that must stay focusable while disabled. The native `disabled` + * attribute drops focus to `` the moment the condition flips under the focused element + * (e.g. clicking "Next" disables it on the final step), which restarts keyboard navigation from + * the top of the page. `aria-disabled` keeps focus and announces the state instead; the CSS in + * `ui-styles/main.css` mirrors SLDS `:disabled` styling for `[aria-disabled='true']`. + * + * `aria-disabled` is not enforced by the browser — and the CSS `pointer-events: none` does not + * block keyboard-initiated clicks — so the click handler is guarded here. Callers must spread + * these props INSTEAD of attaching their own `onClick`/`aria-disabled`. `preventDefault()` on the + * disabled path also makes this safe for link-shaped actions. + */ +export function ariaDisabledButtonProps( + disabled: boolean | undefined, + onClick: (event: MouseEvent) => void, +): AriaDisabledButtonProps { + return { + 'aria-disabled': disabled || undefined, + onClick: (event) => { + if (disabled) { + // A native disabled button emits no click at all, so ancestors never see one — match that + event.preventDefault(); + event.stopPropagation(); + return; + } + onClick(event); + }, + }; +} diff --git a/libs/ui/src/lib/form/checkbox-toggle/CheckboxToggle.tsx b/libs/ui/src/lib/form/checkbox-toggle/CheckboxToggle.tsx index 11f7a72d1..758fc5422 100644 --- a/libs/ui/src/lib/form/checkbox-toggle/CheckboxToggle.tsx +++ b/libs/ui/src/lib/form/checkbox-toggle/CheckboxToggle.tsx @@ -1,3 +1,4 @@ +import { css } from '@emotion/react'; import { RightLeft } from '@jetstream/types'; import classNames from 'classnames'; import { FunctionComponent, HTMLAttributes } from 'react'; @@ -16,6 +17,13 @@ export interface CheckboxCheckboxToggleProps { containerClassname?: string; labelClassname?: string; extraProps?: HTMLAttributes; + /** + * Set both when the toggle reveals content below it, so screen readers announce it as + * expanded/collapsed and can jump to what it controls. `ariaControls` is the id of the revealed + * region, which should stay in the DOM (empty when collapsed) so the reference always resolves. + */ + ariaExpanded?: boolean; + ariaControls?: string; onChange?: (value: boolean) => void; } @@ -32,8 +40,11 @@ export const CheckboxToggle: FunctionComponent = ({ containerClassname, labelClassname, extraProps, + ariaExpanded, + ariaControls, onChange, }) => { + const stateId = `${id}-state`; const handleChange = () => { if (disabled || !onChange) { return; @@ -41,27 +52,59 @@ export const CheckboxToggle: FunctionComponent = ({ onChange(!checked); }; + // Both labels point at the input, so its accessible name is the label text plus the visible + // on/off state — the same name the single wrapping label produced. A hidden label keeps the name. + const labelText = ( + + ); + return (
-
); }; diff --git a/libs/ui/src/lib/form/checkbox-toggle/__tests__/CheckboxToggle.spec.tsx b/libs/ui/src/lib/form/checkbox-toggle/__tests__/CheckboxToggle.spec.tsx new file mode 100644 index 000000000..22fde1549 --- /dev/null +++ b/libs/ui/src/lib/form/checkbox-toggle/__tests__/CheckboxToggle.spec.tsx @@ -0,0 +1,89 @@ +import { axeScan } from '@jetstream/test-utils'; +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import CheckboxToggle from '../CheckboxToggle'; + +describe('CheckboxToggle', () => { + test('Space toggles exactly once when the checkbox is focused', async () => { + const onChange = vi.fn(); + render(); + + screen.getByRole('checkbox').focus(); + await userEvent.keyboard(' '); + + expect(onChange).toHaveBeenCalledTimes(1); + expect(onChange).toHaveBeenCalledWith(true); + }); + + test('clicking the label text toggles exactly once', async () => { + const onChange = vi.fn(); + render(); + + await userEvent.click(screen.getByText('Include deleted records')); + + expect(onChange).toHaveBeenCalledTimes(1); + expect(onChange).toHaveBeenCalledWith(true); + }); + + test('with labelHelp, the label text still toggles the checkbox and names it (help button is not the label control)', async () => { + const onChange = vi.fn(); + render( + , + ); + + expect(screen.getByRole('checkbox', { name: /Include deleted records/ })).toBeTruthy(); + await userEvent.click(screen.getByText('Include deleted records')); + + expect(onChange).toHaveBeenCalledTimes(1); + }); + + test('a hidden label still names the checkbox', () => { + render(); + + expect(screen.getByRole('checkbox', { name: /Include deleted records/ })).toBeTruthy(); + }); + + test('does not toggle when disabled', async () => { + const onChange = vi.fn(); + render(); + + await userEvent.click(screen.getByText('Include deleted records')); + + expect(onChange).not.toHaveBeenCalled(); + }); + + test('has no axe violations', async () => { + const { baseElement } = render( +
+ +
, + ); + const results = await axeScan(baseElement); + expect(results.violations).toEqual([]); + }); + + test('exposes disclosure state when it reveals content below it', () => { + const { rerender } = render( + , + ); + const checkbox = screen.getByRole('checkbox'); + expect(checkbox.getAttribute('aria-expanded')).toBe('false'); + expect(checkbox.getAttribute('aria-controls')).toBe('advanced'); + + rerender(); + expect(checkbox.getAttribute('aria-expanded')).toBe('true'); + }); +}); diff --git a/libs/ui/src/lib/form/checkbox/Checkbox.tsx b/libs/ui/src/lib/form/checkbox/Checkbox.tsx index 2ac61e535..690e46eca 100644 --- a/libs/ui/src/lib/form/checkbox/Checkbox.tsx +++ b/libs/ui/src/lib/form/checkbox/Checkbox.tsx @@ -23,6 +23,8 @@ export interface CheckboxProps { isStandAlone?: boolean; errorMessageId?: string; errorMessage?: React.ReactNode | string; + /** Extra attributes for the native input (e.g. data-grid-inner-focus for grid cells) */ + inputProps?: React.InputHTMLAttributes & { [dataAttribute: `data-${string}`]: string | boolean | undefined }; onChange?: (value: boolean) => void; onChangeNative?: (event: SyntheticEvent) => void; onBlur?: () => void; @@ -48,6 +50,7 @@ export const Checkbox: FunctionComponent = ({ tabIndex, hideLabel = false, isStandAlone = false, + inputProps, onChange, onChangeNative, onBlur, @@ -94,16 +97,18 @@ export const Checkbox: FunctionComponent = ({ type="checkbox" name="options" id={id} + aria-invalid={hasError || undefined} checked={checked || false} disabled={readOnly || disabled} readOnly={readOnly} tabIndex={tabIndex} - aria-describedby={errorMessageId} + aria-describedby={[labelHelp ? `${id}-label-help-text` : undefined, errorMessageId].filter(Boolean).join(' ') || undefined} onChange={(event) => { onChange && onChange(event.target?.checked || false); onChangeNative && onChangeNative(event); }} onBlur={onBlur} + {...inputProps} /> {isStandAlone && } {!isStandAlone && ( diff --git a/libs/ui/src/lib/form/combobox/Combobox.tsx b/libs/ui/src/lib/form/combobox/Combobox.tsx index 45906b90d..b13cb4b1b 100644 --- a/libs/ui/src/lib/form/combobox/Combobox.tsx +++ b/libs/ui/src/lib/form/combobox/Combobox.tsx @@ -9,6 +9,7 @@ import { isEnterKey, isEnterOrSpace, isEscapeKey, + isTabKey, } from '@jetstream/shared/ui-utils'; import { NOOP } from '@jetstream/shared/utils'; import { DropDownItemLength } from '@jetstream/types'; @@ -26,6 +27,7 @@ import React, { useRef, useState, } from 'react'; +import { useEscapeToCloseLayer } from '../../hooks/useEscapeToCloseLayer'; import PopoverContainer from '../../popover/PopoverContainer'; import HelpText from '../../widgets/HelpText'; import Icon from '../../widgets/Icon'; @@ -35,6 +37,7 @@ import { ComboboxListItem } from './ComboboxListItem'; export interface ComboboxPropsRef { clearInputText(): void; + focusInput(): void; getRefs(): { inputEl: React.RefObject; divContainerEl: React.RefObject; @@ -76,8 +79,8 @@ export interface ComboboxProps { * Shows a dropdown at beginning to choose between different types of items. * {@link https://www.lightningdesignsystem.com/components/combobox/?variant=deprecated-multi-entity#Grouped-Comboboxes-(Cross-entity-Polymorphic)} */ - leadingDropdown?: Omit; - trailingDropdown?: Omit; + leadingDropdown?: Omit; + trailingDropdown?: Omit; /** * Depending on how Combobox is used, isEmpty may not be able to be automatically calculated. * if so, Set this field to true if you know there are no items in the list. @@ -96,6 +99,11 @@ export interface ComboboxProps { * This requires `onClear()` to be set, otherwise value cannot be cleared. */ showSelectionAsButton?: boolean; + /** + * Render a clear (X) button in place of the chevron while an item is selected, WITHOUT the + * selection-as-button behavior above (the input stays clickable/typeable). Requires `onClear()`. + */ + showClearButton?: boolean; /** * If using virtual list, this ensures child detection for keyboard navigation is correct. */ @@ -166,6 +174,7 @@ export const Combobox = forwardRef( errorMessageId, errorMessage, showSelectionAsButton, + showClearButton, isVirtual, usePortal, dropdownWidth, @@ -253,6 +262,37 @@ export const Combobox = forwardRef( // eslint-disable-next-line react-hooks/exhaustive-deps }, [selectedItemLabel]); + /** + * Where the Enter press in flight started. Enter on the input is acted on at keyup (below), but a + * keyup only means something when this input also saw the keydown: + * - 'opened': the keydown opened the closed list (Lightning's combobox and the APG select-only + * combobox both do; Picklist and DropDown already did) — its keyup must not pick the first option + * - 'held': the keydown landed here with the list open — its keyup picks the first option + * - null: the keydown landed on an option. The list selects on keydown and hands focus back to + * the input (after a short delay for drill-in items), so the keyup that then arrives here is the + * tail of that press; acting on it picked the first option of the new list over the one chosen. + * A modified Enter (Cmd/Ctrl/Alt) on the closed input is left alone for page-level shortcuts. + */ + const enterPressRef = useRef<'opened' | 'held' | null>(null); + + /** + * Enter never reaches a wrapping form from the input: closed it opens the list, open it selects + * (handled on keyup below). Marking the keydown as handled also keeps page-level Cmd/Ctrl+Enter + * shortcuts (which honour defaultPrevented) from firing on the same press. + */ + function handleInputKeyDown(event: KeyboardEvent) { + const isPlainEnter = isEnterKey(event) && !event.metaKey && !event.ctrlKey && !event.altKey; + const opensList = isPlainEnter && !isOpen && !disabled && !preventOpen; + enterPressRef.current = isEnterKey(event) ? (opensList ? 'opened' : 'held') : null; + if (opensList) { + setIsOpen(true); + } + if (isEnterKey(event) && (isOpen || opensList)) { + event.preventDefault(); + } + inputProps?.onKeyDown?.(event); + } + /** * When on input, move focus down the first list item */ @@ -261,9 +301,17 @@ export const Combobox = forwardRef( return; } if (isEscapeKey(event)) { - // Escape is handled on keydown (see handleInputKeyDown) so it can preempt an ancestor's - // keydown listener (e.g. a Panel with closeOnEscape). Ignore it here so it does not fall - // through to the onInputChange/onFilterInputChange branch below. + // While open, Escape is fully consumed by useEscapeToCloseLayer (keydown AND keyup); this + // guard covers the CLOSED state, where the keyup must not fall through to the + // onInputChange/onFilterInputChange branch below. + return; + } + if (isEnterKey(event)) { + const press = enterPressRef.current; + enterPressRef.current = null; + if (press === 'held' && isOpen && onInputEnter) { + onInputEnter(); + } return; } if (isArrowUpKey(event)) { @@ -272,8 +320,6 @@ export const Combobox = forwardRef( } else if (isArrowDownKey(event)) { !isOpen && setIsOpen(true); onKeyboardNavigation('down'); - } else if (isEnterKey(event) && isOpen && onInputEnter) { - onInputEnter(); } else { if (isAlphaNumericKey(event) && !isOpen) { // save input so that when we open, we can set the value instead of clearing it @@ -285,22 +331,13 @@ export const Combobox = forwardRef( } } - /** - * Handle Escape on keydown so an open dropdown closes before any ancestor keydown listener - * (e.g. a Panel with closeOnEscape) reacts. When the dropdown is already closed, Escape is left - * to bubble so the ancestor can handle it. - */ - function handleInputKeyDown(event: KeyboardEvent) { - if (disabled || preventOpen) { - return; - } - if (isEscapeKey(event) && isOpen) { - event.stopPropagation(); - event.preventDefault(); - setIsOpen(false); - onClose && onClose(); - } - } + // Escape closes ONLY this menu (and returns focus to the input) — consumed at document capture + // so an ancestor modal/popover cannot also close on the same press + useEscapeToCloseLayer(isOpen, () => { + setIsOpen(false); + onClose && onClose(); + inputEl.current?.focus(); + }); /** * Handle keyboard interaction when list items have focus @@ -308,18 +345,28 @@ export const Combobox = forwardRef( */ function handleListKeyDown(event: KeyboardEvent) { try { - if (isOpen && isEscapeKey(event)) { + // Escape is deliberately absent: the list only has focus while open, and + // useEscapeToCloseLayer consumes Escape at document capture for that state + if (isEnterOrSpace(event)) { event.preventDefault(); event.stopPropagation(); + onKeyboardNavigation('enter'); + return; + } + // Tab leaves the combobox: hand focus back to the input WITHOUT preventDefault so the browser's + // sequential navigation continues from the input (with usePortal the option list lives at the + // end of the portal root, so the default Tab would jump to the end of the page) and close + if (isTabKey(event)) { + inputEl.current?.focus(); setIsOpen(false); onClose && onClose(); - inputEl.current?.focus(); return; } - if (isEnterOrSpace(event)) { - event.preventDefault(); - event.stopPropagation(); - onKeyboardNavigation('enter'); + // Typing while an option has focus returns focus to the input so the keystroke lands there and + // filters the list (focus moves during keydown, so the browser inserts the character into the + // input) — the APG "focus moves to the option" pattern still expects typing to filter + if (!event.metaKey && !event.ctrlKey && !event.altKey && (isAlphaNumericKey(event) || event.key === 'Backspace')) { + inputEl.current?.focus(); return; } if (isArrowUpKey(event)) { @@ -341,7 +388,13 @@ export const Combobox = forwardRef( } const handleBlur = (event: FocusEvent) => { - if (entireContainerEl.current?.contains(event.relatedTarget as Node)) { + // With usePortal the option list (which receives real focus during arrow navigation) is NOT + // inside the container — without the popover check, the first arrow press closed the menu + if (entireContainerEl.current?.contains(event.relatedTarget as Node) || popoverRef.current?.contains(event.relatedTarget as Node)) { + return; + } + // Already closed (Tab from an option closes and reports it before focus leaves): nothing to report + if (!isOpen) { return; } setIsOpen(false); @@ -366,9 +419,10 @@ export const Combobox = forwardRef( }; const iconNotLoading = - showSelectionAsButton && onClear && selectedItemLabel ? ( + (showSelectionAsButton || showClearButton) && onClear && selectedItemLabel ? (
)} diff --git a/libs/ui/src/lib/form/combobox/ComboboxWithGroupedItems.tsx b/libs/ui/src/lib/form/combobox/ComboboxWithGroupedItems.tsx index 32d3f882d..39ec2a464 100644 --- a/libs/ui/src/lib/form/combobox/ComboboxWithGroupedItems.tsx +++ b/libs/ui/src/lib/form/combobox/ComboboxWithGroupedItems.tsx @@ -14,6 +14,7 @@ const defaultSelectedItemTitleFn = (item: ListItem) => item.title; export interface ComboboxWithGroupedItemsRef { clearSearchTerm: () => void; + focusInput: () => void; } export interface ComboboxWithGroupedItemsProps { @@ -64,7 +65,10 @@ export const ComboboxWithGroupedItems = forwardRef { const comboboxRef = useRef(null); const [filterTextNonDebounced, setFilterText] = useState(''); - const filterText = useDebounce(filterTextNonDebounced, 300); + const debouncedFilterText = useDebounce(filterTextNonDebounced, 300); + // Clearing applies immediately (M13): a debounced reset showed the previous search's subset for + // 300ms when the list was cleared or reopened — same rule as ComboboxWithItems + const filterText = filterTextNonDebounced ? debouncedFilterText : ''; const [visibleItems, setVisibleItems] = useState(groups); // Derived during render rather than mirrored into state with effects — see ComboboxWithItems // for why the effect-synced version risked "Maximum update depth exceeded". @@ -82,6 +86,9 @@ export const ComboboxWithGroupedItems = forwardRef { + comboboxRef.current?.focusInput(); + }, }), [], ); @@ -106,10 +113,14 @@ export const ComboboxWithGroupedItems = forwardRef { - const items = visibleItems.flatMap((group) => group.items); - if (items.length > 0) { - onSelected(items[0]); + const item = visibleItems.flatMap((group) => group.items).find((groupItem) => !groupItem.disabled); + if (item) { + onSelected(item); + if (!item.isDrillInItem) { + comboboxRef.current?.close(); + } } }, [onSelected, visibleItems]); @@ -155,6 +166,11 @@ export const ComboboxWithGroupedItems = forwardRef { + setFilterText(''); + setFocusedIndex(null); + comboboxRef.current?.clearInputText(); + comboboxProps.onClose?.(); + onClose?.(); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [comboboxProps.onClose, onClose]); + + // Typing changes (and re-orders) visibleItems, so a focused index from before the filter + // points at a different item — without the reset, ArrowDown resumed from the stale position + const handleInputChange = useCallback((value: string) => { + setFilterText(value); + setFocusedIndex(null); + }, []); + + // Enter from the input picks the first selectable option. Close explicitly: the close-on-selection + // effect only fires when the selected label CHANGES, so re-selecting the current item left the list open const onInputEnter = useCallback(() => { - if (visibleItems.length > 0) { - onSelected(visibleItems[0]); + const item = visibleItems.find((visibleItem) => !visibleItem.disabled); + if (item) { + onSelected(item); + if (!item.isDrillInItem) { + comboboxRef.current?.close(); + } } }, [onSelected, visibleItems]); @@ -132,6 +158,11 @@ export const ComboboxWithItems = forwardRef {heading && } {visibleItems.map((item, i) => @@ -218,7 +248,6 @@ export const ComboboxWithItems = forwardRef setLoading(false)); }, [filterText, onSearch]); + // Enter from the input picks the first selectable option (see ComboboxWithItems for why it closes explicitly) const onInputEnter = useCallback(() => { - if (items.length > 0) { - onSelected(items[0]); + const item = items.find((listItem) => !listItem.disabled); + if (item) { + onSelected(item); + comboboxRef.current?.close(); } }, [onSelected, items]); @@ -93,6 +96,11 @@ export const ComboboxWithItemsTypeAhead: FunctionComponent { + setFocusedIndex(null); + comboboxProps.onClose?.(); + onClose?.(); + }} onInputEnter={onInputEnter} onClear={handleClear} showSelectionAsButton diff --git a/libs/ui/src/lib/form/combobox/ComboboxWithItemsVirtual.tsx b/libs/ui/src/lib/form/combobox/ComboboxWithItemsVirtual.tsx index 45e7791c5..c809ea579 100644 --- a/libs/ui/src/lib/form/combobox/ComboboxWithItemsVirtual.tsx +++ b/libs/ui/src/lib/form/combobox/ComboboxWithItemsVirtual.tsx @@ -41,7 +41,10 @@ export const ComboboxWithItemsVirtual: FunctionComponent { const comboboxRef = useRef(null); const [filterTextNonDebounced, setFilterText] = useState(''); - const filterText = useDebounce(filterTextNonDebounced, 300); + const debouncedFilterText = useDebounce(filterTextNonDebounced, 300); + // Clearing applies immediately (M13): a debounced reset showed the previous search's subset for + // 300ms when the list was cleared or reopened — same rule as ComboboxWithItems + const filterText = filterTextNonDebounced ? debouncedFilterText : ''; const [visibleItems, setVisibleItems] = useState(items); const [focusedIndex, setFocusedIndex] = useState(null); // Derived during render rather than mirrored into state with effects — see ComboboxWithItems @@ -126,6 +129,11 @@ export const ComboboxWithItemsVirtual: FunctionComponent { - const firstItem = visibleItems.find((item) => !item.isGroup); + const firstItem = visibleItems.find((item) => !item.isGroup && !item.disabled); if (firstItem) { handleSelection(firstItem); } @@ -183,6 +191,14 @@ export const ComboboxWithItemsVirtual: FunctionComponent { + setFilterText(''); + setFocusedIndex(null); + comboboxRef.current?.clearInputText(); + comboboxProps.onClose?.(); + }} onInputEnter={onInputEnter} onKeyboardNavigation={handleKeyboardNavigation} > diff --git a/libs/ui/src/lib/form/combobox/RecordLookupCombobox.tsx b/libs/ui/src/lib/form/combobox/RecordLookupCombobox.tsx index 8c5286de3..190d0b9d5 100644 --- a/libs/ui/src/lib/form/combobox/RecordLookupCombobox.tsx +++ b/libs/ui/src/lib/form/combobox/RecordLookupCombobox.tsx @@ -283,7 +283,6 @@ export function RecordLookupCombobox({ errorMessageId={`${id}-error`} trailingChildren={ onChange(ev.target.value || null)} aria-describedby={comboboxProps.hasError ? `${id}-error` : undefined} + aria-invalid={comboboxProps.hasError || undefined} maxLength={18} inputMode="text" placeholder={ diff --git a/libs/ui/src/lib/form/combobox/__tests__/Combobox.spec.tsx b/libs/ui/src/lib/form/combobox/__tests__/Combobox.spec.tsx index a5c01d5f0..da4795d32 100644 --- a/libs/ui/src/lib/form/combobox/__tests__/Combobox.spec.tsx +++ b/libs/ui/src/lib/form/combobox/__tests__/Combobox.spec.tsx @@ -1,17 +1,45 @@ -import { fireEvent, render } from '@testing-library/react'; +import { axeScan } from '@jetstream/test-utils'; +import { fireEvent, render, screen } from '@testing-library/react'; +import { useState } from 'react'; +import { vi } from 'vitest'; import { Combobox } from '../Combobox'; import { ComboboxListItem } from '../ComboboxListItem'; +import { ComboboxWithItems } from '../ComboboxWithItems'; const NOOP = () => undefined; +function getInput(container: HTMLElement) { + return container.querySelector('input') as HTMLInputElement; +} + +function getListbox(container: HTMLElement) { + return container.querySelector('[role="listbox"]'); +} + function renderOpen(extra: Record) { const result = render( , ); - fireEvent.click(result.container.querySelector('input') as HTMLInputElement); - return result.container.querySelector('[role="listbox"]') as HTMLElement; + fireEvent.click(getInput(result.container)); + return getListbox(result.container) as HTMLElement; +} + +/** Owns the selection the way every app-level wrapper does, so selecting an option closes the list */ +function SelectableHarness() { + const [selectedItemId, setSelectedItemId] = useState(null); + return ( + setSelectedItemId(item.id)} + /> + ); } describe('Combobox dropdownWidth', () => { @@ -31,3 +59,98 @@ describe('Combobox dropdownWidth', () => { expect(styles.maxWidth).toBe('512px'); }); }); + +describe('Combobox Enter key', () => { + function renderClosed() { + const onInputEnter = vi.fn(); + const result = render( + + + , + ); + return { ...result, onInputEnter, input: getInput(result.container) }; + } + + function pressEnter(element: HTMLElement) { + fireEvent.keyDown(element, { key: 'Enter' }); + fireEvent.keyUp(element, { key: 'Enter' }); + } + + test('opens the closed list without picking an option, keeping focus on the input', async () => { + const { container, baseElement, input, onInputEnter } = renderClosed(); + input.focus(); + expect(getListbox(container)).toBeNull(); + + // fireEvent returns false when a handler called preventDefault — the press never reaches a wrapping form + expect(fireEvent.keyDown(input, { key: 'Enter' })).toBe(false); + fireEvent.keyUp(input, { key: 'Enter' }); + + expect(getListbox(container)).not.toBeNull(); + expect(input.getAttribute('aria-expanded')).toBe('true'); + expect(document.activeElement).toBe(input); + expect(onInputEnter).not.toHaveBeenCalled(); + + const results = await axeScan(baseElement); + expect(results.violations).toEqual([]); + }); + + test('a second Enter picks the first option', () => { + const { input, onInputEnter } = renderClosed(); + pressEnter(input); + pressEnter(input); + expect(onInputEnter).toHaveBeenCalledTimes(1); + }); + + test('leaves a modified Enter to page-level shortcuts', () => { + const { container, input, onInputEnter } = renderClosed(); + expect(fireEvent.keyDown(input, { key: 'Enter', metaKey: true })).toBe(true); + fireEvent.keyUp(input, { key: 'Enter', metaKey: true }); + expect(getListbox(container)).toBeNull(); + expect(onInputEnter).not.toHaveBeenCalled(); + }); + + test('ignores the keyup of an Enter that was pressed on an option while the list is still open', () => { + const onKeyboardNavigation = vi.fn(); + const onInputEnter = vi.fn(); + const { container } = render( + + + , + ); + const input = getInput(container); + fireEvent.click(input); + const option = screen.getByRole('option', { name: 'one' }); + option.focus(); + + // The list selects on keydown; a drill-in item keeps the list open and refocuses the input, so the + // keyup of the same press lands on the input while the list is still open + fireEvent.keyDown(option, { key: 'Enter' }); + expect(onKeyboardNavigation).toHaveBeenCalledWith('enter'); + input.focus(); + fireEvent.keyUp(input, { key: 'Enter' }); + expect(onInputEnter).not.toHaveBeenCalled(); + + // A fresh Enter pressed on the input itself still picks the first option + pressEnter(input); + expect(onInputEnter).toHaveBeenCalledTimes(1); + }); + + test('selecting an option with Enter in the list does not reopen it', () => { + const { container } = render(); + const input = getInput(container); + fireEvent.click(input); + fireEvent.keyUp(input, { key: 'ArrowDown' }); + const option = screen.getByRole('option', { name: 'one' }); + expect(document.activeElement).toBe(option); + + // Selection happens on keydown in the list and hands focus back to the input, so the same + // press's keyup lands on the closed input + fireEvent.keyDown(option, { key: 'Enter' }); + expect(getListbox(container)).toBeNull(); + expect(document.activeElement).toBe(input); + fireEvent.keyUp(input, { key: 'Enter' }); + + expect(getListbox(container)).toBeNull(); + expect(input.value).toBe('one'); + }); +}); diff --git a/libs/ui/src/lib/form/date/DateGrid.tsx b/libs/ui/src/lib/form/date/DateGrid.tsx index acac6ee2c..2e7088761 100644 --- a/libs/ui/src/lib/form/date/DateGrid.tsx +++ b/libs/ui/src/lib/form/date/DateGrid.tsx @@ -77,6 +77,8 @@ function getSelectedDates(dateGrid: DateGridDate[][]): DateGridSelection { } export interface DateGridProps { + /** id of the visible month/year heading that names the grid */ + labelledById: string; minYear: number; maxYear: number; minAvailableDate?: Date; @@ -95,6 +97,7 @@ export interface DateGridProps { } export const DateGrid: FunctionComponent = ({ + labelledById, minYear, maxYear, minAvailableDate, @@ -113,6 +116,12 @@ export const DateGrid: FunctionComponent = ({ const lastFocusedElement = useRef(null); const [dateGrid, setDateGrid] = useState([]); const elRefs = useRef[][]>([]); + // Roving tabindex: the cell that last had focus is the grid's single tab stop, so Tab out and back + // (and the popup's Tab trap, which looks for tabindex="0") returns to where the user was. Reset + // whenever the grid is rebuilt for another month; until a cell is focused, the initial-focus + // candidate (selected → today → first of month) is the tab stop. Unavailable days stay reachable + // with the arrow keys (tabindex -1) and are announced as disabled rather than being skipped. + const [focusedCell, setFocusedCell] = useState<{ week: number; day: number } | null>(null); if (elRefs.current.length !== dateGrid.length) { const refs: RefObject[][] = []; @@ -200,6 +209,7 @@ export const DateGrid: FunctionComponent = ({ currDate = addDays(currDate, 1); } setDateGrid(grid); + setFocusedCell(null); }, [selectedDate, currMonth, currYear, minAvailableDate, maxAvailableDate, minYear, maxYear]); function handleKeyDown(event: KeyboardEvent) { @@ -306,8 +316,11 @@ export const DateGrid: FunctionComponent = ({ } } + const { selectedIdx, todayIdx, firstOfMonthIdx } = getSelectedDates(dateGrid); + const rovingCell = focusedCell ?? selectedIdx ?? todayIdx ?? firstOfMonthIdx; + return ( - +
@@ -352,7 +365,8 @@ export const DateGrid: FunctionComponent = ({ ${day.readOnly && `cursor: not-allowed`} `} aria-disabled={day.readOnly} - tabIndex={day.readOnly ? undefined : day.label === 1 && day.isCurrMonth ? 0 : -1} + tabIndex={rovingCell?.week === i && rovingCell?.day === k ? 0 : -1} + onFocus={() => setFocusedCell({ week: i, day: k })} onClick={() => !day.readOnly && onSelected(day.value)} onKeyDown={handleKeyDown} onKeyUp={(event) => handleKeyUp(day, i, k, event)} diff --git a/libs/ui/src/lib/form/date/DateGridPrevNextSelector.tsx b/libs/ui/src/lib/form/date/DateGridPrevNextSelector.tsx index c73610e79..a6c3e0244 100644 --- a/libs/ui/src/lib/form/date/DateGridPrevNextSelector.tsx +++ b/libs/ui/src/lib/form/date/DateGridPrevNextSelector.tsx @@ -39,6 +39,7 @@ export const DateGridPrevNextSelector: FunctionComponent
+ )} + {/* Read-only fields have no calendar to open, so the trigger is omitted rather than rendered as + an empty, inert button that still announces haspopup/expanded */} + {!readOnly && ( + )} -
-
Format: yyyy-mm-dd
+
+ Format: yyyy-mm-dd +
{ + if (!(event.target as HTMLElement).closest('button, select, input, [tabindex]')) { + event.preventDefault(); + } + }} usePortal={usePortal} > handleToggleOpen(false)} + onClose={() => { + handleToggleOpen(false); + returnFocusToTrigger(); + }} onSelection={handleDateSelection} - onClear={handleClear} + onClear={() => { + handleClear(); + returnFocusToTrigger(); + }} /> {helpText &&
{helpText}
} diff --git a/libs/ui/src/lib/form/date/DatePickerPopup.tsx b/libs/ui/src/lib/form/date/DatePickerPopup.tsx index eccc82e4b..603bdeeb0 100644 --- a/libs/ui/src/lib/form/date/DatePickerPopup.tsx +++ b/libs/ui/src/lib/form/date/DatePickerPopup.tsx @@ -17,6 +17,8 @@ import DateGridPrevNextSelector from './DateGridPrevNextSelector'; export interface DatePickerPopupProps { ref?: React.Ref; + /** The owning DatePicker's id — scopes the heading/select ids so two pickers on a page do not collide */ + id: string; initialSelectedDate?: Date; initialVisibleDate?: Date; dropDownPosition?: PositionLeftRight; @@ -30,6 +32,7 @@ export interface DatePickerPopupProps { export const DatePickerPopup: FunctionComponent = ({ ref, + id, initialSelectedDate, initialVisibleDate = startOfMonth(new Date()), availableYears, @@ -87,10 +90,39 @@ export const DatePickerPopup: FunctionComponent = ({ setVisibleMonth(setYear(visibleMonth, currYear)); } + /** + * Dialog-wide keyboard contract (the popup renders as role="dialog"): + * - Escape closes from ANY element — owned by DatePicker's useEscapeToCloseLayer, which consumes + * the key at document capture before this handler could ever see it + * - Tab/Shift+Tab wrap within the popup (a dialog traps Tab per the APG) + */ + function handleKeyDown(event: React.KeyboardEvent) { + if (event.key !== 'Tab') { + return; + } + const focusables = Array.from( + event.currentTarget.querySelectorAll('button:not(:disabled), select:not(:disabled), [tabindex="0"]'), + ); + if (!focusables.length) { + return; + } + const first = focusables[0]; + const last = focusables[focusables.length - 1]; + if (event.shiftKey && document.activeElement === first) { + event.preventDefault(); + last.focus(); + } else if (!event.shiftKey && document.activeElement === last) { + event.preventDefault(); + first.focus(); + } + } + return ( -
+ // Delegated dialog-level handler (Escape / Tab trap) — the wrapping PopoverContainer provides role="dialog" + // eslint-disable-next-line jsx-a11y/no-static-element-interactions +
= ({ onYearChange={handleYearChange} /> = ({ /> - - diff --git a/libs/ui/src/lib/form/date/DateTimePicker.tsx b/libs/ui/src/lib/form/date/DateTimePicker.tsx index 1c1b9af24..510944989 100644 --- a/libs/ui/src/lib/form/date/DateTimePicker.tsx +++ b/libs/ui/src/lib/form/date/DateTimePicker.tsx @@ -106,7 +106,12 @@ export const Input: FunctionComponent = ({ )} {!rightAddon && clearButton && ( - )} diff --git a/libs/ui/src/lib/form/dropdown/DropDown.tsx b/libs/ui/src/lib/form/dropdown/DropDown.tsx index 31d3f3c26..53aa89f0b 100644 --- a/libs/ui/src/lib/form/dropdown/DropDown.tsx +++ b/libs/ui/src/lib/form/dropdown/DropDown.tsx @@ -5,8 +5,10 @@ import { KeyBuffer, isArrowDownKey, isArrowUpKey, - isEnterKey, - isEscapeKey, + isEndKey, + isEnterOrSpace, + isHomeKey, + isTabKey, menuItemSelectScroll, selectMenuItemFromKeyboard, } from '@jetstream/shared/ui-utils'; @@ -18,6 +20,7 @@ import React, { Fragment, FunctionComponent, KeyboardEvent, + MutableRefObject, ReactNode, RefObject, createRef, @@ -27,6 +30,7 @@ import React, { useRef, useState, } from 'react'; +import { useEscapeToCloseLayer } from '../../hooks/useEscapeToCloseLayer'; import { usePortalContext } from '../../modal/PortalContext'; import OutsideClickHandler from '../../utils/OutsideClickHandler'; import { ConditionalPortal } from '../../widgets/ConditionalPortal'; @@ -49,6 +53,8 @@ export interface DropDownProps { usePortal?: boolean; /** Portal target when `usePortal` is set; defaults to the app's portal root (document.body) */ portalRef?: HTMLElement | null; + /** The trigger button, for callers that must hand focus back to it themselves (e.g. after it was disabled) */ + triggerRef?: MutableRefObject; // eslint-disable-next-line @typescript-eslint/no-explicit-any onSelected: (id: string, metadata?: any) => void; } @@ -69,6 +75,7 @@ export const DropDown: FunctionComponent = ({ description, usePortal = false, portalRef, + triggerRef, onSelected, }) => { const keyBuffer = useRef(new KeyBuffer()); @@ -104,7 +111,23 @@ export const DropDown: FunctionComponent = ({ ); const [focusedItem, setFocusedItem] = useState(null); const [selectedItem, setSelectedItem] = useState(initialSelectedId); + // ArrowUp on the trigger opens the menu on its LAST item (APG menu button); every other way of + // opening lands on the selected item, or the first + const openOnLastItemRef = useRef(false); const ulContainerEl = useRef(null); + const triggerButtonRef = useRef(null); + + /** + * Selecting an item (or pressing Escape) unmounts the portaled menu while focus is inside it, + * which would drop focus to . Focus the trigger SYNCHRONOUSLY, before the selection + * callback runs: if the selection opens a modal, the modal then records the trigger as its + * return-focus target (the menu item it would otherwise record unmounts with the menu), and the + * modal immediately takes focus from there. Outside clicks intentionally never return focus, + * since the user is focusing something else. + */ + function focusTrigger() { + triggerButtonRef.current?.focus(); + } const elRefs = useRef[]>([]); // init array to hold element refs for each item in list @@ -136,30 +159,60 @@ export const DropDown: FunctionComponent = ({ useEffect(() => { if (isOpen && !isNumber(focusedItem)) { - if (selectedItem) { + if (openOnLastItemRef.current) { + setFocusedItem(items.length - 1); + } else if (selectedItem) { let idx = items.findIndex((item) => item.id === selectedItem); idx = idx >= 0 ? idx : 0; setFocusedItem(idx); } else { setFocusedItem(0); } + openOnLastItemRef.current = false; } else if (!isOpen) { setFocusedItem(null); } // eslint-disable-next-line react-hooks/exhaustive-deps }, [isOpen]); - function handleKeyDown(event: KeyboardEvent) { - event.preventDefault(); - event.stopPropagation(); - let newFocusedItem; + // Enter/Space on the trigger already open the menu through the native click; the arrow keys open + // it too so a keyboard user can reach the items the same way as any other menu button + function handleTriggerKeyDown(event: KeyboardEvent) { + if (isArrowDownKey(event) || isArrowUpKey(event)) { + event.preventDefault(); + if (!isOpen) { + openOnLastItemRef.current = isArrowUpKey(event); + setIsOpen(true); + } + } + } + + // Escape closes ONLY this menu (and returns focus to the trigger) — consumed at document capture + // so an ancestor modal/popover cannot also close on the same press + useEscapeToCloseLayer(isOpen, () => { + setIsOpen(false); + focusTrigger(); + }); - if (isEscapeKey(event)) { + // Menu-item keyboard handling. Escape is deliberately absent: the items only have focus while + // the menu is open, and useEscapeToCloseLayer consumes Escape at document capture for that state. + function handleKeyDown(event: KeyboardEvent) { + // Tab leaves the menu (APG menu button): focus the trigger first so the browser's sequential + // navigation continues from it (the menu is portaled), close, and let the default Tab proceed + if (isTabKey(event)) { + focusTrigger(); setIsOpen(false); return; } + event.preventDefault(); + event.stopPropagation(); + let newFocusedItem; - if (isArrowUpKey(event)) { + if (isHomeKey(event)) { + newFocusedItem = 0; + } else if (isEndKey(event)) { + newFocusedItem = items.length - 1; + } else if (isArrowUpKey(event)) { if (!isNumber(focusedItem) || focusedItem === 0) { newFocusedItem = items.length - 1; } else { @@ -171,10 +224,14 @@ export const DropDown: FunctionComponent = ({ } else { newFocusedItem = focusedItem + 1; } - } else if (isEnterKey(event) && isNumber(focusedItem)) { + } else if (isEnterOrSpace(event) && isNumber(focusedItem)) { + // Space activates like Enter (APG menu); without this it fell into the type-ahead below and + // jumped focus to the first item. The trigger only fires its click for a Space whose keydown + // it saw itself, so handing focus back here cannot reopen the menu on the keyup. const item = items[focusedItem]; if (!item.disabled) { setSelectedItem(item.id); + focusTrigger(); onSelected(item.id, item.metadata); setIsOpen(false); } @@ -197,6 +254,7 @@ export const DropDown: FunctionComponent = ({ // eslint-disable-next-line @typescript-eslint/no-explicit-any function handleSelection(event: React.MouseEvent, id: string, metadata?: any) { event.preventDefault(); + focusTrigger(); setIsOpen(false); onSelected(id, metadata); setSelectedItem(id); @@ -209,12 +267,22 @@ export const DropDown: FunctionComponent = ({ className={classNames('slds-dropdown-trigger slds-dropdown-trigger_click', className, { 'slds-is-open': isOpen })} > diff --git a/libs/ui/src/lib/form/file-selector/GoogleFileSelectorExternalButton.tsx b/libs/ui/src/lib/form/file-selector/GoogleFileSelectorExternalButton.tsx index e92440e5f..1165d2487 100644 --- a/libs/ui/src/lib/form/file-selector/GoogleFileSelectorExternalButton.tsx +++ b/libs/ui/src/lib/form/file-selector/GoogleFileSelectorExternalButton.tsx @@ -14,6 +14,7 @@ import HelpText from '../../widgets/HelpText'; import Icon from '../../widgets/Icon'; import Spinner from '../../widgets/Spinner'; import Tooltip from '../../widgets/Tooltip'; +import { ariaDisabledButtonProps } from '../button/aria-disabled-button.utils'; import { useFilename } from './useFilename'; const GOOGLE_APPS_MIME_PREFIX = 'application/vnd.google-apps.'; @@ -190,6 +191,7 @@ export const GoogleFileSelectorExternalButton: FunctionComponent - +
diff --git a/libs/ui/src/lib/form/file-selector/ImageFile.tsx b/libs/ui/src/lib/form/file-selector/ImageFile.tsx index b9f06bd91..8b4362f03 100644 --- a/libs/ui/src/lib/form/file-selector/ImageFile.tsx +++ b/libs/ui/src/lib/form/file-selector/ImageFile.tsx @@ -81,6 +81,7 @@ export const ImageFile: FunctionComponent = ({ {onDelete && ( )} diff --git a/libs/ui/src/lib/form/formGroupDropDown/FormGroupDropdown.tsx b/libs/ui/src/lib/form/formGroupDropDown/FormGroupDropdown.tsx index 06c72ae0d..8f8ecc6ea 100644 --- a/libs/ui/src/lib/form/formGroupDropDown/FormGroupDropdown.tsx +++ b/libs/ui/src/lib/form/formGroupDropDown/FormGroupDropdown.tsx @@ -16,12 +16,12 @@ import classNames from 'classnames'; import isNumber from 'lodash/isNumber'; import uniqueId from 'lodash/uniqueId'; import { createRef, FunctionComponent, KeyboardEvent, RefObject, useEffect, useRef, useState } from 'react'; +import { useEscapeToCloseLayer } from '../../hooks/useEscapeToCloseLayer'; import OutsideClickHandler from '../../utils/OutsideClickHandler'; import Icon from '../../widgets/Icon'; export interface FormGroupDropdownProps { className?: string; - comboboxId: string; label: string; initialSelectedItemId?: string; items: FormGroupDropdownItem[]; @@ -36,7 +36,6 @@ export interface FormGroupDropdownProps { export const FormGroupDropdown: FunctionComponent = ({ className, - comboboxId, label, initialSelectedItemId, items, @@ -89,27 +88,50 @@ export const FormGroupDropdown: FunctionComponent = ({ function selectItem(item: FormGroupDropdownItem) { setSelectedItem(item); setIsOpen(false); + // The focused option unmounts with the list, which would drop focus to — return it to the + // trigger (as Escape does) BEFORE notifying, so anything the selection opens records the trigger + // as its return-focus target + if (inputRef.current && typeof inputRef.current.focus === 'function') { + inputRef.current.focus(); + } if (onSelected) { onSelected(item); } } + // Escape closes ONLY this menu (and returns focus to the trigger) — consumed at document capture + // so an ancestor modal/popover cannot also close on the same press + useEscapeToCloseLayer(isOpen, () => { + setIsOpen(false); + if (inputRef.current && typeof inputRef.current.focus === 'function') { + inputRef.current.focus(); + } + }); + function handleKeyDown(event: KeyboardEvent) { try { if (isTabKey(event)) { + // A focused option unmounts with the list — hand focus to the trigger WITHOUT preventDefault so + // the browser's sequential navigation continues from there in the same press + if (event.target !== inputRef.current && inputRef.current && typeof inputRef.current.focus === 'function') { + inputRef.current.focus(); + } setIsOpen(false); return; } + // Modified keys are browser/app shortcuts (reload, page-level Cmd+Enter), not type-ahead + if (event.metaKey || event.ctrlKey || event.altKey) { + return; + } event.preventDefault(); event.stopPropagation(); let newFocusedItem; + // While open, Escape never reaches here (useEscapeToCloseLayer consumes it at document + // capture); this guard covers the CLOSED state, keeping Escape out of the type-ahead buffer + // in the fallback branch below if (isEscapeKey(event)) { - setIsOpen(false); - if (inputRef.current && typeof inputRef.current.focus === 'function') { - inputRef.current.focus(); - } return; } @@ -131,7 +153,7 @@ export const FormGroupDropdown: FunctionComponent = ({ } else { newFocusedItem = focusedItem + 1; } - } else if (isEnterKey(event) && isNumber(focusedItem)) { + } else if (isOpen && (isEnterKey(event) || isSpaceKey(event)) && isNumber(focusedItem)) { const item = items[focusedItem]; selectItem(item); } else { @@ -166,7 +188,8 @@ export const FormGroupDropdown: FunctionComponent = ({
setIsOpen(true)} >
= ({ } `} id={`${inputId}-selected-value`} - aria-controls={id} + aria-controls={isOpen ? id : undefined} aria-expanded={isOpen} aria-haspopup="listbox" aria-labelledby={`${inputId}-label`} @@ -216,7 +239,7 @@ export const FormGroupDropdown: FunctionComponent = ({ 'slds-has-focus': isOpen, })} id={`${inputId}-selected-value`} - aria-controls={id} + aria-controls={isOpen ? id : undefined} aria-expanded={isOpen} aria-haspopup="listbox" aria-labelledby={`${inputId}-label`} @@ -238,6 +261,7 @@ export const FormGroupDropdown: FunctionComponent = ({ id={id} className={`slds-dropdown slds-dropdown_length-7 slds-dropdown_x-small slds-dropdown_${variant === 'end' ? 'right' : 'left'}`} role="listbox" + aria-labelledby={`${inputId}-label`} >