diff --git a/src/components/EditableCell/EditableCell.tsx b/src/components/EditableCell/EditableCell.tsx index d76913848ded..a49bc10676a3 100644 --- a/src/components/EditableCell/EditableCell.tsx +++ b/src/components/EditableCell/EditableCell.tsx @@ -48,6 +48,13 @@ type EditableCellProps = { /** Which side should show the edit icon in display mode */ editIconPosition?: EditIconPosition; + + /** + * Whether display mode wraps children in the padded, bordered `editableCell` box. + * A composite cell whose display content is taller than one line owns its own layout, + * and the box would add height to it. Edit mode still draws the box around its input. + */ + shouldRenderDisplayChrome?: boolean; }; /** @@ -61,7 +68,17 @@ type EditableCellProps = { * 4. canEdit=false → styled container View, no pressable (transient: loading / no permission) * 5. default → passive cell body with hover/focus edit icon button */ -function EditableCell({children, editContent, popoverContent, isEditing, canEdit, onStartEditing, anchorRef, editIconPosition = 'right'}: EditableCellProps) { +function EditableCell({ + children, + editContent, + popoverContent, + isEditing, + canEdit, + onStartEditing, + anchorRef, + editIconPosition = 'right', + shouldRenderDisplayChrome = true, +}: EditableCellProps) { const styles = useThemeStyles(); const theme = useTheme(); const {translate} = useLocalize(); @@ -133,7 +150,7 @@ function EditableCell({children, editContent, popoverContent, isEditing, canEdit // Tooltip / Hoverable / BoundsObserver subtree during the initial skeleton render, // unblocking the main thread so the network response can be processed sooner. if (!canEdit || !isInteractive) { - return {children}; + return shouldRenderDisplayChrome ? {children} : children; } return ( @@ -143,7 +160,7 @@ function EditableCell({children, editContent, popoverContent, isEditing, canEdit return ( - {children} + {shouldRenderDisplayChrome ? {children} : children} ; /** @@ -32,6 +33,14 @@ type InlineTextEditCellProps = { * Defaults to sanitizeName so a pasted non-breaking space is treated as unchanged when the action would no-op. */ sanitize?: (value: string) => string; + + /** + * Display content replacing the default single-line text, for a cell that shows more than the + * value being edited, such as a cardholder block whose subtitle is the card name. The box that + * normally wraps display content is dropped so that layout keeps its own line heights. Editing + * still swaps the whole cell for the input. + */ + children?: ReactNode; } & EditableProps; /** @@ -43,7 +52,7 @@ type InlineTextEditCellProps = { * original value. On narrow layouts EditableCell renders the display text only, so tables keep * their existing tap-to-navigate behavior there. */ -function InlineTextEditCell({value, accessibilityLabel, displayTextStyle, canEdit, onSave, sanitize = StringUtils.sanitizeName}: InlineTextEditCellProps) { +function InlineTextEditCell({value, accessibilityLabel, displayTextStyle, canEdit, onSave, sanitize = StringUtils.sanitizeName, children}: InlineTextEditCellProps) { const styles = useThemeStyles(); const inputRef = useRef(null); @@ -68,6 +77,7 @@ function InlineTextEditCell({value, accessibilityLabel, displayTextStyle, canEdi canEdit={canEdit} isEditing={isEditing} onStartEditing={startEditing} + shouldRenderDisplayChrome={!children} editContent={ } > - + {children ?? ( + + )} ); } diff --git a/src/components/Tables/WorkspaceExpensifyCardsTable/WorkspaceExpensifyCardsTableRow.tsx b/src/components/Tables/WorkspaceExpensifyCardsTable/WorkspaceExpensifyCardsTableRow.tsx index 85a965e48266..1dc256d2ad6c 100644 --- a/src/components/Tables/WorkspaceExpensifyCardsTable/WorkspaceExpensifyCardsTableRow.tsx +++ b/src/components/Tables/WorkspaceExpensifyCardsTable/WorkspaceExpensifyCardsTableRow.tsx @@ -1,4 +1,5 @@ import UserAvatar from '@components/Avatar/UserAvatar'; +import {InlineTextEditCell} from '@components/EditableCell'; import Icon from '@components/Icon'; import {useSession} from '@components/OnyxListItemProvider'; import Table from '@components/Table'; @@ -126,33 +127,44 @@ export default function WorkspaceExpensifyCardsTableRow({item, rowIndex, shouldU style={[styles.flex1, styles.mnw0, styles.flexRow, styles.gap3, styles.alignItemsCenter]} {...getCellAccessibilityProps(isTableSemanticsEnabled)} > + {/* The avatar sits outside the edit cell so it stays on screen while the card name is edited. */} - - - {shouldUseNarrowTableLayout ? ( - - ) : ( - - )} + + + + + {shouldUseNarrowTableLayout ? ( + + ) : ( + + )} + + diff --git a/src/components/Tables/WorkspaceExpensifyCardsTable/index.tsx b/src/components/Tables/WorkspaceExpensifyCardsTable/index.tsx index ad575adf2683..c41a548a15cd 100644 --- a/src/components/Tables/WorkspaceExpensifyCardsTable/index.tsx +++ b/src/components/Tables/WorkspaceExpensifyCardsTable/index.tsx @@ -60,9 +60,11 @@ type WorkspaceExpensifyCardTableRowData = TableData & { frozenDate?: string; errors?: OnyxCommon.Errors; pendingAction?: OnyxCommon.PendingAction; + canEditName?: boolean; canEditLimitType?: boolean; canEditLimit?: boolean; action: () => void; + onRenameName?: (newName: string) => void; onChangeLimitType?: (limitType: CardLimitType) => void; /** Return false, or a promise of false, to keep the limit editor open until a confirm modal resolves. */ onChangeLimit?: (newLimit: string) => InlineEditSaveResult; diff --git a/src/libs/actions/Policy/InlineEdit.ts b/src/libs/actions/Policy/InlineEdit.ts index 4de4633596ff..cd00eef61ca3 100644 --- a/src/libs/actions/Policy/InlineEdit.ts +++ b/src/libs/actions/Policy/InlineEdit.ts @@ -23,7 +23,7 @@ import {getCleanedTagName, getTagList} from '@libs/PolicyUtils'; import StringUtils from '@libs/StringUtils'; import {getTagNameError, getTagNameErrorMessage} from '@libs/TagUtils'; -import {updateExpensifyCardLimit, updateExpensifyCardLimitType} from '@userActions/Card'; +import {updateExpensifyCardLimit, updateExpensifyCardLimitType, updateExpensifyCardTitle} from '@userActions/Card'; import {updateCompanyCardName} from '@userActions/CompanyCards'; import CONST from '@src/CONST'; @@ -103,6 +103,21 @@ function renameCompanyCardInline( updateCompanyCardName(domainOrWorkspaceAccountID, cardID, sanitized, bankName, storedName); } +function renameExpensifyCardInline(translate: LocalizedTranslate, workspaceAccountID: number, cardID: number, newName: string, currentName: string): string | undefined { + const sanitized = StringUtils.sanitizeName(newName); + + if (sanitized === currentName) { + return; + } + + const cardNameError = getCardNameError(newName); + if (cardNameError) { + return getCardNameErrorMessage(translate, cardNameError, newName); + } + + updateExpensifyCardTitle(workspaceAccountID, cardID, sanitized, currentName); +} + function renameDistanceRateInline(translate: LocalizedTranslate, policyID: string, customUnit: CustomUnit, rate: Rate, newName: string): string | undefined { const sanitized = StringUtils.sanitizeName(newName); const currentName = rate.name ?? ''; @@ -300,6 +315,7 @@ export { renameCategoryInline, renameTagInline, renameCompanyCardInline, + renameExpensifyCardInline, renameDistanceRateInline, updateDistanceRateValueInline, updateMemberRoleInline, diff --git a/src/pages/workspace/expensifyCard/WorkspaceExpensifyCardListPage.tsx b/src/pages/workspace/expensifyCard/WorkspaceExpensifyCardListPage.tsx index 08b49cfb5b11..9834037b5ecb 100644 --- a/src/pages/workspace/expensifyCard/WorkspaceExpensifyCardListPage.tsx +++ b/src/pages/workspace/expensifyCard/WorkspaceExpensifyCardListPage.tsx @@ -40,6 +40,7 @@ import { canUpdateExpensifyCardLimitTypeInline, getExpensifyCardLimitInlineEditError, getExpensifyCardLimitInlineUpdate, + renameExpensifyCardInline, updateExpensifyCardLimitInline, updateExpensifyCardLimitTypeInline, } from '@libs/actions/Policy/InlineEdit'; @@ -276,9 +277,11 @@ function WorkspaceExpensifyCardListPage({route, cardsList, fundID}: WorkspaceExp frozenDate: card.nameValuePairs?.frozen?.date, errors: card.errors, pendingAction: card.pendingAction, + canEditName: canEditCard, canEditLimitType: canEditCard, canEditLimit: canEditCard, action: () => Navigation.navigate(createDynamicRoute(DYNAMIC_ROUTES.WORKSPACE_EXPENSIFY_CARD_DETAILS.getRoute(card.cardID.toString()))), + onRenameName: (newName: string) => showInlineEditError(renameExpensifyCardInline(translate, fundID, card.cardID, newName, card.nameValuePairs?.cardTitle ?? '')), onChangeLimitType: (newLimitType: CardLimitType) => changeCardLimitType(card, newLimitType), onChangeLimit: (newLimit: string) => changeCardLimit(card, newLimit), onClose: () => clearDeletePaymentMethodError(`${ONYXKEYS.COLLECTION.WORKSPACE_CARDS_LIST}${fundID}_${CONST.EXPENSIFY_CARD.BANK}`, card.cardID), diff --git a/tests/unit/inlineEditing/PolicyInlineEdit.test.ts b/tests/unit/inlineEditing/PolicyInlineEdit.test.ts index c854796e359a..c1ef8a68b1b8 100644 --- a/tests/unit/inlineEditing/PolicyInlineEdit.test.ts +++ b/tests/unit/inlineEditing/PolicyInlineEdit.test.ts @@ -5,6 +5,7 @@ import { getExpensifyCardLimitInlineEditError, getExpensifyCardLimitInlineUpdate, renameCategoryInline, + renameExpensifyCardInline, updateExpensifyCardLimitInline, updateExpensifyCardLimitTypeInline, updateMemberRoleInline, @@ -177,6 +178,67 @@ describe('PolicyInlineEdit', () => { }); }); + describe('renameExpensifyCardInline', () => { + it('does not persist a name the user never changed', () => { + // Given a card named "Travel" + // When the editor closes on blur with that same name + const rejection = renameExpensifyCardInline(translate, 1, 10, 'Travel', 'Travel'); + + // Then nothing is written and no toast is raised, so opening the editor and clicking away costs no request + expect(rejection).toBeUndefined(); + expect(mockWrite).not.toHaveBeenCalled(); + }); + + it('returns the required-field error for a name that sanitizes to nothing', () => { + // Given a card named "Travel" + // When the editor commits whitespace only + const rejection = renameExpensifyCardInline(translate, 1, 10, ' ', 'Travel'); + + // Then the page can toast the same required message as the card name form, and nothing is written + expect(rejection).toBe('common.error.fieldRequired'); + expect(mockWrite).not.toHaveBeenCalled(); + }); + + it('returns the invalid-character error for a name the card form rejects', () => { + // Given a card named "Travel" + // When the editor commits a name that contains an HTML-like tag + const rejection = renameExpensifyCardInline(translate, 1, 10, '', 'Travel'); + + // Then the page can toast the same invalid-character message as the card name form, and nothing is written + expect(rejection).toBe('common.error.invalidCharacter'); + expect(mockWrite).not.toHaveBeenCalled(); + }); + + it('returns the length error for a name past the card form limit', () => { + // Given a card named "Travel" and a name one byte past the shared length limit + const tooLongName = 'a'.repeat(CONST.STANDARD_LENGTH_LIMIT + 1); + + // When the editor commits that name + const rejection = renameExpensifyCardInline(translate, 1, 10, tooLongName, 'Travel'); + + // Then the page can toast the same character-limit message as the card name form, and nothing is written + expect(rejection).toBe(`common.error.characterLimitExceedCounter|${CONST.STANDARD_LENGTH_LIMIT + 1}|${CONST.STANDARD_LENGTH_LIMIT}`); + expect(mockWrite).not.toHaveBeenCalled(); + }); + + it('persists the sanitized name', () => { + // Given a card named "Travel" + // When the editor commits a new name padded with whitespace + const rejection = renameExpensifyCardInline(translate, 1, 10, ' Travel card ', 'Travel'); + + // Then the trimmed name reaches the backend and no toast is raised, so a padded paste cannot be stored verbatim + expect(rejection).toBeUndefined(); + expect(mockWrite).toHaveBeenCalledWith( + WRITE_COMMANDS.UPDATE_EXPENSIFY_CARD_TITLE, + expect.objectContaining({ + cardID: 10, + cardTitle: 'Travel card', + }), + expect.anything(), + ); + }); + }); + describe('getExpensifyCardLimitInlineUpdate', () => { it('returns the field error for an invalid limit so the page can toast it', () => { // Given a limit the card form rejects because it is not a whole dollar amount