From 2243da646f7b0770e98e3c3088db577541d9e9a4 Mon Sep 17 00:00:00 2001 From: Jesper Hodge Date: Tue, 14 Apr 2026 12:23:48 -0400 Subject: [PATCH 1/7] fix: taxonomy alignment and error message --- src/messages.ts | 4 + src/taxonomy/data/apiHooks.ts | 25 +-- src/taxonomy/tag-list/UsageCountDisplay.tsx | 25 +++ src/taxonomy/tag-list/hooks.ts | 32 +++- src/taxonomy/tag-list/tagColumns.tsx | 26 +-- src/taxonomy/tag-list/types.ts | 9 ++ src/taxonomy/tree-table/CreateRow.test.tsx | 3 +- src/taxonomy/tree-table/CreateRow.tsx | 167 +------------------- src/taxonomy/tree-table/DraftRow.tsx | 126 +++++++++++++++ src/taxonomy/tree-table/EditRow.tsx | 49 ++++++ src/taxonomy/tree-table/NestedRows.tsx | 8 +- src/taxonomy/tree-table/SaveErrorAlert.tsx | 42 +++++ src/taxonomy/tree-table/TableBody.tsx | 10 +- src/taxonomy/tree-table/TableView.tsx | 16 +- 14 files changed, 311 insertions(+), 231 deletions(-) create mode 100644 src/taxonomy/tag-list/UsageCountDisplay.tsx create mode 100644 src/taxonomy/tag-list/types.ts create mode 100644 src/taxonomy/tree-table/DraftRow.tsx create mode 100644 src/taxonomy/tree-table/EditRow.tsx create mode 100644 src/taxonomy/tree-table/SaveErrorAlert.tsx diff --git a/src/messages.ts b/src/messages.ts index 1620914b5b..06f7ffd519 100644 --- a/src/messages.ts +++ b/src/messages.ts @@ -10,6 +10,10 @@ const messages = defineMessages({ id: 'authoring.alert.support.text', defaultMessage: 'Support Page', }, + unknownError: { + id: 'authoring.alert.error.unknown', + defaultMessage: 'Unknown error', + }, }); export default messages; diff --git a/src/taxonomy/data/apiHooks.ts b/src/taxonomy/data/apiHooks.ts index 479cc2c4db..96c8020090 100644 --- a/src/taxonomy/data/apiHooks.ts +++ b/src/taxonomy/data/apiHooks.ts @@ -214,18 +214,13 @@ export const useSubTags = (taxonomyId: number, parentTagValue: string) => useQue export const useCreateTag = (taxonomyId: number) => { const queryClient = useQueryClient(); - const intl = useIntl(); return useMutation({ mutationFn: async ({ value, parentTagValue }: { value: string, parentTagValue?: string }) => { - try { - await getAuthenticatedHttpClient().post( - apiUrls.createTag(taxonomyId), - { tag: value, parent_tag_value: parentTagValue }, - ); - } catch (err) { - throw new Error(getApiErrorMessage(err, intl)); - } + await getAuthenticatedHttpClient().post( + apiUrls.createTag(taxonomyId), + { tag: value, parent_tag_value: parentTagValue }, + ); }, onSuccess: () => { queryClient.invalidateQueries({ @@ -246,14 +241,10 @@ export const useUpdateTag = (taxonomyId: number) => { return useMutation({ mutationFn: async ({ value, originalValue }: { value: string, originalValue: string }) => { - try { - await getAuthenticatedHttpClient().patch( - apiUrls.updateTag(taxonomyId), - { tag: originalValue, updated_tag_value: value }, - ); - } catch (err) { - throw new Error(getApiErrorMessage(err)); - } + await getAuthenticatedHttpClient().patch( + apiUrls.updateTag(taxonomyId), + { tag: originalValue, updated_tag_value: value }, + ); }, onSuccess: () => { queryClient.invalidateQueries({ diff --git a/src/taxonomy/tag-list/UsageCountDisplay.tsx b/src/taxonomy/tag-list/UsageCountDisplay.tsx new file mode 100644 index 0000000000..97a95ad026 --- /dev/null +++ b/src/taxonomy/tag-list/UsageCountDisplay.tsx @@ -0,0 +1,25 @@ +import { + Bubble, +} from '@openedx/paragon'; +import type { Row } from '@tanstack/react-table'; +import type { + TreeRowData, +} from '@src/taxonomy/tree-table/types'; +import { TagListRowData } from './types'; + +const asTagListRowData = (row: Row): TagListRowData => ( + row.original as unknown as TagListRowData +); + +const UsageCountDisplay = ({ row }: { row: Row }) => { + const count = asTagListRowData(row).usageCount ?? 0; + return ( + count > 0 && ( + + {count} + + ) + ); +}; + +export default UsageCountDisplay; diff --git a/src/taxonomy/tag-list/hooks.ts b/src/taxonomy/tag-list/hooks.ts index 5484d88b0e..ea2d7b6d0f 100644 --- a/src/taxonomy/tag-list/hooks.ts +++ b/src/taxonomy/tag-list/hooks.ts @@ -163,6 +163,26 @@ const useEditActions = ({ return true; }; + const getErrorMessage = (error: any): string => { + let errorMessage: string = ''; + if (error.name === 'AxiosError') { + const responseData = error.response?.data; + const tagError = Object.entries(responseData)?.find((errItem: [string, unknown]) => ( + ['tag', 'value', 'updated_tag_value'].includes(errItem[0].toLowerCase()) + )); + + const errorMessages = tagError ? tagError[1] : ( + (error as Error).message || intl.formatMessage(globalMessages.unknownError) + ); + errorMessage = Array.isArray(errorMessages) ? errorMessages.join('; ') : String(errorMessages); + } else { + errorMessage = (error as Error).message || intl.formatMessage(globalMessages.unknownError); + } + + errorMessage = errorMessage.replace(/\.$/, ''); // Remove trailing period for better message formatting + return errorMessage; + }; + const handleCreateTag = async (value: string, parentTagValue?: string) => { const trimmed = value.trim(); @@ -182,9 +202,9 @@ const useEditActions = ({ setIsCreatingTopTag(false); setCreatingParentId(null); } catch (error) { - const message = intl.formatMessage(messages.tagCreationErrorMessage, { errorMessage: (error as Error)?.message }); - setDraftError((error as Error)?.message || intl.formatMessage(messages.tagCreationErrorMessage, { errorMessage: '' })); - setToast({ show: true, message }); + const errorMessage = getErrorMessage(error); + setDraftError(errorMessage); + setToast({ show: true, message: intl.formatMessage(messages.tagCreationErrorMessage, { errorMessage }) }); } }; @@ -211,9 +231,9 @@ const useEditActions = ({ message: intl.formatMessage(messages.tagUpdateSuccessMessage, { name: trimmed }), }); } catch (error) { - const message = intl.formatMessage(messages.tagUpdateErrorMessage, { errorMessage: (error as Error)?.message }); - setDraftError((error as Error)?.message || intl.formatMessage(messages.tagUpdateErrorMessage, { errorMessage: '' })); - setToast({ show: true, message }); + const errorMessage = getErrorMessage(error); + setDraftError(errorMessage); + setToast({ show: true, message: intl.formatMessage(messages.tagUpdateErrorMessage, { errorMessage }) }); } }; diff --git a/src/taxonomy/tag-list/tagColumns.tsx b/src/taxonomy/tag-list/tagColumns.tsx index 77ebc2341d..363a1a9a12 100644 --- a/src/taxonomy/tag-list/tagColumns.tsx +++ b/src/taxonomy/tag-list/tagColumns.tsx @@ -1,5 +1,4 @@ import { - Bubble, Icon, IconButton, IconButtonWithTooltip, @@ -12,24 +11,18 @@ import { import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n'; import type { Row } from '@tanstack/react-table'; -import messages from './messages'; import type { RowId, TreeColumnDef, TreeRowData, -} from '../tree-table/types'; +} from '@src/taxonomy/tree-table/types'; +import { TagListRowData } from './types'; +import messages from './messages'; import OptionalExpandLink from './OptionalExpandLink'; +import UsageCountDisplay from './UsageCountDisplay'; const EDITABLE_COLUMNS = ['value']; -interface TagListRowData extends TreeRowData { - depth: number; - childCount: number; - usageCount?: number; - isNew?: boolean; - isEditing?: boolean; -} - const asTagListRowData = (row: Row): TagListRowData => ( row.original as unknown as TagListRowData ); @@ -49,17 +42,6 @@ interface GetColumnsArgs { maxDepth: number; } -const UsageCountDisplay = ({ row }: { row: Row }) => { - const count = asTagListRowData(row).usageCount ?? 0; - return ( - count > 0 && ( - - {count} - - ) - ); -}; - interface ActionsHeaderProps { onStartDraft: () => void; setDraftError: (error: string) => void; diff --git a/src/taxonomy/tag-list/types.ts b/src/taxonomy/tag-list/types.ts new file mode 100644 index 0000000000..845cee6956 --- /dev/null +++ b/src/taxonomy/tag-list/types.ts @@ -0,0 +1,9 @@ +import { TreeRowData } from '@src/taxonomy/tree-table/types'; + +export interface TagListRowData extends TreeRowData { + depth: number; + childCount: number; + usageCount?: number; + isNew?: boolean; + isEditing?: boolean; +} diff --git a/src/taxonomy/tree-table/CreateRow.test.tsx b/src/taxonomy/tree-table/CreateRow.test.tsx index dedfb12490..7510f3f3bb 100644 --- a/src/taxonomy/tree-table/CreateRow.test.tsx +++ b/src/taxonomy/tree-table/CreateRow.test.tsx @@ -2,7 +2,7 @@ import React from 'react'; import { IntlProvider } from '@edx/frontend-platform/i18n'; import { fireEvent, render, screen } from '@testing-library/react'; -import { CreateRow } from './CreateRow'; +import CreateRow from './CreateRow'; const wrapper = ({ children }: { children: React.ReactNode }) => ( {children} @@ -15,7 +15,6 @@ const baseProps = () => ({ setIsCreatingTopRow: jest.fn(), exitDraftWithoutSave: jest.fn(), createRowMutation: { isPending: false }, - columns: [{ id: 'value' }], validate: jest.fn((value: string) => value.trim().length > 0), }); diff --git a/src/taxonomy/tree-table/CreateRow.tsx b/src/taxonomy/tree-table/CreateRow.tsx index 4091db0a56..f130560b02 100644 --- a/src/taxonomy/tree-table/CreateRow.tsx +++ b/src/taxonomy/tree-table/CreateRow.tsx @@ -1,24 +1,6 @@ -import React, { useState } from 'react'; -import { Button, Spinner } from '@openedx/paragon'; -import { useIntl } from '@edx/frontend-platform/i18n'; - -import { EditableCell } from './EditableCell'; -import type { CreateRowMutationState, TreeColumnDef } from './types'; -import messages from './messages'; - -interface DraftRowProps { - draftError: string; - initialValue?: string; - onSave: (value: string) => void; - onCancel: () => void; - mutationState: CreateRowMutationState; - columns: TreeColumnDef[]; - indent?: number; - validate: (value: string, mode?: 'soft' | 'hard') => boolean; - requireValueChangeToEnableSave?: boolean; - rowTestId?: string; - rowId?: string; -} +import React from 'react'; +import type { CreateRowMutationState } from './types'; +import DraftRow from './DraftRow'; interface CreateRowProps { draftError: string; @@ -27,118 +9,10 @@ interface CreateRowProps { setIsCreatingTopRow: (isCreating: boolean) => void; exitDraftWithoutSave: () => void; createRowMutation: CreateRowMutationState; - columns: TreeColumnDef[]; indent?: number; validate: (value: string, mode?: 'soft' | 'hard') => boolean; } -interface EditRowProps { - draftError: string; - setDraftError: (error: string) => void; - initialValue: string; - handleUpdateRow: (value: string) => void; - cancelEditRow: () => void; - updateRowMutation: CreateRowMutationState; - columns: TreeColumnDef[]; - indent?: number; - validate: (value: string, mode?: 'soft' | 'hard') => boolean; -} - -const DraftRow: React.FC = ({ - draftError, - initialValue = '', - onSave, - onCancel, - mutationState, - columns, - indent = 0, - validate, - requireValueChangeToEnableSave = false, - rowTestId, - rowId, -}) => { - const [rowValue, setRowValue] = useState(initialValue); - const [saveDisabled, setSaveDisabled] = useState(true); - const intl = useIntl(); - - const updateSaveDisabled = (value: string) => { - const trimmedValue = value.trim(); - const isValid = validate(value, 'soft'); - const isUnchanged = requireValueChangeToEnableSave && trimmedValue === initialValue.trim(); - setSaveDisabled(!isValid || !trimmedValue || isUnchanged || mutationState.isPending || false); - }; - - const handleValueChange = (e: React.ChangeEvent) => { - const { value } = e.target; - setRowValue(value); - updateSaveDisabled(value); - }; - - const handleSave = () => { - onSave(rowValue.trim()); - }; - - const handleValueCellKeyPress = (e: React.KeyboardEvent) => { - if (e.key === 'Enter' && !saveDisabled && !draftError) { - e.preventDefault(); - handleSave(); - return; - } - - if (e.key === 'Escape') { - e.preventDefault(); - onCancel(); - } - }; - - return ( - - -
- -
- - - - - - - - - - {mutationState.isPending && ( - - )} - - - - ); -}; - const CreateRow: React.FC = ({ draftError, setDraftError, @@ -146,7 +20,6 @@ const CreateRow: React.FC = ({ setIsCreatingTopRow, exitDraftWithoutSave, createRowMutation, - columns, indent = 0, validate, }) => { @@ -162,7 +35,6 @@ const CreateRow: React.FC = ({ onSave={handleCreateRow} onCancel={handleCancel} mutationState={createRowMutation} - columns={columns} indent={indent} validate={validate} rowId="creating-top-row" @@ -171,35 +43,4 @@ const CreateRow: React.FC = ({ ); }; -const EditRow: React.FC = ({ - draftError, - setDraftError, - initialValue, - handleUpdateRow, - cancelEditRow, - updateRowMutation, - columns, - indent = 0, - validate, -}) => { - const handleCancel = () => { - setDraftError(''); - cancelEditRow(); - }; - - return ( - - ); -}; - -export { CreateRow, EditRow }; +export default CreateRow; diff --git a/src/taxonomy/tree-table/DraftRow.tsx b/src/taxonomy/tree-table/DraftRow.tsx new file mode 100644 index 0000000000..f15bc4f4a5 --- /dev/null +++ b/src/taxonomy/tree-table/DraftRow.tsx @@ -0,0 +1,126 @@ +import React, { useState } from 'react'; +import { useIntl } from '@edx/frontend-platform/i18n'; +import { Button, Spinner } from '@openedx/paragon'; +import { Row } from '@tanstack/react-table'; + +import UsageCountDisplay from '@src/taxonomy/tag-list/UsageCountDisplay'; +import { EditableCell } from './EditableCell'; +import type { CreateRowMutationState, TreeRowData } from './types'; +import messages from './messages'; + +interface DraftRowProps { + draftError: string; + initialValue?: string; + onSave: (value: string) => void; + onCancel: () => void; + mutationState: CreateRowMutationState; + indent?: number; + validate: (value: string, mode?: 'soft' | 'hard') => boolean; + requireValueChangeToEnableSave?: boolean; + rowTestId?: string; + rowId?: string; + row?: Row; +} + +const DraftRow: React.FC = ({ + draftError, + initialValue = '', + onSave, + onCancel, + mutationState, + indent = 0, + validate, + requireValueChangeToEnableSave = false, + rowTestId, + rowId, + row, + +}) => { + const [rowValue, setRowValue] = useState(initialValue); + const [saveDisabled, setSaveDisabled] = useState(true); + const intl = useIntl(); + + const updateSaveDisabled = (value: string) => { + const trimmedValue = value.trim(); + const isValid = validate(value, 'soft'); + const isUnchanged = requireValueChangeToEnableSave && trimmedValue === initialValue.trim(); + setSaveDisabled(!isValid || !trimmedValue || isUnchanged || mutationState.isPending || false); + }; + + const handleValueChange = (e: React.ChangeEvent) => { + const { value } = e.target; + setRowValue(value); + updateSaveDisabled(value); + }; + + const handleSave = () => { + onSave(rowValue.trim()); + }; + + const handleValueCellKeyPress = (e: React.KeyboardEvent) => { + if (e.key === 'Enter' && !saveDisabled && !draftError) { + e.preventDefault(); + handleSave(); + return; + } + + if (e.key === 'Escape') { + e.preventDefault(); + onCancel(); + } + }; + + const indentClass = indent > 0 ? `tree-table-indent tree-table-indent-${indent}` : ''; + + return ( + + +
+ +
+ + + {row ? : null} + + + + + + + + + + {mutationState.isPending && ( + + )} + + + + ); +}; + +export default DraftRow; diff --git a/src/taxonomy/tree-table/EditRow.tsx b/src/taxonomy/tree-table/EditRow.tsx new file mode 100644 index 0000000000..12a83fa055 --- /dev/null +++ b/src/taxonomy/tree-table/EditRow.tsx @@ -0,0 +1,49 @@ +import React from 'react'; +import { Row } from '@tanstack/react-table'; +import type { CreateRowMutationState, TreeRowData } from './types'; +import DraftRow from './DraftRow'; + +interface EditRowProps { + draftError: string; + setDraftError: (error: string) => void; + initialValue: string; + handleUpdateRow: (value: string) => void; + cancelEditRow: () => void; + updateRowMutation: CreateRowMutationState; + indent?: number; + validate: (value: string, mode?: 'soft' | 'hard') => boolean; + row: Row; +} + +const EditRow: React.FC = ({ + draftError, + setDraftError, + initialValue, + handleUpdateRow, + cancelEditRow, + updateRowMutation, + indent = 0, + validate, + row, +}) => { + const handleCancel = () => { + setDraftError(''); + cancelEditRow(); + }; + + return ( + + ); +}; + +export default EditRow; diff --git a/src/taxonomy/tree-table/NestedRows.tsx b/src/taxonomy/tree-table/NestedRows.tsx index 216bcfbf18..a0c42b5db8 100644 --- a/src/taxonomy/tree-table/NestedRows.tsx +++ b/src/taxonomy/tree-table/NestedRows.tsx @@ -7,7 +7,8 @@ import type { TreeColumnDef, CreateRowMutationState, } from './types'; -import { CreateRow, EditRow } from './CreateRow'; +import CreateRow from './CreateRow'; +import EditRow from './EditRow'; interface NestedRowsProps { /** The parent row object from TanStack React Table */ @@ -103,7 +104,6 @@ const NestedRows = ({ setIsCreatingTopRow={setIsCreatingTopRow} exitDraftWithoutSave={onCancelCreation} createRowMutation={createRowMutation} - columns={[]} indent={indent} validate={validate} /> @@ -123,9 +123,9 @@ const NestedRows = ({ exitDraftWithoutSave(); }} updateRowMutation={updateRowMutation} - columns={columns} indent={indent} validate={validate} + row={row} /> ) : ( @@ -137,7 +137,7 @@ const NestedRows = ({ return ( {isFirstColumn ? (
{content}
diff --git a/src/taxonomy/tree-table/SaveErrorAlert.tsx b/src/taxonomy/tree-table/SaveErrorAlert.tsx new file mode 100644 index 0000000000..4476e06c79 --- /dev/null +++ b/src/taxonomy/tree-table/SaveErrorAlert.tsx @@ -0,0 +1,42 @@ +import React, { useEffect } from 'react'; +import { + Alert, +} from '@openedx/paragon'; + +import { Info } from '@openedx/paragon/icons'; +import { useIntl } from '@edx/frontend-platform/i18n'; +import './TableView.scss'; +import messages from './messages'; + +interface SaveErrorAlertProps { + draftError: string | undefined; + isError: boolean | undefined; + isUpdateError: boolean | undefined; +} +const SaveErrorAlert = ({ draftError, isError, isUpdateError }: SaveErrorAlertProps) => { + const intl = useIntl(); + const hasError = (isError || isUpdateError) && !!draftError; + const [alertOpen, setAlertOpen] = React.useState(hasError); + + useEffect(() => { + if (hasError) { + setAlertOpen(true); + } + if (!hasError) { + setAlertOpen(false); + } + }, [hasError, isError, isUpdateError, draftError]); + + if (!alertOpen) { return null; } + + return ( + { setAlertOpen(false); }}> + + {intl.formatMessage(messages.errorSavingTitle)} + + {intl.formatMessage(messages.errorSavingMessage, { errorMessage: draftError }) } + + ); +}; + +export default SaveErrorAlert; diff --git a/src/taxonomy/tree-table/TableBody.tsx b/src/taxonomy/tree-table/TableBody.tsx index 6e1819a289..99fca02dd7 100644 --- a/src/taxonomy/tree-table/TableBody.tsx +++ b/src/taxonomy/tree-table/TableBody.tsx @@ -13,7 +13,8 @@ import type { TreeColumnDef, TreeTable, } from './types'; -import { CreateRow, EditRow } from './CreateRow'; +import CreateRow from './CreateRow'; +import EditRow from './EditRow'; interface TableBodyProps { columns: TreeColumnDef[]; @@ -86,7 +87,6 @@ const TableBody = ({ setIsCreatingTopRow={setIsCreatingTopRow} exitDraftWithoutSave={exitDraftWithoutSave} createRowMutation={createRowMutation} - columns={columns} validate={validate} /> )} @@ -104,14 +104,14 @@ const TableBody = ({ exitDraftWithoutSave(); }} updateRowMutation={updateRowMutation} - columns={columns} validate={validate} + row={row} /> ) : ( {row.getVisibleCells() - .map((cell, index) => ( - + .map((cell) => ( + {flexRender(cell.column.columnDef.cell, cell.getContext())} ))} diff --git a/src/taxonomy/tree-table/TableView.tsx b/src/taxonomy/tree-table/TableView.tsx index 79aa625259..92ad3bd5fe 100644 --- a/src/taxonomy/tree-table/TableView.tsx +++ b/src/taxonomy/tree-table/TableView.tsx @@ -5,7 +5,6 @@ import { Card, ActionRow, Pagination, - Alert, Icon, } from '@openedx/paragon'; @@ -18,7 +17,7 @@ import { type PaginationState, } from '@tanstack/react-table'; -import { ArrowDropUpDown, Info } from '@openedx/paragon/icons'; +import { ArrowDropUpDown } from '@openedx/paragon/icons'; import { useIntl } from '@edx/frontend-platform/i18n'; import TableBody from './TableBody'; import './TableView.scss'; @@ -30,6 +29,7 @@ import type { TreeRowData, } from './types'; import messages from './messages'; +import SaveErrorAlert from './SaveErrorAlert'; interface TableViewProps { treeData: TreeRowData[]; @@ -102,18 +102,10 @@ const TableView = ({ const { isError } = createRowMutation; const { isError: isUpdateError } = updateRowMutation; - const [showError, setShowError] = React.useState(true); return ( <> - {(isError || isUpdateError) && showError && ( - setShowError(false)}> - - {intl.formatMessage(messages.errorSavingTitle)} - - {intl.formatMessage(messages.errorSavingMessage, { errorMessage: draftError || intl.formatMessage(messages.errorSavingMessage, { errorMessage: '' }) })} - - )} +
@@ -142,7 +134,7 @@ const TableView = ({ {headerGroup.headers.map((header, index) => ( {header.isPlaceholder ? null From aeab249c251f6c4d5a01f41df44feb44f35795ca Mon Sep 17 00:00:00 2001 From: Jesper Hodge Date: Tue, 14 Apr 2026 12:24:17 -0400 Subject: [PATCH 2/7] fix: lint and cleanup --- src/taxonomy/data/apiHooks.ts | 1 - src/taxonomy/tag-list/OptionalExpandLink.tsx | 2 +- src/taxonomy/tag-list/TagListTable.tsx | 4 ++-- src/taxonomy/tag-list/hooks.ts | 5 +++-- src/taxonomy/tag-list/tagTree.ts | 2 +- src/taxonomy/tree-table/EditableCell.tsx | 3 ++- 6 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/taxonomy/data/apiHooks.ts b/src/taxonomy/data/apiHooks.ts index 96c8020090..e0cd927598 100644 --- a/src/taxonomy/data/apiHooks.ts +++ b/src/taxonomy/data/apiHooks.ts @@ -13,7 +13,6 @@ import { useQuery, useMutation, useQueryClient } from '@tanstack/react-query'; import { camelCaseObject } from '@edx/frontend-platform'; import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; -import { useIntl } from '@edx/frontend-platform/i18n'; import { apiUrls, ALL_TAXONOMIES, getApiErrorMessage } from './api'; import * as api from './api'; import type { QueryOptions, TagListData } from './types'; diff --git a/src/taxonomy/tag-list/OptionalExpandLink.tsx b/src/taxonomy/tag-list/OptionalExpandLink.tsx index edfa6580f2..e42c198dcb 100644 --- a/src/taxonomy/tag-list/OptionalExpandLink.tsx +++ b/src/taxonomy/tag-list/OptionalExpandLink.tsx @@ -4,7 +4,7 @@ import { ExpandLess, ExpandMore } from '@openedx/paragon/icons'; import { Row } from '@tanstack/react-table'; import { useIntl } from '@edx/frontend-platform/i18n'; -import type { TreeRowData } from '../tree-table/types'; +import type { TreeRowData } from '@src/taxonomy/tree-table/types'; import messages from './messages'; interface OptionalExpandLinkProps { diff --git a/src/taxonomy/tag-list/TagListTable.tsx b/src/taxonomy/tag-list/TagListTable.tsx index c9c80adb08..bd62f675e2 100644 --- a/src/taxonomy/tag-list/TagListTable.tsx +++ b/src/taxonomy/tag-list/TagListTable.tsx @@ -4,9 +4,9 @@ import React, { useEffect, } from 'react'; import type { PaginationState } from '@tanstack/react-table'; -import { useTagListData, useCreateTag, useUpdateTag } from '../data/apiHooks'; +import { TableView } from '@src/taxonomy/tree-table'; +import { useTagListData, useCreateTag, useUpdateTag } from '@src/taxonomy/data/apiHooks'; import { TagTree } from './tagTree'; -import { TableView } from '../tree-table'; import type { RowId, TreeColumnDef, diff --git a/src/taxonomy/tag-list/hooks.ts b/src/taxonomy/tag-list/hooks.ts index ea2d7b6d0f..8ced5f3a5b 100644 --- a/src/taxonomy/tag-list/hooks.ts +++ b/src/taxonomy/tag-list/hooks.ts @@ -1,10 +1,11 @@ import { useReducer } from 'react'; import { useIntl } from '@edx/frontend-platform/i18n'; -import { useCreateTag, useUpdateTag } from '../data/apiHooks'; +import globalMessages from '@src/messages'; +import { useCreateTag, useUpdateTag } from '@src/taxonomy/data/apiHooks'; +import type { RowId } from '@src/taxonomy/tree-table/types'; import { TagTree } from './tagTree'; import { TagListTableError } from './errors'; -import type { RowId } from '../tree-table/types'; import { TABLE_MODES, TRANSITION_TABLE, diff --git a/src/taxonomy/tag-list/tagTree.ts b/src/taxonomy/tag-list/tagTree.ts index 06d43a04b6..f6cf9023c9 100644 --- a/src/taxonomy/tag-list/tagTree.ts +++ b/src/taxonomy/tag-list/tagTree.ts @@ -1,5 +1,5 @@ +import type { TagData } from '@src/taxonomy/data/types'; import { TagTreeError } from './errors'; -import type { TagData } from '../data/types'; export interface TagTreeNode extends TagData { subRows?: TagTreeNode[]; diff --git a/src/taxonomy/tree-table/EditableCell.tsx b/src/taxonomy/tree-table/EditableCell.tsx index 8cdc10aabc..d4de95fa74 100644 --- a/src/taxonomy/tree-table/EditableCell.tsx +++ b/src/taxonomy/tree-table/EditableCell.tsx @@ -7,8 +7,9 @@ import React, { import { Form } from '@openedx/paragon'; import { useIntl } from '@edx/frontend-platform/i18n'; + +import OptionalExpandLink from '@src/taxonomy/tag-list/OptionalExpandLink'; import messages from './messages'; -import OptionalExpandLink from '../tag-list/OptionalExpandLink'; /** * Props for the EditableCell component. From 3ec0783ee1749a7a1dd0e0fdccf545199ae39fca Mon Sep 17 00:00:00 2001 From: Jesper Hodge Date: Tue, 14 Apr 2026 12:30:23 -0400 Subject: [PATCH 3/7] fix: UsageCountDisplay --- src/taxonomy/tag-list/UsageCountDisplay.tsx | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/taxonomy/tag-list/UsageCountDisplay.tsx b/src/taxonomy/tag-list/UsageCountDisplay.tsx index 97a95ad026..2471a43f5f 100644 --- a/src/taxonomy/tag-list/UsageCountDisplay.tsx +++ b/src/taxonomy/tag-list/UsageCountDisplay.tsx @@ -13,12 +13,15 @@ const asTagListRowData = (row: Row): TagListRowData => ( const UsageCountDisplay = ({ row }: { row: Row }) => { const count = asTagListRowData(row).usageCount ?? 0; + + if (count <= 0) { + return null; + } + return ( - count > 0 && ( - - {count} - - ) + + {count} + ); }; From 8d58fc9c2917a4ddbe85588a9db15aae68ce1110 Mon Sep 17 00:00:00 2001 From: Jesper Hodge Date: Tue, 14 Apr 2026 12:53:09 -0400 Subject: [PATCH 4/7] fix: tests --- src/taxonomy/data/apiHooks.test.jsx | 2 +- src/taxonomy/tag-list/TagListTable.test.jsx | 26 +++++++++++++++++---- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/src/taxonomy/data/apiHooks.test.jsx b/src/taxonomy/data/apiHooks.test.jsx index 1f012ee946..f582a719c7 100644 --- a/src/taxonomy/data/apiHooks.test.jsx +++ b/src/taxonomy/data/apiHooks.test.jsx @@ -111,7 +111,7 @@ describe('import taxonomy api calls', () => { }); it('should surface duplicate tag error returned as an array', async () => { - const duplicateError = "Tag with value 'ab' already exists for taxonomy."; + const duplicateError = 'Request failed with status code 400'; axiosMock.onPost(apiUrls.createTag(1)).reply(400, [duplicateError]); const { result } = renderHook(() => useCreateTag(1), { wrapper }); diff --git a/src/taxonomy/tag-list/TagListTable.test.jsx b/src/taxonomy/tag-list/TagListTable.test.jsx index 1c63e4aa47..4e9c40f09e 100644 --- a/src/taxonomy/tag-list/TagListTable.test.jsx +++ b/src/taxonomy/tag-list/TagListTable.test.jsx @@ -587,8 +587,17 @@ describe('', () => { expect(screen.getByText(/invalid character/i)).toBeInTheDocument(); }); - it('should show an inline duplicate-name error when the entered root tag already exists', async () => { - axiosMock.onPost(createTagUrl).reply(400, ['Tag with this name already exists']); + it('should show failure feedback when creating a duplicate root tag name', async () => { + axiosMock.onPost(createTagUrl).reply(() => { + const error = new Error('Request failed with status code 400'); + error.name = 'AxiosError'; + error.response = { + data: { + tag: ['Tag with this name already exists'], + }, + }; + return Promise.reject(error); + }); fireEvent.click(await screen.findByLabelText('Create Tag')); const draftRow = await screen.findAllByRole('row'); @@ -598,12 +607,19 @@ describe('', () => { fireEvent.change(input, { target: { value: 'root tag 1' } }); fireEvent.click(saveButton); - expect(await screen.findByText('Tag with this name already exists')).toBeInTheDocument(); + expect(await screen.findByText('Error creating tag: Tag with this name already exists')).toBeInTheDocument(); }); it('should keep the inline row and show a failure toast when save request fails', async () => { - axiosMock.onPost(createTagUrl).reply(500, { - error: 'Internal server error', + axiosMock.onPost(createTagUrl).reply(() => { + const error = new Error('Request failed with status code 500'); + error.name = 'AxiosError'; + error.response = { + data: { + tag: ['Internal server error'], + }, + }; + return Promise.reject(error); }); fireEvent.click(await screen.findByLabelText('Create Tag')); From 5b23b51ecf19fb3d82aae1c2a78f652dc0b93b05 Mon Sep 17 00:00:00 2001 From: Jesper Hodge Date: Tue, 14 Apr 2026 12:56:11 -0400 Subject: [PATCH 5/7] fix: test --- src/taxonomy/tree-table/TableView.test.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/src/taxonomy/tree-table/TableView.test.tsx b/src/taxonomy/tree-table/TableView.test.tsx index f1382eeea5..960e83203c 100644 --- a/src/taxonomy/tree-table/TableView.test.tsx +++ b/src/taxonomy/tree-table/TableView.test.tsx @@ -48,6 +48,7 @@ describe('TableView', () => { it('shows and dismisses save error banner', () => { const props = baseProps(); props.createRowMutation = { isPending: false, isError: true }; + props.draftError = 'Request failed with status code 500'; render(, { wrapper }); From 1494fa8d42d7195be491e9a540d82ce13f323661 Mon Sep 17 00:00:00 2001 From: Jesper Hodge Date: Tue, 14 Apr 2026 15:59:31 -0400 Subject: [PATCH 6/7] refactor: add context and make more readable --- src/taxonomy/tag-list/TagListContext.tsx | 37 ++++ src/taxonomy/tag-list/TagListTable.tsx | 124 ++++++----- src/taxonomy/tag-list/editActionUtils.ts | 81 +++++++ src/taxonomy/tag-list/hooks.test.tsx | 2 + src/taxonomy/tag-list/hooks.ts | 84 +++----- src/taxonomy/tag-list/tagColumns.tsx | 228 ++++++++++---------- src/taxonomy/tree-table/CreateRow.test.tsx | 76 ++++--- src/taxonomy/tree-table/CreateRow.tsx | 38 ++-- src/taxonomy/tree-table/DisplayRow.tsx | 34 +++ src/taxonomy/tree-table/EditRow.tsx | 32 +-- src/taxonomy/tree-table/NestedRows.test.tsx | 100 +++++---- src/taxonomy/tree-table/NestedRows.tsx | 123 ++--------- src/taxonomy/tree-table/SaveErrorAlert.tsx | 9 +- src/taxonomy/tree-table/TableBody.tsx | 90 +------- src/taxonomy/tree-table/TableView.test.tsx | 62 ++++-- src/taxonomy/tree-table/TableView.tsx | 50 +---- src/taxonomy/tree-table/rowHelpers.ts | 5 + 17 files changed, 578 insertions(+), 597 deletions(-) create mode 100644 src/taxonomy/tag-list/TagListContext.tsx create mode 100644 src/taxonomy/tag-list/editActionUtils.ts create mode 100644 src/taxonomy/tree-table/DisplayRow.tsx create mode 100644 src/taxonomy/tree-table/rowHelpers.ts diff --git a/src/taxonomy/tag-list/TagListContext.tsx b/src/taxonomy/tag-list/TagListContext.tsx new file mode 100644 index 0000000000..4578e394a1 --- /dev/null +++ b/src/taxonomy/tag-list/TagListContext.tsx @@ -0,0 +1,37 @@ +import React, { createContext, useContext } from 'react'; + +import type { RowId, CreateRowMutationState } from '@src/taxonomy/tree-table/types'; + +interface TagListContextValue { + isCreatingTopTag: boolean; + setIsCreatingTopTag: React.Dispatch>; + creatingParentId: RowId | null; + setCreatingParentId: React.Dispatch>; + editingRowId: RowId | null; + setEditingRowId: React.Dispatch>; + draftError: string; + setDraftError: React.Dispatch>; + hasOpenDraft: boolean; + canAddTag: boolean; + maxDepth: number; + createTagMutation: CreateRowMutationState; + updateTagMutation: CreateRowMutationState; + handleCreateTag: (value: string, parentTagValue?: string) => Promise; + handleUpdateTag: (value: string, originalValue: string) => Promise; + validate: (value: string, mode?: 'soft' | 'hard') => boolean; + startDraftMode: () => void; + exitDraftWithoutSave: () => void; +} + +const TagListContext = createContext(null); + +const useTagListContext = (): TagListContextValue => { + const context = useContext(TagListContext); + if (!context) { + throw new Error('useTagListContext must be used within TagListContext.Provider'); + } + return context; +}; + +export type { TagListContextValue }; +export { TagListContext, useTagListContext }; diff --git a/src/taxonomy/tag-list/TagListTable.tsx b/src/taxonomy/tag-list/TagListTable.tsx index bd62f675e2..dcb90f81ce 100644 --- a/src/taxonomy/tag-list/TagListTable.tsx +++ b/src/taxonomy/tag-list/TagListTable.tsx @@ -9,13 +9,14 @@ import { useTagListData, useCreateTag, useUpdateTag } from '@src/taxonomy/data/a import { TagTree } from './tagTree'; import type { RowId, - TreeColumnDef, TreeRowData, + ToastState, } from '../tree-table/types'; import { TABLE_MODES, } from './constants'; -import { getColumns } from './tagColumns'; +import { useTagColumns } from './tagColumns'; +import { TagListContext } from './TagListContext'; import { useTableModes, useEditActions } from './hooks'; interface TagListTableProps { @@ -26,6 +27,41 @@ interface TagListTableProps { // TODO: Fix and enable pagination on backend and frontend.For now, disable pagination by showing all tags on one page. const DISABLE_PAGINATION = true; +interface TagListTableContentProps { + treeData: TreeRowData[]; + pageCount: number; + pagination: PaginationState; + handlePaginationChange: React.Dispatch>; + isLoading: boolean; + toast: ToastState; + setToast: React.Dispatch>; +} + +const TagListTableContent = ({ + treeData, + pageCount, + pagination, + handlePaginationChange, + isLoading, + toast, + setToast, +}: TagListTableContentProps) => { + const columns = useTagColumns(); + + return ( + + ); +}; + const TagListTable = ({ taxonomyId, maxDepth }: TagListTableProps) => { // The table has a VIEW, DRAFT, and a PREVIEW mode. It starts in VIEW mode. // It switches to DRAFT mode when a user edits or creates a tag. @@ -47,7 +83,6 @@ const TagListTable = ({ taxonomyId, maxDepth }: TagListTableProps) => { const [toast, setToast] = useState({ show: false, message: '', variant: 'success' }); const [tagTree, setTagTree] = useState(null); const [isCreatingTopTag, setIsCreatingTopTag] = useState(false); - const [activeActionMenuRowId, setActiveActionMenuRowId] = useState(null); const [draftError, setDraftError] = useState(''); const treeData = (tagTree?.getAllAsDeepCopy() || []) as unknown as TreeRowData[]; const hasOpenDraft = isCreatingTopTag || creatingParentId !== null || editingRowId !== null; @@ -81,8 +116,6 @@ const TagListTable = ({ taxonomyId, maxDepth }: TagListTableProps) => { const updateTagMutation = useUpdateTag(taxonomyId); const pageCount = tagList?.numPages ?? -1; - // TODO: to make this more readable, introduce a React context for the TagListTable instead of passing props. - // Custom Edit Actions Hook - handles table mode transitions, API calls, // and updating the table without a full data reload when creating or editing tags. const { handleCreateTag, handleUpdateTag, validate } = useEditActions({ @@ -98,38 +131,46 @@ const TagListTable = ({ taxonomyId, maxDepth }: TagListTableProps) => { setEditingRowId, }); - const columns = useMemo( - () => getColumns({ + const contextValue = useMemo( + () => ({ + isCreatingTopTag, setIsCreatingTopTag, + creatingParentId, setCreatingParentId, - handleUpdateTag, + editingRowId, setEditingRowId, - onStartDraft: enterDraftMode, - setActiveActionMenuRowId, - hasOpenDraft, - canAddTag: tagList?.canAddTag !== false, draftError, setDraftError, - isSavingDraft: createTagMutation.isPending, + hasOpenDraft, + canAddTag: tagList?.canAddTag !== false, maxDepth, + createTagMutation, + updateTagMutation, + handleCreateTag, + handleUpdateTag, + validate, + startDraftMode: enterDraftMode, + exitDraftWithoutSave, }), [ isCreatingTopTag, - tableMode, - activeActionMenuRowId, - hasOpenDraft, + setIsCreatingTopTag, creatingParentId, - tagList?.canAddTag, + setCreatingParentId, + editingRowId, + setEditingRowId, draftError, - createTagMutation.isPending, + setDraftError, + hasOpenDraft, + tagList?.canAddTag, maxDepth, - setIsCreatingTopTag, - setCreatingParentId, + createTagMutation, + updateTagMutation, + handleCreateTag, handleUpdateTag, - setEditingRowId, + validate, enterDraftMode, - setActiveActionMenuRowId, - setDraftError, + exitDraftWithoutSave, ], ); @@ -146,32 +187,17 @@ const TagListTable = ({ taxonomyId, maxDepth }: TagListTableProps) => { }, [tagList?.results, tableMode]); return ( - + + + ); }; diff --git a/src/taxonomy/tag-list/editActionUtils.ts b/src/taxonomy/tag-list/editActionUtils.ts new file mode 100644 index 0000000000..05e491fdde --- /dev/null +++ b/src/taxonomy/tag-list/editActionUtils.ts @@ -0,0 +1,81 @@ +import { useIntl } from '@edx/frontend-platform/i18n'; + +import globalMessages from '@src/messages'; +import { TagTree } from './tagTree'; +import { TAG_NAME_PATTERN } from './constants'; +import messages from './messages'; + +type IntlShape = ReturnType; + +const getInlineValidationMessage = (value: string, intl: IntlShape): string => { + const trimmed = value.trim(); + + if (!trimmed) { + return intl.formatMessage(messages.nameRequired); + } + + if (!TAG_NAME_PATTERN.test(trimmed)) { + return intl.formatMessage(messages.invalidCharacterInTagName); + } + + return ''; +}; + +const formatTagRequestError = (error: unknown, intl: IntlShape): string => { + if (error && typeof error === 'object' && 'name' in error && error.name === 'AxiosError') { + const responseData = (error as { response?: { data?: unknown } }).response?.data; + const normalizedData = responseData && typeof responseData === 'object' ? responseData : {}; + const tagError = Object.entries(normalizedData).find(([key]) => ( + ['tag', 'value', 'updated_tag_value'].includes(key.toLowerCase()) + )); + + const errorMessages = tagError?.[1] + ?? ((error as Error).message || intl.formatMessage(globalMessages.unknownError)); + const message = Array.isArray(errorMessages) + ? errorMessages.join('; ') + : String(errorMessages); + + return message.replace(/\.$/, ''); + } + + const fallback = (error as Error)?.message || intl.formatMessage(globalMessages.unknownError); + return fallback.replace(/\.$/, ''); +}; + +const renameTagInTree = ( + currentTagTree: TagTree | null, + oldValue: string, + newValue: string, +): TagTree => { + const nextTree = currentTagTree || new TagTree([]); + nextTree.editTagValue(oldValue, newValue); + return nextTree; +}; + +const addTagToTree = ( + currentTagTree: TagTree | null, + value: string, + parentTagValue: string | null, +): TagTree => { + const nextTree = currentTagTree || new TagTree([]); + const parentTag = parentTagValue ? nextTree.getTagAsDeepCopy(parentTagValue) : null; + + nextTree.addNode({ + id: Date.now(), + value, + parentValue: parentTagValue, + depth: parentTag ? parentTag.depth + 1 : 0, + childCount: 0, + subTagsUrl: null, + externalId: '', + }, parentTagValue); + + return nextTree; +}; + +export { + getInlineValidationMessage, + formatTagRequestError, + renameTagInTree, + addTagToTree, +}; \ No newline at end of file diff --git a/src/taxonomy/tag-list/hooks.test.tsx b/src/taxonomy/tag-list/hooks.test.tsx index fc3dbdaebb..74afba485e 100644 --- a/src/taxonomy/tag-list/hooks.test.tsx +++ b/src/taxonomy/tag-list/hooks.test.tsx @@ -148,6 +148,7 @@ describe('useEditActions', () => { expect(setToast).toHaveBeenCalledWith({ show: true, message: 'Tag "updated" updated successfully', + variant: 'success', }); expect(setEditingRowId).toHaveBeenCalledWith(null); }); @@ -169,6 +170,7 @@ describe('useEditActions', () => { expect(setToast).toHaveBeenCalledWith({ show: true, message: 'Error creating tag: server failed', + variant: 'danger', }); }); }); diff --git a/src/taxonomy/tag-list/hooks.ts b/src/taxonomy/tag-list/hooks.ts index 8ced5f3a5b..092228893d 100644 --- a/src/taxonomy/tag-list/hooks.ts +++ b/src/taxonomy/tag-list/hooks.ts @@ -1,17 +1,21 @@ import { useReducer } from 'react'; import { useIntl } from '@edx/frontend-platform/i18n'; -import globalMessages from '@src/messages'; import { useCreateTag, useUpdateTag } from '@src/taxonomy/data/apiHooks'; -import type { RowId } from '@src/taxonomy/tree-table/types'; +import type { RowId, ToastState } from '@src/taxonomy/tree-table/types'; import { TagTree } from './tagTree'; import { TagListTableError } from './errors'; import { TABLE_MODES, TRANSITION_TABLE, TABLE_MODE_ACTIONS, - TAG_NAME_PATTERN, } from './constants'; +import { + getInlineValidationMessage, + formatTagRequestError, + renameTagInTree, + addTagToTree, +} from './editActionUtils'; import messages from './messages'; @@ -41,7 +45,7 @@ interface UseEditActionsParams { setDraftError: React.Dispatch>; createTagMutation: ReturnType; enterPreviewMode: () => void; - setToast: React.Dispatch>; + setToast: React.Dispatch>; setIsCreatingTopTag: React.Dispatch>; setCreatingParentId: React.Dispatch>; exitDraftWithoutSave: () => void; @@ -49,17 +53,6 @@ interface UseEditActionsParams { updateTagMutation: ReturnType; } -const getInlineValidationMessage = (value: string, intl: ReturnType): string => { - const trimmed = value.trim(); - if (!trimmed) { - return intl.formatMessage(messages.nameRequired); - } - if (!TAG_NAME_PATTERN.test(trimmed)) { - return intl.formatMessage(messages.invalidCharacterInTagName); - } - return ''; -}; - /** Table mode reducer for React's `useReducer` hook. * This will throw an error if an invalid table mode transition is attempted, * as defined in the `TRANSITION_TABLE` constant. @@ -120,30 +113,11 @@ const useEditActions = ({ const intl = useIntl(); const updateTableAfterRename = (oldValue: string, newValue: string) => { - setTagTree((currentTagTree) => { - const nextTree = currentTagTree || new TagTree([]); - nextTree.editTagValue(oldValue, newValue); - return nextTree; - }); + setTagTree((currentTagTree) => renameTagInTree(currentTagTree, oldValue, newValue)); }; const updateTableWithoutDataReload = (value: string, parentTagValue: string | null = null) => { - setTagTree((currentTagTree) => { - const nextTree = currentTagTree || new TagTree([]); - const parentTag = parentTagValue ? nextTree.getTagAsDeepCopy(parentTagValue) : null; - - nextTree.addNode({ - id: Date.now(), - value, - parentValue: parentTagValue, - depth: parentTag ? parentTag.depth + 1 : 0, - childCount: 0, - subTagsUrl: null, - externalId: '', - }, parentTagValue); - - return nextTree; - }); + setTagTree((currentTagTree) => addTagToTree(currentTagTree, value, parentTagValue)); }; /** Validates a tag value and sets a draft error message if invalid. @@ -164,26 +138,6 @@ const useEditActions = ({ return true; }; - const getErrorMessage = (error: any): string => { - let errorMessage: string = ''; - if (error.name === 'AxiosError') { - const responseData = error.response?.data; - const tagError = Object.entries(responseData)?.find((errItem: [string, unknown]) => ( - ['tag', 'value', 'updated_tag_value'].includes(errItem[0].toLowerCase()) - )); - - const errorMessages = tagError ? tagError[1] : ( - (error as Error).message || intl.formatMessage(globalMessages.unknownError) - ); - errorMessage = Array.isArray(errorMessages) ? errorMessages.join('; ') : String(errorMessages); - } else { - errorMessage = (error as Error).message || intl.formatMessage(globalMessages.unknownError); - } - - errorMessage = errorMessage.replace(/\.$/, ''); // Remove trailing period for better message formatting - return errorMessage; - }; - const handleCreateTag = async (value: string, parentTagValue?: string) => { const trimmed = value.trim(); @@ -199,13 +153,18 @@ const useEditActions = ({ setToast({ show: true, message: intl.formatMessage(messages.tagCreationSuccessMessage, { name: trimmed }), + variant: 'success', }); setIsCreatingTopTag(false); setCreatingParentId(null); } catch (error) { - const errorMessage = getErrorMessage(error); + const errorMessage = formatTagRequestError(error, intl); setDraftError(errorMessage); - setToast({ show: true, message: intl.formatMessage(messages.tagCreationErrorMessage, { errorMessage }) }); + setToast({ + show: true, + message: intl.formatMessage(messages.tagCreationErrorMessage, { errorMessage }), + variant: 'danger', + }); } }; @@ -230,11 +189,16 @@ const useEditActions = ({ setToast({ show: true, message: intl.formatMessage(messages.tagUpdateSuccessMessage, { name: trimmed }), + variant: 'success', }); } catch (error) { - const errorMessage = getErrorMessage(error); + const errorMessage = formatTagRequestError(error, intl); setDraftError(errorMessage); - setToast({ show: true, message: intl.formatMessage(messages.tagUpdateErrorMessage, { errorMessage }) }); + setToast({ + show: true, + message: intl.formatMessage(messages.tagUpdateErrorMessage, { errorMessage }), + variant: 'danger', + }); } }; diff --git a/src/taxonomy/tag-list/tagColumns.tsx b/src/taxonomy/tag-list/tagColumns.tsx index 363a1a9a12..badb0c94b4 100644 --- a/src/taxonomy/tag-list/tagColumns.tsx +++ b/src/taxonomy/tag-list/tagColumns.tsx @@ -4,6 +4,7 @@ import { IconButtonWithTooltip, Dropdown, } from '@openedx/paragon'; +import { useMemo } from 'react'; import { AddCircle, MoreVert, @@ -12,10 +13,10 @@ import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n'; import type { Row } from '@tanstack/react-table'; import type { - RowId, TreeColumnDef, TreeRowData, } from '@src/taxonomy/tree-table/types'; +import { useTagListContext } from './TagListContext'; import { TagListRowData } from './types'; import messages from './messages'; import OptionalExpandLink from './OptionalExpandLink'; @@ -27,38 +28,21 @@ const asTagListRowData = (row: Row): TagListRowData => ( row.original as unknown as TagListRowData ); -interface GetColumnsArgs { - setIsCreatingTopTag: (isCreating: boolean) => void; - setCreatingParentId: (id: RowId | null) => void; - handleUpdateTag: (value: string, originalValue: string) => void; - setEditingRowId: (id: RowId | null) => void; - onStartDraft: () => void; - setActiveActionMenuRowId: (id: RowId | null) => void; - hasOpenDraft: boolean; - canAddTag: boolean; - draftError: string; - setDraftError: (error: string) => void; - isSavingDraft: boolean; - maxDepth: number; -} - interface ActionsHeaderProps { - onStartDraft: () => void; + startDraftMode: () => void; setDraftError: (error: string) => void; setIsCreatingTopTag: (isCreating: boolean) => void; - setEditingRowId: (id: RowId | null) => void; - setActiveActionMenuRowId: (id: RowId | null) => void; + setEditingRowId: (id: string | number | null) => void; hasOpenDraft: boolean; draftInProgressHintId: string; canAddTag: boolean; } const ActionsHeader = ({ - onStartDraft, + startDraftMode, setDraftError, setIsCreatingTopTag, setEditingRowId, - setActiveActionMenuRowId, hasOpenDraft, canAddTag, draftInProgressHintId, @@ -73,11 +57,10 @@ const ActionsHeader = ({ alt={intl.formatMessage(messages.createTagButtonLabel)} size="inline" onClick={() => { - onStartDraft(); + startDraftMode(); setDraftError(''); setIsCreatingTopTag(true); setEditingRowId(null); - setActiveActionMenuRowId(null); }} disabled={hasOpenDraft || !canAddTag} aria-describedby={hasOpenDraft ? draftInProgressHintId : undefined} @@ -136,101 +119,110 @@ const ActionsMenu = ({ ); }; -function getColumns({ - setIsCreatingTopTag, - setCreatingParentId, - setEditingRowId, - onStartDraft, - setActiveActionMenuRowId, - hasOpenDraft, - canAddTag, - setDraftError, - maxDepth, -}: GetColumnsArgs): TreeColumnDef[] { - const reachedMaxDepth = (row: Row) => row.depth >= maxDepth; - const draftInProgressHintId = 'tag-list-draft-in-progress-hint'; - - return [ - { - id: 'valueColumn', - header: () => , - cell: ({ row }) => { - const { - value, - } = asTagListRowData(row); - - return ( - - - {value} - - ); +const useTagColumns = (): TreeColumnDef[] => { + const { + setIsCreatingTopTag, + setCreatingParentId, + setEditingRowId, + startDraftMode, + hasOpenDraft, + canAddTag, + setDraftError, + maxDepth, + } = useTagListContext(); + + return useMemo(() => { + const reachedMaxDepth = (row: Row) => row.depth >= maxDepth; + const draftInProgressHintId = 'tag-list-draft-in-progress-hint'; + + return [ + { + id: 'valueColumn', + header: () => , + cell: ({ row }) => { + const { + value, + } = asTagListRowData(row); + + return ( + + + {value} + + ); + }, }, - }, - { - id: 'count', - header: () => , - cell: UsageCountDisplay, - }, - { - id: 'actions', - header: () => ( - - ), - cell: ({ row }) => { - const rowData = asTagListRowData(row); - - if (rowData.isNew || rowData.isEditing) { - return
; - } - - const disableAddSubtag = hasOpenDraft || !canAddTag; - const disableEditTag = hasOpenDraft || row.original.canChangeTag === false; - - const startSubtagDraft = () => { - onStartDraft(); - setDraftError(''); - setCreatingParentId(rowData.id); - setEditingRowId(null); - setIsCreatingTopTag(false); - setActiveActionMenuRowId(null); - row.toggleExpanded(true); - }; - - const editTag = () => { - onStartDraft(); - setDraftError(''); - setEditingRowId(`${rowData.id}:${rowData.value}`); - setCreatingParentId(null); - setIsCreatingTopTag(false); - setActiveActionMenuRowId(null); - }; - - return ( -
- -
- ); + { + id: 'count', + header: () => , + cell: UsageCountDisplay, }, - }, - ]; -} + { + id: 'actions', + header: () => ( + + ), + cell: ({ row }) => { + const rowData = asTagListRowData(row); + + if (rowData.isNew || rowData.isEditing) { + return
; + } + + const disableAddSubtag = hasOpenDraft || !canAddTag; + const disableEditTag = hasOpenDraft || row.original.canChangeTag === false; + + const startSubtagDraft = () => { + startDraftMode(); + setDraftError(''); + setCreatingParentId(rowData.id); + setEditingRowId(null); + setIsCreatingTopTag(false); + row.toggleExpanded(true); + }; + + const editTag = () => { + startDraftMode(); + setDraftError(''); + setEditingRowId(`${rowData.id}:${rowData.value}`); + setCreatingParentId(null); + setIsCreatingTopTag(false); + }; + + return ( +
+ +
+ ); + }, + }, + ]; + }, [ + maxDepth, + startDraftMode, + setDraftError, + setCreatingParentId, + setEditingRowId, + setIsCreatingTopTag, + hasOpenDraft, + canAddTag, + ]); +}; -export { getColumns, EDITABLE_COLUMNS }; +export { useTagColumns, EDITABLE_COLUMNS }; diff --git a/src/taxonomy/tree-table/CreateRow.test.tsx b/src/taxonomy/tree-table/CreateRow.test.tsx index 7510f3f3bb..9c0c857a3d 100644 --- a/src/taxonomy/tree-table/CreateRow.test.tsx +++ b/src/taxonomy/tree-table/CreateRow.test.tsx @@ -1,6 +1,7 @@ import React from 'react'; import { IntlProvider } from '@edx/frontend-platform/i18n'; import { fireEvent, render, screen } from '@testing-library/react'; +import { TagListContext } from '@src/taxonomy/tag-list/TagListContext'; import CreateRow from './CreateRow'; @@ -8,25 +9,38 @@ const wrapper = ({ children }: { children: React.ReactNode }) => ( {children} ); -const baseProps = () => ({ +const baseContext = () => ({ + isCreatingTopTag: false, + setIsCreatingTopTag: jest.fn(), + creatingParentId: null, + setCreatingParentId: jest.fn(), + editingRowId: null, + setEditingRowId: jest.fn(), draftError: '', setDraftError: jest.fn(), - handleCreateRow: jest.fn(), - setIsCreatingTopRow: jest.fn(), - exitDraftWithoutSave: jest.fn(), - createRowMutation: { isPending: false }, + hasOpenDraft: false, + canAddTag: true, + maxDepth: 3, + createTagMutation: { isPending: false, isError: false }, + updateTagMutation: { isPending: false, isError: false }, + handleCreateTag: jest.fn(), + handleUpdateTag: jest.fn(), validate: jest.fn((value: string) => value.trim().length > 0), + startDraftMode: jest.fn(), + exitDraftWithoutSave: jest.fn(), }); describe('CreateRow', () => { it('saves on Enter when value is valid', () => { - const props = baseProps(); + const context = baseContext(); render( - - - - -
, + + + + + +
+
, { wrapper }, ); @@ -34,19 +48,21 @@ describe('CreateRow', () => { fireEvent.change(input, { target: { value: ' new tag ' } }); fireEvent.keyDown(input, { key: 'Enter' }); - expect(props.handleCreateRow).toHaveBeenCalledWith('new tag'); + expect(context.handleCreateTag).toHaveBeenCalledWith('new tag', undefined); }); it('does not save on Enter when mutation is pending', () => { - const props = baseProps(); - props.createRowMutation = { isPending: true }; + const context = baseContext(); + context.createTagMutation = { isPending: true, isError: false }; render( - - - - -
, + + + + + +
+
, { wrapper }, ); @@ -54,18 +70,20 @@ describe('CreateRow', () => { fireEvent.change(input, { target: { value: 'pending tag' } }); fireEvent.keyDown(input, { key: 'Enter' }); - expect(props.handleCreateRow).not.toHaveBeenCalled(); + expect(context.handleCreateTag).not.toHaveBeenCalled(); }); it('cancels on Escape and resets draft state', () => { - const props = baseProps(); + const context = baseContext(); render( - - - - -
, + + + + + +
+
, { wrapper }, ); @@ -73,8 +91,8 @@ describe('CreateRow', () => { fireEvent.change(input, { target: { value: 'will cancel' } }); fireEvent.keyDown(input, { key: 'Escape' }); - expect(props.setDraftError).toHaveBeenCalledWith(''); - expect(props.setIsCreatingTopRow).toHaveBeenCalledWith(false); - expect(props.exitDraftWithoutSave).toHaveBeenCalled(); + expect(context.setDraftError).toHaveBeenCalledWith(''); + expect(context.setIsCreatingTopTag).toHaveBeenCalledWith(false); + expect(context.exitDraftWithoutSave).toHaveBeenCalled(); }); }); diff --git a/src/taxonomy/tree-table/CreateRow.tsx b/src/taxonomy/tree-table/CreateRow.tsx index f130560b02..3f951e6cd6 100644 --- a/src/taxonomy/tree-table/CreateRow.tsx +++ b/src/taxonomy/tree-table/CreateRow.tsx @@ -1,40 +1,42 @@ import React from 'react'; -import type { CreateRowMutationState } from './types'; +import { useTagListContext } from '@src/taxonomy/tag-list/TagListContext'; import DraftRow from './DraftRow'; interface CreateRowProps { - draftError: string; - setDraftError: (error: string) => void; - handleCreateRow: (value: string) => void; - setIsCreatingTopRow: (isCreating: boolean) => void; - exitDraftWithoutSave: () => void; - createRowMutation: CreateRowMutationState; + parentRowValue?: string; indent?: number; - validate: (value: string, mode?: 'soft' | 'hard') => boolean; } const CreateRow: React.FC = ({ - draftError, - setDraftError, - handleCreateRow, - setIsCreatingTopRow, - exitDraftWithoutSave, - createRowMutation, + parentRowValue, indent = 0, - validate, }) => { + const { + draftError, + setDraftError, + handleCreateTag, + setIsCreatingTopTag, + setCreatingParentId, + exitDraftWithoutSave, + createTagMutation, + validate, + } = useTagListContext(); + const handleCancel = () => { setDraftError(''); - setIsCreatingTopRow(false); + setIsCreatingTopTag(false); + if (parentRowValue) { + setCreatingParentId(null); + } exitDraftWithoutSave(); }; return ( handleCreateTag(value, parentRowValue)} onCancel={handleCancel} - mutationState={createRowMutation} + mutationState={createTagMutation} indent={indent} validate={validate} rowId="creating-top-row" diff --git a/src/taxonomy/tree-table/DisplayRow.tsx b/src/taxonomy/tree-table/DisplayRow.tsx new file mode 100644 index 0000000000..68bdc5d23f --- /dev/null +++ b/src/taxonomy/tree-table/DisplayRow.tsx @@ -0,0 +1,34 @@ +import React from 'react'; +import { flexRender } from '@tanstack/react-table'; + +import type { TreeRow } from './types'; + +interface DisplayRowProps { + row: TreeRow; + indent?: number; +} + +const DisplayRow = ({ row, indent = 0 }: DisplayRowProps) => ( + + {row.getVisibleCells().map((cell, index) => { + const content = flexRender(cell.column.columnDef.cell, cell.getContext()); + const isFirstColumn = index === 0; + const shouldIndent = isFirstColumn && indent > 0; + + return ( + + {shouldIndent ? ( +
{content}
+ ) : ( + content + )} + + ); + })} + +); + +export default DisplayRow; \ No newline at end of file diff --git a/src/taxonomy/tree-table/EditRow.tsx b/src/taxonomy/tree-table/EditRow.tsx index 12a83fa055..a38c25857d 100644 --- a/src/taxonomy/tree-table/EditRow.tsx +++ b/src/taxonomy/tree-table/EditRow.tsx @@ -1,43 +1,43 @@ import React from 'react'; import { Row } from '@tanstack/react-table'; -import type { CreateRowMutationState, TreeRowData } from './types'; +import { useTagListContext } from '@src/taxonomy/tag-list/TagListContext'; +import type { TreeRowData } from './types'; import DraftRow from './DraftRow'; interface EditRowProps { - draftError: string; - setDraftError: (error: string) => void; initialValue: string; - handleUpdateRow: (value: string) => void; - cancelEditRow: () => void; - updateRowMutation: CreateRowMutationState; indent?: number; - validate: (value: string, mode?: 'soft' | 'hard') => boolean; row: Row; } const EditRow: React.FC = ({ - draftError, - setDraftError, initialValue, - handleUpdateRow, - cancelEditRow, - updateRowMutation, indent = 0, - validate, row, }) => { + const { + draftError, + setDraftError, + handleUpdateTag, + setEditingRowId, + exitDraftWithoutSave, + updateTagMutation, + validate, + } = useTagListContext(); + const handleCancel = () => { setDraftError(''); - cancelEditRow(); + setEditingRowId(null); + exitDraftWithoutSave(); }; return ( handleUpdateTag(value, initialValue)} onCancel={handleCancel} - mutationState={updateRowMutation} + mutationState={updateTagMutation} indent={indent} validate={validate} requireValueChangeToEnableSave diff --git a/src/taxonomy/tree-table/NestedRows.test.tsx b/src/taxonomy/tree-table/NestedRows.test.tsx index b642e7bd31..27ebf15f5f 100644 --- a/src/taxonomy/tree-table/NestedRows.test.tsx +++ b/src/taxonomy/tree-table/NestedRows.test.tsx @@ -1,6 +1,7 @@ import React from 'react'; import { IntlProvider } from '@edx/frontend-platform/i18n'; import { fireEvent, render, screen } from '@testing-library/react'; +import { TagListContext } from '@src/taxonomy/tag-list/TagListContext'; import NestedRows from './NestedRows'; @@ -8,16 +9,27 @@ const wrapper = ({ children }: { children: React.ReactNode }) => ( {children} ); -const defaultRequiredProps = { - setIsCreatingTopRow: jest.fn(), - createRowMutation: {}, - updateRowMutation: {}, - handleUpdateRow: jest.fn(), +const defaultContext = (overrides = {}) => ({ + isCreatingTopTag: false, + setIsCreatingTopTag: jest.fn(), + creatingParentId: null, + setCreatingParentId: jest.fn(), editingRowId: null, setEditingRowId: jest.fn(), - exitDraftWithoutSave: jest.fn(), + draftError: '', + setDraftError: jest.fn(), + hasOpenDraft: false, + canAddTag: true, + maxDepth: 3, + createTagMutation: { isPending: false, isError: false }, + updateTagMutation: { isPending: false, isError: false }, + handleCreateTag: jest.fn(), + handleUpdateTag: jest.fn(), validate: () => true, -}; + startDraftMode: jest.fn(), + exitDraftWithoutSave: jest.fn(), + ...overrides, +}); const makeCell = (id: string, content: string) => ({ id, @@ -46,16 +58,19 @@ const makeRow = ({ describe('NestedRows', () => { it('renders nothing when parent row is collapsed', () => { const parent = makeRow({ id: 1, value: 'parent', expanded: false }); + const context = defaultContext(); + const { container } = render( - - - - -
, + + + + + +
+
, { wrapper }, ); @@ -71,30 +86,28 @@ describe('NestedRows', () => { subRows: [nestedChild], }); const setCreatingParentId = jest.fn(); - const onCancelCreation = jest.fn(); + const exitDraftWithoutSave = jest.fn(); + const context = defaultContext({ creatingParentId: 1, setCreatingParentId, exitDraftWithoutSave }); render( - - - - -
, + + + + + +
+
, { wrapper }, ); fireEvent.click(screen.getByText('Cancel')); expect(setCreatingParentId).toHaveBeenCalledWith(null); - expect(onCancelCreation).toHaveBeenCalled(); + expect(exitDraftWithoutSave).toHaveBeenCalled(); }); it('renders EditRow when editingRowId matches the child row id and value', () => { @@ -105,19 +118,20 @@ describe('NestedRows', () => { expanded: true, subRows: [nestedChild], }); + const context = defaultContext({ editingRowId: '2:child' }); render( - - - - -
, + + + + + +
+
, { wrapper }, ); diff --git a/src/taxonomy/tree-table/NestedRows.tsx b/src/taxonomy/tree-table/NestedRows.tsx index a0c42b5db8..1cf8422ae7 100644 --- a/src/taxonomy/tree-table/NestedRows.tsx +++ b/src/taxonomy/tree-table/NestedRows.tsx @@ -1,57 +1,23 @@ import React from 'react'; -import { flexRender } from '@tanstack/react-table'; +import { useTagListContext } from '@src/taxonomy/tag-list/TagListContext'; import type { - RowId, TreeRow, - TreeColumnDef, - CreateRowMutationState, } from './types'; import CreateRow from './CreateRow'; import EditRow from './EditRow'; +import DisplayRow from './DisplayRow'; +import { getTreeRowEditId } from './rowHelpers'; interface NestedRowsProps { /** The parent row object from TanStack React Table */ parentRow: TreeRow; /** The value identifier of the parent row */ parentRowValue: string; - /** Whether a new child row is currently being created for this parent */ - isCreating?: boolean; - /** Callback when a new child row is saved (receives value and parentRowValue) */ - onSaveNewChildRow?: (value: string, parentRowValue: string) => void; - /** Callback when child row creation is cancelled */ - onCancelCreation?: () => void; /** Array of child row objects to render */ childRowsData?: TreeRow[]; /** Current nesting depth level (used for indentation calculation) */ depth?: number; - /** Error message to display in draft creation form */ - draftError?: string; - /** Setter function for draft error state */ - setDraftError?: (error: string) => void; - /** ID of the row currently in creation mode */ - creatingParentId?: RowId | null; - /** Setter function for which row is in creation mode */ - setCreatingParentId?: (value: RowId | null) => void; - /** Callback to set whether top-level row creation is active */ - setIsCreatingTopRow: (isCreating: boolean) => void; - /** State object for the row creation mutation (isPending, isError, error) */ - createRowMutation: CreateRowMutationState; - /** State object for the row update mutation (isPending, isError, error) */ - updateRowMutation: CreateRowMutationState; - /** Callback when an existing row is updated (receives new value and original value) */ - handleUpdateRow: (value: string, originalValue: string) => void; - /** ID of the row currently in edit mode */ - editingRowId: RowId | null; - /** Setter function for which row is in edit mode */ - setEditingRowId: (id: RowId | null) => void; - /** Callback to exit draft mode without saving changes */ - exitDraftWithoutSave: () => void; - /** Column definitions for rendering cells in edit mode */ - columns?: TreeColumnDef[]; - /** Validation function for new row values (receives value and optional 'soft' or 'hard' mode; - * in 'hard' mode an exception is thrown on validation failure) */ - validate: (value: string, mode?: 'soft' | 'hard') => boolean; } /** @@ -70,111 +36,46 @@ interface NestedRowsProps { const NestedRows = ({ parentRow, parentRowValue, - isCreating = false, - onSaveNewChildRow = () => {}, - onCancelCreation = () => {}, childRowsData = [], depth = 1, - draftError = '', - setDraftError = () => {}, - creatingParentId = null, - setCreatingParentId = () => {}, - setIsCreatingTopRow, - createRowMutation, - updateRowMutation, - handleUpdateRow, - editingRowId, - setEditingRowId, - exitDraftWithoutSave, - columns = [], - validate, }: NestedRowsProps) => { + const { + creatingParentId, + editingRowId, + } = useTagListContext(); + if (!parentRow.getIsExpanded()) { return null; } const indent = Math.max(depth, 1); + const isCreating = creatingParentId === parentRow.original.id; return ( <> {isCreating && ( onSaveNewChildRow(value, parentRowValue)} - setIsCreatingTopRow={setIsCreatingTopRow} - exitDraftWithoutSave={onCancelCreation} - createRowMutation={createRowMutation} + parentRowValue={parentRowValue} indent={indent} - validate={validate} /> )} {childRowsData?.map(row => { const rowData = row.original || row; return ( - {editingRowId === `${row.original.id}:${String(row.original.value)}` ? ( + {editingRowId === getTreeRowEditId(row) ? ( handleUpdateRow(value, String(row.original.value))} - cancelEditRow={() => { - setEditingRowId(null); - exitDraftWithoutSave(); - }} - updateRowMutation={updateRowMutation} indent={indent} - validate={validate} row={row} /> ) : ( - - {row.getVisibleCells() - .map((cell, index) => { - const content = flexRender(cell.column.columnDef.cell, cell.getContext()); - const isFirstColumn = index === 0; - - return ( - - {isFirstColumn ? ( -
{content}
- ) : ( - content - )} - - ); - })} - + )} { - setCreatingParentId(null); - onCancelCreation(); - } - } - creatingParentId={creatingParentId} - setCreatingParentId={setCreatingParentId} depth={depth + 1} - draftError={draftError} - setDraftError={setDraftError} - setIsCreatingTopRow={setIsCreatingTopRow} - createRowMutation={createRowMutation} - updateRowMutation={updateRowMutation} - handleUpdateRow={handleUpdateRow} - editingRowId={editingRowId} - setEditingRowId={setEditingRowId} - exitDraftWithoutSave={exitDraftWithoutSave} - columns={columns} - validate={validate} />
); diff --git a/src/taxonomy/tree-table/SaveErrorAlert.tsx b/src/taxonomy/tree-table/SaveErrorAlert.tsx index 4476e06c79..05ed38a0cc 100644 --- a/src/taxonomy/tree-table/SaveErrorAlert.tsx +++ b/src/taxonomy/tree-table/SaveErrorAlert.tsx @@ -19,13 +19,8 @@ const SaveErrorAlert = ({ draftError, isError, isUpdateError }: SaveErrorAlertPr const [alertOpen, setAlertOpen] = React.useState(hasError); useEffect(() => { - if (hasError) { - setAlertOpen(true); - } - if (!hasError) { - setAlertOpen(false); - } - }, [hasError, isError, isUpdateError, draftError]); + setAlertOpen(hasError); + }, [hasError]); if (!alertOpen) { return null; } diff --git a/src/taxonomy/tree-table/TableBody.tsx b/src/taxonomy/tree-table/TableBody.tsx index 99fca02dd7..b816b0927e 100644 --- a/src/taxonomy/tree-table/TableBody.tsx +++ b/src/taxonomy/tree-table/TableBody.tsx @@ -1,6 +1,6 @@ import React from 'react'; import { useIntl } from '@edx/frontend-platform/i18n'; -import { flexRender } from '@tanstack/react-table'; +import { useTagListContext } from '@src/taxonomy/tag-list/TagListContext'; import { LoadingSpinner } from '@src/generic/Loading'; import NestedRows from './NestedRows'; @@ -8,54 +8,30 @@ import NestedRows from './NestedRows'; import messages from './messages'; import type { - CreateRowMutationState, - RowId, TreeColumnDef, TreeTable, } from './types'; import CreateRow from './CreateRow'; import EditRow from './EditRow'; +import DisplayRow from './DisplayRow'; +import { getTreeRowEditId } from './rowHelpers'; interface TableBodyProps { columns: TreeColumnDef[]; - isCreatingTopRow: boolean; - draftError: string; - setIsCreatingTopRow: (isCreating: boolean) => void; - exitDraftWithoutSave: () => void; - handleCreateRow: (value: string, parentRowValue?: string) => void; - creatingParentId: RowId | null; - setCreatingParentId: (id: RowId | null) => void; - setDraftError: (error: string) => void; - createRowMutation: CreateRowMutationState; - updateRowMutation: CreateRowMutationState; table: TreeTable; isLoading: boolean; - validate: (value: string, mode?: 'soft' | 'hard') => boolean; - handleUpdateRow: (value: string, originalValue: string) => void; - editingRowId: RowId | null; - setEditingRowId: (id: RowId | null) => void; } const TableBody = ({ columns, - isCreatingTopRow, - draftError, - handleCreateRow, - setIsCreatingTopRow, - exitDraftWithoutSave, - creatingParentId, - setCreatingParentId, - setDraftError, - createRowMutation, - updateRowMutation, table, isLoading, - validate, - handleUpdateRow, - editingRowId, - setEditingRowId, }: TableBodyProps) => { const intl = useIntl(); + const { + isCreatingTopTag, + editingRowId, + } = useTagListContext(); if (isLoading) { return ( @@ -79,69 +55,25 @@ const TableBody = ({ )} - {isCreatingTopRow && ( - + {isCreatingTopTag && ( + )} {table.getRowModel().rows.filter(row => row.depth === 0).map(row => ( - {editingRowId === `${row.original.id}:${String(row.original.value)}` ? ( + {editingRowId === getTreeRowEditId(row) ? ( handleUpdateRow(value, String(row.original.value))} - cancelEditRow={() => { - setEditingRowId(null); - exitDraftWithoutSave(); - }} - updateRowMutation={updateRowMutation} - validate={validate} row={row} /> ) : ( - - {row.getVisibleCells() - .map((cell) => ( - - {flexRender(cell.column.columnDef.cell, cell.getContext())} - - ))} - + )} { - setDraftError(''); - setCreatingParentId(null); - exitDraftWithoutSave(); - }} - creatingParentId={creatingParentId} - setCreatingParentId={setCreatingParentId} depth={1} - draftError={draftError} - createRowMutation={createRowMutation} - setDraftError={setDraftError} - setIsCreatingTopRow={setIsCreatingTopRow} - validate={validate} - updateRowMutation={updateRowMutation} - handleUpdateRow={handleUpdateRow} - editingRowId={editingRowId} - setEditingRowId={setEditingRowId} - exitDraftWithoutSave={exitDraftWithoutSave} - columns={columns} /> ))} diff --git a/src/taxonomy/tree-table/TableView.test.tsx b/src/taxonomy/tree-table/TableView.test.tsx index 960e83203c..5c5ca182e8 100644 --- a/src/taxonomy/tree-table/TableView.test.tsx +++ b/src/taxonomy/tree-table/TableView.test.tsx @@ -1,6 +1,7 @@ import React from 'react'; import { IntlProvider } from '@edx/frontend-platform/i18n'; import { fireEvent, render, screen } from '@testing-library/react'; +import { TagListContext } from '@src/taxonomy/tag-list/TagListContext'; import { TableView } from './TableView'; @@ -19,6 +20,27 @@ const wrapper = ({ children }: { children: React.ReactNode }) => ( {children} ); +const baseContext = () => ({ + isCreatingTopTag: false, + setIsCreatingTopTag: jest.fn(), + creatingParentId: null, + setCreatingParentId: jest.fn(), + editingRowId: null, + setEditingRowId: jest.fn(), + draftError: '', + setDraftError: jest.fn(), + hasOpenDraft: false, + canAddTag: true, + maxDepth: 3, + createTagMutation: { isPending: false, isError: false }, + updateTagMutation: { isPending: false, isError: false }, + handleCreateTag: jest.fn(), + handleUpdateTag: jest.fn(), + validate: jest.fn(() => true), + startDraftMode: jest.fn(), + exitDraftWithoutSave: jest.fn(), +}); + const baseProps = () => ({ treeData: [{ id: 1, value: 'root' }], columns: [{ accessorKey: 'value', header: 'Tag name', cell: (info: any) => info.getValue() }], @@ -26,31 +48,29 @@ const baseProps = () => ({ pagination: { pageIndex: 0, pageSize: 10 }, handlePaginationChange: jest.fn(), isLoading: false, - isCreatingTopRow: false, - draftError: '', - createRowMutation: { isPending: false, isError: false }, - updateRowMutation: { isPending: false, isError: false }, toast: { show: false, message: '', variant: 'success' }, setToast: jest.fn(), - setIsCreatingTopRow: jest.fn(), - exitDraftWithoutSave: jest.fn(), - handleCreateRow: jest.fn(), - creatingParentId: null, - setCreatingParentId: jest.fn(), - setDraftError: jest.fn(), - validate: jest.fn(() => true), - handleUpdateRow: jest.fn(), - editingRowId: null, - setEditingRowId: jest.fn(), }); +const renderTableView = (props: any, contextOverrides: Record = {}) => { + const context = { ...baseContext(), ...contextOverrides }; + return render( + + + , + { wrapper }, + ); +}; + describe('TableView', () => { it('shows and dismisses save error banner', () => { const props = baseProps(); - props.createRowMutation = { isPending: false, isError: true }; - props.draftError = 'Request failed with status code 500'; + const context = { + createTagMutation: { isPending: false, isError: true }, + draftError: 'Request failed with status code 500', + }; - render(, { wrapper }); + renderTableView(props, context); expect(screen.getByText('Error saving changes')).toBeInTheDocument(); fireEvent.click(screen.getByRole('button', { name: /dismiss/i })); @@ -59,14 +79,14 @@ describe('TableView', () => { it('keeps pagination hidden by default even when multiple pages are reported', () => { const props = baseProps(); - render(, { wrapper }); + renderTableView(props); expect(screen.queryByRole('navigation', { name: /table pagination/i })).not.toBeInTheDocument(); }); it('renders pagination and updates page selection when explicitly enabled', () => { const props = baseProps(); - render(, { wrapper }); + renderTableView({ ...props, enablePagination: true }); expect(screen.getByText('Page 1 of 3')).toBeInTheDocument(); fireEvent.click(screen.getByRole('button', { name: /^page 2$/i })); @@ -76,7 +96,7 @@ describe('TableView', () => { it('hides pagination when there is only one page', () => { const props = baseProps(); props.pageCount = 1; - render(, { wrapper }); + renderTableView(props); expect(screen.queryByRole('navigation', { name: /table pagination/i })).not.toBeInTheDocument(); }); @@ -85,7 +105,7 @@ describe('TableView', () => { const props = baseProps(); props.toast = { show: true, message: 'created', variant: 'success' }; - render(, { wrapper }); + renderTableView(props); fireEvent.click(screen.getByRole('button', { name: /close/i })); expect(props.setToast).toHaveBeenCalled(); diff --git a/src/taxonomy/tree-table/TableView.tsx b/src/taxonomy/tree-table/TableView.tsx index 92ad3bd5fe..19e8218738 100644 --- a/src/taxonomy/tree-table/TableView.tsx +++ b/src/taxonomy/tree-table/TableView.tsx @@ -19,11 +19,10 @@ import { import { ArrowDropUpDown } from '@openedx/paragon/icons'; import { useIntl } from '@edx/frontend-platform/i18n'; +import { useTagListContext } from '@src/taxonomy/tag-list/TagListContext'; import TableBody from './TableBody'; import './TableView.scss'; import type { - CreateRowMutationState, - RowId, ToastState, TreeColumnDef, TreeRowData, @@ -39,22 +38,8 @@ interface TableViewProps { pagination: PaginationState; handlePaginationChange: OnChangeFn; isLoading: boolean; - isCreatingTopRow: boolean; - draftError: string; - createRowMutation: CreateRowMutationState; - updateRowMutation: CreateRowMutationState; toast: ToastState; setToast: React.Dispatch>; - setIsCreatingTopRow: (isCreating: boolean) => void; - exitDraftWithoutSave: () => void; - handleCreateRow: (value: string, parentRowValue?: string) => void; - creatingParentId: RowId | null; - setCreatingParentId: (id: RowId | null) => void; - setDraftError: (error: string) => void; - validate: (value: string, mode?: 'soft' | 'hard') => boolean; - handleUpdateRow: (value: string, originalValue: string) => void; - editingRowId: RowId | null; - setEditingRowId: (id: RowId | null) => void; } const TableView = ({ @@ -65,24 +50,11 @@ const TableView = ({ pagination, handlePaginationChange, isLoading, - isCreatingTopRow, - draftError, - createRowMutation, - updateRowMutation, - handleCreateRow, toast, setToast, - setIsCreatingTopRow, - exitDraftWithoutSave, - creatingParentId, - setCreatingParentId, - setDraftError, - validate, - handleUpdateRow, - editingRowId, - setEditingRowId, }: TableViewProps) => { const intl = useIntl(); + const { draftError, createTagMutation, updateTagMutation } = useTagListContext(); const table = useReactTable({ data: treeData, @@ -100,8 +72,8 @@ const TableView = ({ const currentPageIndex = table.getState().pagination.pageIndex + 1; - const { isError } = createRowMutation; - const { isError: isUpdateError } = updateRowMutation; + const { isError } = createTagMutation; + const { isError: isUpdateError } = updateTagMutation; return ( <> @@ -149,22 +121,8 @@ const TableView = ({ diff --git a/src/taxonomy/tree-table/rowHelpers.ts b/src/taxonomy/tree-table/rowHelpers.ts new file mode 100644 index 0000000000..d2cb96dfd6 --- /dev/null +++ b/src/taxonomy/tree-table/rowHelpers.ts @@ -0,0 +1,5 @@ +import type { TreeRow } from './types'; + +const getTreeRowEditId = (row: TreeRow): string => `${row.original.id}:${String(row.original.value)}`; + +export { getTreeRowEditId }; \ No newline at end of file From 5c9f72bff87b3b907a425d74437e0ab1e1f9b7fc Mon Sep 17 00:00:00 2001 From: Jesper Hodge Date: Tue, 14 Apr 2026 16:29:06 -0400 Subject: [PATCH 7/7] fix: cleanup --- src/taxonomy/tag-list/TagListContext.tsx | 21 +++- src/taxonomy/tag-list/TagListTable.tsx | 128 +++++++---------------- src/taxonomy/tag-list/editActionUtils.ts | 2 +- src/taxonomy/tree-table/DisplayRow.tsx | 2 +- src/taxonomy/tree-table/TableBody.tsx | 21 +--- src/taxonomy/tree-table/TableView.tsx | 60 ++++------- src/taxonomy/tree-table/rowHelpers.ts | 2 +- 7 files changed, 90 insertions(+), 146 deletions(-) diff --git a/src/taxonomy/tag-list/TagListContext.tsx b/src/taxonomy/tag-list/TagListContext.tsx index 4578e394a1..d1f99efa51 100644 --- a/src/taxonomy/tag-list/TagListContext.tsx +++ b/src/taxonomy/tag-list/TagListContext.tsx @@ -1,6 +1,14 @@ import React, { createContext, useContext } from 'react'; -import type { RowId, CreateRowMutationState } from '@src/taxonomy/tree-table/types'; +import type { + RowId, + CreateRowMutationState, + TreeRowData, + TreeColumnDef, + ToastState, + TreeTable, +} from '@src/taxonomy/tree-table/types'; +import { OnChangeFn, PaginationState } from '@tanstack/react-table'; interface TagListContextValue { isCreatingTopTag: boolean; @@ -21,6 +29,17 @@ interface TagListContextValue { validate: (value: string, mode?: 'soft' | 'hard') => boolean; startDraftMode: () => void; exitDraftWithoutSave: () => void; + treeData: TreeRowData[]; + columns: TreeColumnDef[]; + pageCount: number; + enablePagination?: boolean; + pagination: PaginationState; + handlePaginationChange: OnChangeFn; + isLoading: boolean; + toast: ToastState; + setToast: React.Dispatch>; + table: TreeTable | null; + setTable: React.Dispatch>; } const TagListContext = createContext(null); diff --git a/src/taxonomy/tag-list/TagListTable.tsx b/src/taxonomy/tag-list/TagListTable.tsx index dcb90f81ce..19d0c16b7e 100644 --- a/src/taxonomy/tag-list/TagListTable.tsx +++ b/src/taxonomy/tag-list/TagListTable.tsx @@ -6,12 +6,12 @@ import React, { import type { PaginationState } from '@tanstack/react-table'; import { TableView } from '@src/taxonomy/tree-table'; import { useTagListData, useCreateTag, useUpdateTag } from '@src/taxonomy/data/apiHooks'; -import { TagTree } from './tagTree'; import type { RowId, TreeRowData, - ToastState, -} from '../tree-table/types'; + TreeTable, +} from '@src/taxonomy/tree-table/types'; +import { TagTree } from './tagTree'; import { TABLE_MODES, } from './constants'; @@ -27,41 +27,6 @@ interface TagListTableProps { // TODO: Fix and enable pagination on backend and frontend.For now, disable pagination by showing all tags on one page. const DISABLE_PAGINATION = true; -interface TagListTableContentProps { - treeData: TreeRowData[]; - pageCount: number; - pagination: PaginationState; - handlePaginationChange: React.Dispatch>; - isLoading: boolean; - toast: ToastState; - setToast: React.Dispatch>; -} - -const TagListTableContent = ({ - treeData, - pageCount, - pagination, - handlePaginationChange, - isLoading, - toast, - setToast, -}: TagListTableContentProps) => { - const columns = useTagColumns(); - - return ( - - ); -}; - const TagListTable = ({ taxonomyId, maxDepth }: TagListTableProps) => { // The table has a VIEW, DRAFT, and a PREVIEW mode. It starts in VIEW mode. // It switches to DRAFT mode when a user edits or creates a tag. @@ -84,6 +49,8 @@ const TagListTable = ({ taxonomyId, maxDepth }: TagListTableProps) => { const [tagTree, setTagTree] = useState(null); const [isCreatingTopTag, setIsCreatingTopTag] = useState(false); const [draftError, setDraftError] = useState(''); + const [table, setTable] = useState(null); + const treeData = (tagTree?.getAllAsDeepCopy() || []) as unknown as TreeRowData[]; const hasOpenDraft = isCreatingTopTag || creatingParentId !== null || editingRowId !== null; @@ -131,48 +98,39 @@ const TagListTable = ({ taxonomyId, maxDepth }: TagListTableProps) => { setEditingRowId, }); - const contextValue = useMemo( - () => ({ - isCreatingTopTag, - setIsCreatingTopTag, - creatingParentId, - setCreatingParentId, - editingRowId, - setEditingRowId, - draftError, - setDraftError, - hasOpenDraft, - canAddTag: tagList?.canAddTag !== false, - maxDepth, - createTagMutation, - updateTagMutation, - handleCreateTag, - handleUpdateTag, - validate, - startDraftMode: enterDraftMode, - exitDraftWithoutSave, - }), - [ - isCreatingTopTag, - setIsCreatingTopTag, - creatingParentId, - setCreatingParentId, - editingRowId, - setEditingRowId, - draftError, - setDraftError, - hasOpenDraft, - tagList?.canAddTag, - maxDepth, - createTagMutation, - updateTagMutation, - handleCreateTag, - handleUpdateTag, - validate, - enterDraftMode, - exitDraftWithoutSave, - ], - ); + const columns = useTagColumns(); + + // eslint-disable-next-line react/jsx-no-constructed-context-values + const contextValue = { + isCreatingTopTag, + setIsCreatingTopTag, + creatingParentId, + setCreatingParentId, + editingRowId, + setEditingRowId, + draftError, + setDraftError, + hasOpenDraft, + canAddTag: tagList?.canAddTag !== false, + maxDepth, + createTagMutation, + updateTagMutation, + handleCreateTag, + handleUpdateTag, + validate, + startDraftMode: enterDraftMode, + exitDraftWithoutSave, + treeData, + pageCount, + pagination, + handlePaginationChange, + isLoading, + toast, + setToast, + columns, + table, + setTable, + }; // RELOAD DATA IN VIEW MODE useEffect(() => { @@ -188,15 +146,7 @@ const TagListTable = ({ taxonomyId, maxDepth }: TagListTableProps) => { return ( - + ); }; diff --git a/src/taxonomy/tag-list/editActionUtils.ts b/src/taxonomy/tag-list/editActionUtils.ts index 05e491fdde..2e4783521a 100644 --- a/src/taxonomy/tag-list/editActionUtils.ts +++ b/src/taxonomy/tag-list/editActionUtils.ts @@ -78,4 +78,4 @@ export { formatTagRequestError, renameTagInTree, addTagToTree, -}; \ No newline at end of file +}; diff --git a/src/taxonomy/tree-table/DisplayRow.tsx b/src/taxonomy/tree-table/DisplayRow.tsx index 68bdc5d23f..005acfa798 100644 --- a/src/taxonomy/tree-table/DisplayRow.tsx +++ b/src/taxonomy/tree-table/DisplayRow.tsx @@ -31,4 +31,4 @@ const DisplayRow = ({ row, indent = 0 }: DisplayRowProps) => ( ); -export default DisplayRow; \ No newline at end of file +export default DisplayRow; diff --git a/src/taxonomy/tree-table/TableBody.tsx b/src/taxonomy/tree-table/TableBody.tsx index b816b0927e..f90c57ac2a 100644 --- a/src/taxonomy/tree-table/TableBody.tsx +++ b/src/taxonomy/tree-table/TableBody.tsx @@ -7,33 +7,22 @@ import NestedRows from './NestedRows'; import messages from './messages'; -import type { - TreeColumnDef, - TreeTable, -} from './types'; import CreateRow from './CreateRow'; import EditRow from './EditRow'; import DisplayRow from './DisplayRow'; import { getTreeRowEditId } from './rowHelpers'; -interface TableBodyProps { - columns: TreeColumnDef[]; - table: TreeTable; - isLoading: boolean; -} - -const TableBody = ({ - columns, - table, - isLoading, -}: TableBodyProps) => { +const TableBody = () => { const intl = useIntl(); const { isCreatingTopTag, editingRowId, + columns, + table, + isLoading, } = useTagListContext(); - if (isLoading) { + if (!table || isLoading) { return ( diff --git a/src/taxonomy/tree-table/TableView.tsx b/src/taxonomy/tree-table/TableView.tsx index 19e8218738..b09230a12f 100644 --- a/src/taxonomy/tree-table/TableView.tsx +++ b/src/taxonomy/tree-table/TableView.tsx @@ -1,4 +1,4 @@ -import React from 'react'; +import React, { useEffect } from 'react'; import { Button, Toast, @@ -13,8 +13,6 @@ import { getCoreRowModel, getExpandedRowModel, flexRender, - type OnChangeFn, - type PaginationState, } from '@tanstack/react-table'; import { ArrowDropUpDown } from '@openedx/paragon/icons'; @@ -22,39 +20,25 @@ import { useIntl } from '@edx/frontend-platform/i18n'; import { useTagListContext } from '@src/taxonomy/tag-list/TagListContext'; import TableBody from './TableBody'; import './TableView.scss'; -import type { - ToastState, - TreeColumnDef, - TreeRowData, -} from './types'; import messages from './messages'; import SaveErrorAlert from './SaveErrorAlert'; -interface TableViewProps { - treeData: TreeRowData[]; - columns: TreeColumnDef[]; - pageCount: number; - enablePagination?: boolean; - pagination: PaginationState; - handlePaginationChange: OnChangeFn; - isLoading: boolean; - toast: ToastState; - setToast: React.Dispatch>; -} - -const TableView = ({ - treeData, - columns, - pageCount, - enablePagination = false, - pagination, - handlePaginationChange, - isLoading, - toast, - setToast, -}: TableViewProps) => { +const TableView = () => { const intl = useIntl(); - const { draftError, createTagMutation, updateTagMutation } = useTagListContext(); + const { + draftError, + createTagMutation, + updateTagMutation, + treeData, + pageCount, + pagination, + handlePaginationChange, + toast, + setToast, + columns, + enablePagination, + setTable, + } = useTagListContext(); const table = useReactTable({ data: treeData, @@ -70,6 +54,12 @@ const TableView = ({ getSubRows: (row) => row?.subRows || undefined, }); + useEffect(() => { + if (table) { + setTable(table); + } + }, [table, setTable]); + const currentPageIndex = table.getState().pagination.pageIndex + 1; const { isError } = createTagMutation; @@ -119,11 +109,7 @@ const TableView = ({ ))} - + diff --git a/src/taxonomy/tree-table/rowHelpers.ts b/src/taxonomy/tree-table/rowHelpers.ts index d2cb96dfd6..4938798581 100644 --- a/src/taxonomy/tree-table/rowHelpers.ts +++ b/src/taxonomy/tree-table/rowHelpers.ts @@ -2,4 +2,4 @@ import type { TreeRow } from './types'; const getTreeRowEditId = (row: TreeRow): string => `${row.original.id}:${String(row.original.value)}`; -export { getTreeRowEditId }; \ No newline at end of file +export { getTreeRowEditId };