From 958fe368a51d530777f840b7bedd8e723e42f16e Mon Sep 17 00:00:00 2001 From: Jesper Hodge Date: Mon, 20 Apr 2026 14:04:35 -0400 Subject: [PATCH 01/16] feat: add delete menu option --- src/taxonomy/tag-list/TagListTable.test.jsx | 38 +++++++++++++++ src/taxonomy/tag-list/messages.ts | 8 +++ src/taxonomy/tag-list/tagColumns.tsx | 54 ++++++++++++++++----- 3 files changed, 89 insertions(+), 11 deletions(-) diff --git a/src/taxonomy/tag-list/TagListTable.test.jsx b/src/taxonomy/tag-list/TagListTable.test.jsx index d9022c84fb..c93752fe3a 100644 --- a/src/taxonomy/tag-list/TagListTable.test.jsx +++ b/src/taxonomy/tag-list/TagListTable.test.jsx @@ -991,6 +991,44 @@ describe('', () => { expect(within(grandchildTagRow).getByText('Add Subtag')).toHaveAttribute('aria-disabled', 'true'); }); }); + + describe('Delete Tags', () => { + const tagDepthScenarios = [ + { + description: 'Delete a top-level tag', + tagName: 'root tag 1', + }, + { description: 'Delete a sub-tag', tagName: 'the child tag' }, + { description: 'Delete a grandchild tag', tagName: 'the grandchild tag' }, + ]; + + tagDepthScenarios.forEach(({ description, tagName }) => { + describe(description, () => { + beforeEach(async () => { + axiosMock.resetHistory(); + }); + + it('should disable delete action and show tooltip if tag includes `can_delete: false`', async () => { + axiosMock.reset(); + axiosMock.onGet(rootTagsListUrl).reply(200, mockTagResponseDisallowingEdits); + axiosMock.onGet(subTagsUrl).reply(200, subTagsResponse); + cleanup(); + ({ axiosMock } = initializeMocks({ user: adminUser })); + axiosMock.onGet(rootTagsListUrl).reply(200, mockTagResponseDisallowingEdits); + axiosMock.onGet(subTagsUrl).reply(200, subTagsResponse); + renderTagListTable(); + await waitForRootTag(); + + openActionsMenuForTag(tagName); + const deleteButton = screen.getByRole('button', { name: /Delete/i }); + expect(deleteButton).toBeInTheDocument(); + expect(deleteButton).toHaveAttribute('aria-disabled', 'true'); + fireEvent.mouseOver(deleteButton); + expect(screen.getByText(/This tag does not allow deletion/i)).toBeInTheDocument(); + }); + }); + }); + }); }); // These async creation flows are intentionally isolated because they pass individually diff --git a/src/taxonomy/tag-list/messages.ts b/src/taxonomy/tag-list/messages.ts index 69e74221ea..bd3d2e7be7 100644 --- a/src/taxonomy/tag-list/messages.ts +++ b/src/taxonomy/tag-list/messages.ts @@ -65,6 +65,14 @@ const messages = defineMessages({ id: 'course-authoring.tag-list.rename-tag', defaultMessage: 'Rename', }, + deleteTag: { + id: 'course-authoring.tag-list.delete-tag', + defaultMessage: 'Delete', + }, + deleteTagDisabledTooltip: { + id: 'course-authoring.tag-list.delete-tag-disabled-tooltip', + defaultMessage: 'This tag does not allow deletion', + }, }); export default messages; diff --git a/src/taxonomy/tag-list/tagColumns.tsx b/src/taxonomy/tag-list/tagColumns.tsx index 7b8874331f..15dab666b0 100644 --- a/src/taxonomy/tag-list/tagColumns.tsx +++ b/src/taxonomy/tag-list/tagColumns.tsx @@ -3,6 +3,8 @@ import { IconButton, IconButtonWithTooltip, Dropdown, + OverlayTrigger, + Tooltip, } from '@openedx/paragon'; import { AddCircle, @@ -25,24 +27,21 @@ import { getTagListRowData } from './utils'; const EDITABLE_COLUMNS = ['value']; interface GetColumnsArgs { - setIsCreatingTopTag: (isCreating: boolean) => void; + setIsCreatingTopRow: (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; setDraftError: (error: string) => void; - setIsCreatingTopTag: (isCreating: boolean) => void; + setIsCreatingTopRow: (isCreating: boolean) => void; setEditingRowId: (id: RowId | null) => void; setActiveActionMenuRowId: (id: RowId | null) => void; hasOpenDraft: boolean; @@ -53,7 +52,7 @@ interface ActionsHeaderProps { const ActionsHeader = ({ onStartDraft, setDraftError, - setIsCreatingTopTag, + setIsCreatingTopRow, setEditingRowId, setActiveActionMenuRowId, hasOpenDraft, @@ -72,7 +71,7 @@ const ActionsHeader = ({ onClick={() => { onStartDraft(); setDraftError(''); - setIsCreatingTopTag(true); + setIsCreatingTopRow(true); setEditingRowId(null); setActiveActionMenuRowId(null); }} @@ -90,6 +89,9 @@ interface ActionsMenuProps { editTag: () => void; disableEditTag: boolean; reachedMaxDepth: (row: Row) => boolean; + deleteTag: () => void; + disableDeleteTag: boolean; + row: Row; } @@ -101,9 +103,20 @@ const ActionsMenu = ({ editTag, disableEditTag, reachedMaxDepth, + deleteTag, + disableDeleteTag, }: ActionsMenuProps) => { const intl = useIntl(); + const deleteTagMenuItem = ( + + {intl.formatMessage(messages.deleteTag)} + + ) + return ( {intl.formatMessage(messages.renameTag)} + {disableDeleteTag ? ( + + {intl.formatMessage(messages.deleteTagDisabledTooltip)} + + } + > + {deleteTagMenuItem} + + ) : deleteTagMenuItem} ); }; function getColumns({ - setIsCreatingTopTag, + setIsCreatingTopRow, setCreatingParentId, setEditingRowId, onStartDraft, @@ -175,7 +200,7 @@ function getColumns({ { onStartDraft(); setDraftError(''); setCreatingParentId(rowData.id); setEditingRowId(null); - setIsCreatingTopTag(false); + setIsCreatingTopRow(false); setActiveActionMenuRowId(null); row.toggleExpanded(true); }; @@ -208,10 +234,14 @@ function getColumns({ setDraftError(''); setEditingRowId(`${rowData.id}:${rowData.value}`); setCreatingParentId(null); - setIsCreatingTopTag(false); + setIsCreatingTopRow(false); setActiveActionMenuRowId(null); }; + const deleteTag = () => { + // todo: implement + }; + return (
); From a4d2eb6b07bfcc28567eac8b59eefbafe51fca16 Mon Sep 17 00:00:00 2001 From: Jesper Hodge Date: Mon, 20 Apr 2026 14:05:52 -0400 Subject: [PATCH 02/16] refactor: introduce tree table context --- src/taxonomy/tag-list/TagListTable.test.jsx | 4 +- src/taxonomy/tag-list/TagListTable.tsx | 131 +++++++++--------- .../tag-list/createTreeTableContextValue.ts | 21 +++ src/taxonomy/tree-table/TableView.test.tsx | 46 +++--- src/taxonomy/tree-table/TableView.tsx | 80 ++++------- src/taxonomy/tree-table/TreeTableContext.tsx | 61 ++++++++ src/taxonomy/tree-table/index.ts | 1 + 7 files changed, 205 insertions(+), 139 deletions(-) create mode 100644 src/taxonomy/tag-list/createTreeTableContextValue.ts create mode 100644 src/taxonomy/tree-table/TreeTableContext.tsx diff --git a/src/taxonomy/tag-list/TagListTable.test.jsx b/src/taxonomy/tag-list/TagListTable.test.jsx index c93752fe3a..7d0b11c184 100644 --- a/src/taxonomy/tag-list/TagListTable.test.jsx +++ b/src/taxonomy/tag-list/TagListTable.test.jsx @@ -1248,8 +1248,8 @@ describe(' pagination transition behavior', () => { handleUpdateTag: jest.fn(), validate: jest.fn(() => true), }); - jest.spyOn(treeTableModule, 'TableView').mockImplementation((props) => { - tableViewProps = props; + jest.spyOn(treeTableModule, 'TableView').mockImplementation(() => { + tableViewProps = React.useContext(treeTableModule.TreeTableContext); return
; }); }); diff --git a/src/taxonomy/tag-list/TagListTable.tsx b/src/taxonomy/tag-list/TagListTable.tsx index c2d00319d8..06718fe5cf 100644 --- a/src/taxonomy/tag-list/TagListTable.tsx +++ b/src/taxonomy/tag-list/TagListTable.tsx @@ -4,19 +4,18 @@ import React, { useEffect, } from '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 { TableView, TreeTableContext } from '@src/taxonomy/tree-table'; import type { RowId, - TreeColumnDef, TreeRowData, } from '../tree-table/types'; +import { TagTree } from './tagTree'; import { TABLE_MODES, } from './constants'; -import { getColumns } from './tagColumns'; import { useTableModes, useEditActions } from './hooks'; +import { createTreeTableContextValue } from './createTreeTableContextValue'; interface TagListTableProps { taxonomyId: number; @@ -102,81 +101,83 @@ const TagListTable = ({ taxonomyId, maxDepth }: TagListTableProps) => { setEditingRowId, }); - const columns = useMemo( - () => - getColumns({ - setIsCreatingTopTag, - setCreatingParentId, - handleUpdateTag, - setEditingRowId, - onStartDraft: enterDraftMode, - setActiveActionMenuRowId, - hasOpenDraft, - canAddTag: tagList?.canAddTag !== false, - draftError, - setDraftError, - isSavingDraft: createTagMutation.isPending, - maxDepth, - }), + // RELOAD DATA IN VIEW MODE + useEffect(() => { + // Get row data in VIEW mode. Otherwise keep current data to avoid disrupting + // users while they edit or create a tag. + if (tableMode === TABLE_MODES.VIEW && tagList?.results) { + const tree = new TagTree(tagList?.results); + if (tree) { + setTagTree(tree); + } + } + }, [tagList?.results, tableMode]); + + const contextValue = useMemo( + () => createTreeTableContextValue({ + treeData, + pageCount, + pagination, + handlePaginationChange, + isLoading, + isCreatingTopRow: isCreatingTopTag, + draftError, + createRowMutation: createTagMutation, + updateRowMutation: updateTagMutation, + toast, + setToast, + setIsCreatingTopRow: setIsCreatingTopTag, + exitDraftWithoutSave, + handleCreateRow: handleCreateTag, + creatingParentId, + setCreatingParentId, + setDraftError, + validate, + handleUpdateRow: handleUpdateTag, + editingRowId, + setEditingRowId, + onStartDraft: enterDraftMode, + setActiveActionMenuRowId, + hasOpenDraft, + canAddTag: tagList?.canAddTag !== false, + maxDepth, + }), [ + treeData, + pageCount, + pagination, + handlePaginationChange, + isLoading, isCreatingTopTag, - tableMode, - activeActionMenuRowId, - hasOpenDraft, - creatingParentId, - tagList?.canAddTag, draftError, - createTagMutation.isPending, - maxDepth, + createTagMutation, + updateTagMutation, + toast, + setToast, setIsCreatingTopTag, + exitDraftWithoutSave, + handleCreateTag, + creatingParentId, setCreatingParentId, + setDraftError, + validate, handleUpdateTag, + editingRowId, setEditingRowId, enterDraftMode, setActiveActionMenuRowId, - setDraftError, + hasOpenDraft, + tagList?.canAddTag, + maxDepth, ], ); - // RELOAD DATA IN VIEW MODE - useEffect(() => { - // Get row data in VIEW mode. Otherwise keep current data to avoid disrupting - // users while they edit or create a tag. - if (tableMode === TABLE_MODES.VIEW && tagList?.results) { - const tree = new TagTree(tagList?.results); - if (tree) { - setTagTree(tree); - } - } - }, [tagList?.results, tableMode]); + return ( - + + + ); }; diff --git a/src/taxonomy/tag-list/createTreeTableContextValue.ts b/src/taxonomy/tag-list/createTreeTableContextValue.ts new file mode 100644 index 0000000000..a376eed29d --- /dev/null +++ b/src/taxonomy/tag-list/createTreeTableContextValue.ts @@ -0,0 +1,21 @@ +import type { RowId } from '@src/taxonomy/tree-table/types'; +import type { TreeTableContextValue } from '@src/taxonomy/tree-table/TreeTableContext'; + +import { getColumns } from './tagColumns'; + +interface CreateTagListTreeTableContextValueArgs extends Omit { + onStartDraft: () => void; + setActiveActionMenuRowId: (id: RowId | null) => void; + hasOpenDraft: boolean; + canAddTag: boolean; + maxDepth: number; +} + +const createTreeTableContextValue = ( + args: CreateTagListTreeTableContextValueArgs, +): TreeTableContextValue => ({ + ...args, + columns: getColumns(args), +}); + +export { createTreeTableContextValue }; \ No newline at end of file diff --git a/src/taxonomy/tree-table/TableView.test.tsx b/src/taxonomy/tree-table/TableView.test.tsx index eca55131b7..1157713339 100644 --- a/src/taxonomy/tree-table/TableView.test.tsx +++ b/src/taxonomy/tree-table/TableView.test.tsx @@ -2,6 +2,7 @@ import React from 'react'; import { IntlProvider } from '@edx/frontend-platform/i18n'; import { fireEvent, render, screen } from '@testing-library/react'; +import { TreeTableContext } from './TreeTableContext'; import { TableView } from './TableView'; jest.mock('./TableBody', () => { @@ -19,7 +20,7 @@ const wrapper = ({ children }: { children: React.ReactNode; }) => ( {children} ); -const baseProps = () => ({ +const baseContextValue = () => ({ treeData: [{ id: 1, value: 'root' }], columns: [{ accessorKey: 'value', header: 'Tag name', cell: (info: any) => info.getValue() }], pageCount: 3, @@ -44,13 +45,23 @@ const baseProps = () => ({ setEditingRowId: jest.fn(), }); +const renderTableView = ( + contextValue = baseContextValue(), + props: React.ComponentProps = {}, +) => 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 contextValue = baseContextValue(); + contextValue.createRowMutation = { isPending: false, isError: true }; + contextValue.draftError = 'Request failed with status code 500'; - render(, { wrapper }); + renderTableView(contextValue); expect(screen.getByText('Error saving changes')).toBeInTheDocument(); fireEvent.click(screen.getByRole('button', { name: /dismiss/i })); @@ -58,38 +69,37 @@ describe('TableView', () => { }); it('keeps pagination hidden by default even when multiple pages are reported', () => { - const props = baseProps(); - render(, { wrapper }); + renderTableView(); 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 }); + const contextValue = baseContextValue(); + renderTableView(contextValue, { enablePagination: true }); expect(screen.getByText('Page 1 of 3')).toBeInTheDocument(); fireEvent.click(screen.getByRole('button', { name: /^page 2$/i })); - expect(props.handlePaginationChange).toHaveBeenCalled(); + expect(contextValue.handlePaginationChange).toHaveBeenCalled(); }); it('hides pagination when there is only one page', () => { - const props = baseProps(); - props.pageCount = 1; - render(, { wrapper }); + const contextValue = baseContextValue(); + contextValue.pageCount = 1; + renderTableView(contextValue); expect(screen.queryByRole('navigation', { name: /table pagination/i })).not.toBeInTheDocument(); }); it('closes toast by setting show to false', () => { - const props = baseProps(); - props.toast = { show: true, message: 'created', variant: 'success' }; + const contextValue = baseContextValue(); + contextValue.toast = { show: true, message: 'created', variant: 'success' }; - render(, { wrapper }); + renderTableView(contextValue); fireEvent.click(screen.getByRole('button', { name: /close/i })); - expect(props.setToast).toHaveBeenCalled(); - const updater = props.setToast.mock.calls[0][0]; + expect(contextValue.setToast).toHaveBeenCalled(); + const updater = contextValue.setToast.mock.calls[0][0]; expect(updater({ show: true, message: 'created', variant: 'success' })).toEqual({ show: false, message: 'created', diff --git a/src/taxonomy/tree-table/TableView.tsx b/src/taxonomy/tree-table/TableView.tsx index 707792f452..4e53fe3c3c 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, { useContext } from 'react'; import { Button, Toast, @@ -13,76 +13,48 @@ import { getCoreRowModel, getExpandedRowModel, flexRender, - type OnChangeFn, - type PaginationState, } from '@tanstack/react-table'; import { ArrowDropUpDown } from '@openedx/paragon/icons'; import { useIntl } from '@edx/frontend-platform/i18n'; import TableBody from './TableBody'; import './TableView.scss'; -import type { - CreateRowMutationState, - RowId, - ToastState, - TreeColumnDef, - TreeRowData, -} from './types'; import messages from './messages'; import SaveErrorAlert from './SaveErrorAlert'; +import { TreeTableContext } from './TreeTableContext'; interface TableViewProps { - treeData: TreeRowData[]; - columns: TreeColumnDef[]; - pageCount: number; enablePagination?: boolean; - 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 = ({ - treeData, - columns, - pageCount, enablePagination = false, - pagination, - handlePaginationChange, - isLoading, - isCreatingTopRow, - draftError, - createRowMutation, - updateRowMutation, - handleCreateRow, - toast, - setToast, - setIsCreatingTopRow, - exitDraftWithoutSave, - creatingParentId, - setCreatingParentId, - setDraftError, - validate, - handleUpdateRow, - editingRowId, - setEditingRowId, }: TableViewProps) => { const intl = useIntl(); + const { + treeData, + columns, + pageCount, + pagination, + handlePaginationChange, + isLoading, + isCreatingTopRow, + draftError, + createRowMutation, + updateRowMutation, + handleCreateRow, + toast, + setToast, + setIsCreatingTopRow, + exitDraftWithoutSave, + creatingParentId, + setCreatingParentId, + setDraftError, + validate, + handleUpdateRow, + editingRowId, + setEditingRowId, + } = useContext(TreeTableContext); const table = useReactTable({ data: treeData, diff --git a/src/taxonomy/tree-table/TreeTableContext.tsx b/src/taxonomy/tree-table/TreeTableContext.tsx new file mode 100644 index 0000000000..b2ad9c2f3f --- /dev/null +++ b/src/taxonomy/tree-table/TreeTableContext.tsx @@ -0,0 +1,61 @@ +import { createContext } from 'react'; +import type { Dispatch, SetStateAction } from 'react'; +import type { OnChangeFn, PaginationState } from '@tanstack/react-table'; + +import type { + CreateRowMutationState, + RowId, + ToastState, + TreeColumnDef, + TreeRowData, +} from './types'; + +export interface TreeTableContextValue { + treeData: TreeRowData[]; + columns: TreeColumnDef[]; + pageCount: number; + pagination: PaginationState; + handlePaginationChange: OnChangeFn; + isLoading: boolean; + isCreatingTopRow: boolean; + draftError: string; + createRowMutation: CreateRowMutationState; + updateRowMutation: CreateRowMutationState; + toast: ToastState; + setToast: 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; +} + +export const TreeTableContext = createContext({ + treeData: [], + columns: [], + pageCount: -1, + pagination: { pageIndex: 0, pageSize: 0 }, + handlePaginationChange: () => {}, + isLoading: false, + isCreatingTopRow: false, + draftError: '', + createRowMutation: {}, + updateRowMutation: {}, + toast: { show: false, message: '', variant: 'success' }, + setToast: () => {}, + setIsCreatingTopRow: () => {}, + exitDraftWithoutSave: () => {}, + handleCreateRow: () => {}, + creatingParentId: null, + setCreatingParentId: () => {}, + setDraftError: () => {}, + validate: () => true, + handleUpdateRow: () => {}, + editingRowId: null, + setEditingRowId: () => {}, +}); \ No newline at end of file diff --git a/src/taxonomy/tree-table/index.ts b/src/taxonomy/tree-table/index.ts index eca12d684c..d44d8f393f 100644 --- a/src/taxonomy/tree-table/index.ts +++ b/src/taxonomy/tree-table/index.ts @@ -1,2 +1,3 @@ export { EditableCell } from './EditableCell'; +export { TreeTableContext } from './TreeTableContext'; export { TableView } from './TableView'; From 00efee1adb42e98072017db91416aeff115557a5 Mon Sep 17 00:00:00 2001 From: Jesper Hodge Date: Mon, 20 Apr 2026 14:14:51 -0400 Subject: [PATCH 03/16] refactor: inline unnecessary extra function --- src/taxonomy/tag-list/TagListTable.tsx | 109 ++++++------------ .../tag-list/createTreeTableContextValue.ts | 21 ---- 2 files changed, 35 insertions(+), 95 deletions(-) delete mode 100644 src/taxonomy/tag-list/createTreeTableContextValue.ts diff --git a/src/taxonomy/tag-list/TagListTable.tsx b/src/taxonomy/tag-list/TagListTable.tsx index 06718fe5cf..fa9a1f3cf6 100644 --- a/src/taxonomy/tag-list/TagListTable.tsx +++ b/src/taxonomy/tag-list/TagListTable.tsx @@ -1,7 +1,6 @@ import React, { useState, useMemo, - useEffect, } from 'react'; import type { PaginationState } from '@tanstack/react-table'; import { useTagListData, useCreateTag, useUpdateTag } from '@src/taxonomy/data/apiHooks'; @@ -11,11 +10,11 @@ import type { TreeRowData, } from '../tree-table/types'; import { TagTree } from './tagTree'; +import { getColumns } from './tagColumns'; import { TABLE_MODES, } from './constants'; import { useTableModes, useEditActions } from './hooks'; -import { createTreeTableContextValue } from './createTreeTableContextValue'; interface TagListTableProps { taxonomyId: number; @@ -83,6 +82,7 @@ const TagListTable = ({ taxonomyId, maxDepth }: TagListTableProps) => { const createTagMutation = useCreateTag(taxonomyId); const updateTagMutation = useUpdateTag(taxonomyId); const pageCount = tagList?.numPages ?? -1; + const canAddTag = tagList?.canAddTag !== false; // TODO: to make this more readable, introduce a React context for the TagListTable instead of passing props. @@ -101,78 +101,39 @@ const TagListTable = ({ taxonomyId, maxDepth }: TagListTableProps) => { setEditingRowId, }); - // RELOAD DATA IN VIEW MODE - useEffect(() => { - // Get row data in VIEW mode. Otherwise keep current data to avoid disrupting - // users while they edit or create a tag. - if (tableMode === TABLE_MODES.VIEW && tagList?.results) { - const tree = new TagTree(tagList?.results); - if (tree) { - setTagTree(tree); - } - } - }, [tagList?.results, tableMode]); - - const contextValue = useMemo( - () => createTreeTableContextValue({ - treeData, - pageCount, - pagination, - handlePaginationChange, - isLoading, - isCreatingTopRow: isCreatingTopTag, - draftError, - createRowMutation: createTagMutation, - updateRowMutation: updateTagMutation, - toast, - setToast, - setIsCreatingTopRow: setIsCreatingTopTag, - exitDraftWithoutSave, - handleCreateRow: handleCreateTag, - creatingParentId, - setCreatingParentId, - setDraftError, - validate, - handleUpdateRow: handleUpdateTag, - editingRowId, - setEditingRowId, - onStartDraft: enterDraftMode, - setActiveActionMenuRowId, - hasOpenDraft, - canAddTag: tagList?.canAddTag !== false, - maxDepth, - }), - [ - treeData, - pageCount, - pagination, - handlePaginationChange, - isLoading, - isCreatingTopTag, - draftError, - createTagMutation, - updateTagMutation, - toast, - setToast, - setIsCreatingTopTag, - exitDraftWithoutSave, - handleCreateTag, - creatingParentId, - setCreatingParentId, - setDraftError, - validate, - handleUpdateTag, - editingRowId, - setEditingRowId, - enterDraftMode, - setActiveActionMenuRowId, - hasOpenDraft, - tagList?.canAddTag, - maxDepth, - ], - ); - - + // TreeTable context + const contextValueArgs = { + treeData, + pageCount, + pagination, + handlePaginationChange, + isLoading, + isCreatingTopRow: isCreatingTopTag, + draftError, + createRowMutation: createTagMutation, + updateRowMutation: updateTagMutation, + toast, + setToast, + setIsCreatingTopRow: setIsCreatingTopTag, + exitDraftWithoutSave, + handleCreateRow: handleCreateTag, + creatingParentId, + setCreatingParentId, + setDraftError, + validate, + handleUpdateRow: handleUpdateTag, + editingRowId, + setEditingRowId, + onStartDraft: enterDraftMode, + setActiveActionMenuRowId, + hasOpenDraft, + canAddTag, + maxDepth, + }; + const contextValue = { + ...contextValueArgs, + columns: getColumns(contextValueArgs), + }; return ( diff --git a/src/taxonomy/tag-list/createTreeTableContextValue.ts b/src/taxonomy/tag-list/createTreeTableContextValue.ts deleted file mode 100644 index a376eed29d..0000000000 --- a/src/taxonomy/tag-list/createTreeTableContextValue.ts +++ /dev/null @@ -1,21 +0,0 @@ -import type { RowId } from '@src/taxonomy/tree-table/types'; -import type { TreeTableContextValue } from '@src/taxonomy/tree-table/TreeTableContext'; - -import { getColumns } from './tagColumns'; - -interface CreateTagListTreeTableContextValueArgs extends Omit { - onStartDraft: () => void; - setActiveActionMenuRowId: (id: RowId | null) => void; - hasOpenDraft: boolean; - canAddTag: boolean; - maxDepth: number; -} - -const createTreeTableContextValue = ( - args: CreateTagListTreeTableContextValueArgs, -): TreeTableContextValue => ({ - ...args, - columns: getColumns(args), -}); - -export { createTreeTableContextValue }; \ No newline at end of file From b277c2e14d2d11491482129d396f94aedbf631d5 Mon Sep 17 00:00:00 2001 From: Jesper Hodge Date: Mon, 20 Apr 2026 14:31:19 -0400 Subject: [PATCH 04/16] fix: regression error --- src/taxonomy/tag-list/TagListTable.tsx | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/taxonomy/tag-list/TagListTable.tsx b/src/taxonomy/tag-list/TagListTable.tsx index fa9a1f3cf6..05860898cf 100644 --- a/src/taxonomy/tag-list/TagListTable.tsx +++ b/src/taxonomy/tag-list/TagListTable.tsx @@ -1,6 +1,7 @@ import React, { useState, useMemo, + useEffect, } from 'react'; import type { PaginationState } from '@tanstack/react-table'; import { useTagListData, useCreateTag, useUpdateTag } from '@src/taxonomy/data/apiHooks'; @@ -101,6 +102,18 @@ const TagListTable = ({ taxonomyId, maxDepth }: TagListTableProps) => { setEditingRowId, }); + // RELOAD DATA IN VIEW MODE + useEffect(() => { + // Get row data in VIEW mode. Otherwise keep current data to avoid disrupting + // users while they edit or create a tag. + if (tableMode === TABLE_MODES.VIEW && tagList?.results) { + const tree = new TagTree(tagList?.results); + if (tree) { + setTagTree(tree); + } + } + }, [tagList?.results, tableMode]); + // TreeTable context const contextValueArgs = { treeData, From 70ec5a8cce910ebae47a8d7471c016c6a9c68420 Mon Sep 17 00:00:00 2001 From: Jesper Hodge Date: Mon, 20 Apr 2026 16:47:55 -0400 Subject: [PATCH 05/16] feat: add delete confirm modal --- src/taxonomy/tag-list/Actions.tsx | 138 ++++++++++++++ src/taxonomy/tag-list/TagListTable.tsx | 55 ++++-- src/taxonomy/tag-list/TypeXToConfirmPopup.tsx | 70 +++++++ src/taxonomy/tag-list/hooks.ts | 59 +++++- src/taxonomy/tag-list/messages.ts | 12 ++ src/taxonomy/tag-list/tagColumns.tsx | 177 ++---------------- src/taxonomy/tree-table/TableView.test.tsx | 13 +- src/taxonomy/tree-table/TreeTableContext.tsx | 6 +- src/taxonomy/tree-table/index.ts | 2 +- 9 files changed, 346 insertions(+), 186 deletions(-) create mode 100644 src/taxonomy/tag-list/Actions.tsx create mode 100644 src/taxonomy/tag-list/TypeXToConfirmPopup.tsx diff --git a/src/taxonomy/tag-list/Actions.tsx b/src/taxonomy/tag-list/Actions.tsx new file mode 100644 index 0000000000..3865a6173d --- /dev/null +++ b/src/taxonomy/tag-list/Actions.tsx @@ -0,0 +1,138 @@ +import { + Icon, + IconButton, + IconButtonWithTooltip, + Dropdown, +} from '@openedx/paragon'; +import { + AddCircle, + MoreVert, +} from '@openedx/paragon/icons'; +import { useIntl } from '@edx/frontend-platform/i18n'; +import type { Row } from '@tanstack/react-table'; + +import type { + RowId, + TreeRowData, +} from '@src/taxonomy/tree-table/types'; +import type { TagListRowData } from './types'; +import messages from './messages'; + +interface ActionsHeaderProps { + onStartDraft: () => void; + setDraftError: (error: string) => void; + setIsCreatingTopRow: (isCreating: boolean) => void; + setEditingRowId: (id: RowId | null) => void; + setActiveActionMenuRowId: (id: RowId | null) => void; + hasOpenDraft: boolean; + draftInProgressHintId: string; + canAddTag: boolean; +} + +const ActionsHeader = ({ + onStartDraft, + setDraftError, + setIsCreatingTopRow, + setEditingRowId, + setActiveActionMenuRowId, + hasOpenDraft, + canAddTag, + draftInProgressHintId, +}: ActionsHeaderProps) => { + const intl = useIntl(); + return ( +
+ {intl.formatMessage(messages.createNewTagTooltip)}
} + src={AddCircle} + alt={intl.formatMessage(messages.createTagButtonLabel)} + size="inline" + onClick={() => { + onStartDraft(); + setDraftError(''); + setIsCreatingTopRow(true); + setEditingRowId(null); + setActiveActionMenuRowId(null); + }} + disabled={hasOpenDraft || !canAddTag} + aria-describedby={hasOpenDraft ? draftInProgressHintId : undefined} + /> +
+ ); +}; + +interface ActionsMenuProps { + rowData: TagListRowData; + startSubtagDraft: () => void; + disableAddSubtag: boolean; + startEditTag: () => void; + disableEditTag: boolean; + reachedMaxDepth: (row: Row) => boolean; + startDeleteTag: (row: Row) => void; + disableDeleteTag: boolean; + row: Row; +} + +const ActionsMenu = ({ + rowData, + row, + startSubtagDraft, + disableAddSubtag, + startEditTag, + disableEditTag, + reachedMaxDepth, + startDeleteTag, + disableDeleteTag, +}: ActionsMenuProps) => { + const intl = useIntl(); + + const deleteTagMenuItem = ( + startDeleteTag(row)} + disabled={disableDeleteTag} + > + {intl.formatMessage(messages.deleteTag)} + + ); + + const editTagMenuItem = ( + + {intl.formatMessage(messages.renameTag)} + + ); + + return ( + + + + + {intl.formatMessage(messages.addSubtag)} + + {editTagMenuItem} + {deleteTagMenuItem} + + + ); +}; + +const Actions = { + Header: ActionsHeader, + Menu: ActionsMenu, +}; + +export default Actions; diff --git a/src/taxonomy/tag-list/TagListTable.tsx b/src/taxonomy/tag-list/TagListTable.tsx index 05860898cf..d3a4d7ba0f 100644 --- a/src/taxonomy/tag-list/TagListTable.tsx +++ b/src/taxonomy/tag-list/TagListTable.tsx @@ -16,6 +16,7 @@ import { TABLE_MODES, } from './constants'; import { useTableModes, useEditActions } from './hooks'; +import TypeXToConfirmPopup from './TypeXToConfirmPopup'; interface TagListTableProps { taxonomyId: number; @@ -50,6 +51,8 @@ const TagListTable = ({ taxonomyId, maxDepth }: TagListTableProps) => { const [draftError, setDraftError] = useState(''); const treeData = (tagTree?.getAllAsDeepCopy() || []) as unknown as TreeRowData[]; const hasOpenDraft = isCreatingTopTag || creatingParentId !== null || editingRowId !== null; + const [confirmDeleteDialogOpen, setConfirmDeleteDialogOpen] = useState(false); + const [confirmDeleteDialogContext, setConfirmDeleteDialogContext] = useState | null>(null); // TABLE MODES const { @@ -89,18 +92,25 @@ const TagListTable = ({ taxonomyId, maxDepth }: TagListTableProps) => { // 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({ - setTagTree, - setDraftError, - createTagMutation, - updateTagMutation, - enterPreviewMode, - setToast, - setIsCreatingTopTag, - setCreatingParentId, - exitDraftWithoutSave, - setEditingRowId, - }); + const { handleCreateTag, handleUpdateTag, validate, startSubtagDraft, startEditTag, startDeleteTag, handleDeleteTag } = useEditActions( + { + enterDraftMode, + enterPreviewMode, + enterViewMode, + setTagTree, + setDraftError, + createTagMutation, + updateTagMutation, + setToast, + setIsCreatingTopTag, + setCreatingParentId, + exitDraftWithoutSave, + setEditingRowId, + setActiveActionMenuRowId, + setConfirmDeleteDialogOpen, + setConfirmDeleteDialogContext, + }, + ); // RELOAD DATA IN VIEW MODE useEffect(() => { @@ -142,6 +152,13 @@ const TagListTable = ({ taxonomyId, maxDepth }: TagListTableProps) => { hasOpenDraft, canAddTag, maxDepth, + startSubtagDraft, + startEditTag, + startDeleteTag, + confirmDeleteDialogOpen, + setConfirmDeleteDialogOpen, + confirmDeleteDialogContext, + setConfirmDeleteDialogContext, }; const contextValue = { ...contextValueArgs, @@ -151,6 +168,20 @@ const TagListTable = ({ taxonomyId, maxDepth }: TagListTableProps) => { return ( + { handleDeleteTag(row); setConfirmDeleteDialogOpen(false); setConfirmDeleteDialogContext(null); }} + onCancel={() => { + setConfirmDeleteDialogOpen(false); + setConfirmDeleteDialogContext(null); + }} + /> ); }; diff --git a/src/taxonomy/tag-list/TypeXToConfirmPopup.tsx b/src/taxonomy/tag-list/TypeXToConfirmPopup.tsx new file mode 100644 index 0000000000..e9a2706666 --- /dev/null +++ b/src/taxonomy/tag-list/TypeXToConfirmPopup.tsx @@ -0,0 +1,70 @@ +import React from 'react'; +import PropTypes from 'prop-types'; +import { Button, Form, ModalDialog } from '@openedx/paragon'; + +const TypeXToConfirmPopup = ({ + label, + X, + bodyText, + confirmLabel, + cancelLabel, + isOpen, + context, + onConfirm, + onCancel, +}) => { + const [confirmedByTyping, setConfirmedByTyping] = React.useState(false); + + return ( + + + {label} + + +
{bodyText}
+
+
+ Type {X} to confirm +
+ e.target.value === X ? setConfirmedByTyping(true) : setConfirmedByTyping(false)} + /> +
+ + + + + + + ); +}; + +TypeXToConfirmPopup.propTypes = { + label: PropTypes.string.isRequired, + bodyText: PropTypes.string.isRequired, + onConfirm: PropTypes.func.isRequired, + onCancel: PropTypes.func.isRequired, + confirmLabel: PropTypes.string.isRequired, + cancelLabel: PropTypes.string.isRequired, + X: PropTypes.string.isRequired, + context: PropTypes.any, + confirmButtonClass: PropTypes.string, + cancelButtonClass: PropTypes.string, + confirmVariant: PropTypes.string, +}; +TypeXToConfirmPopup.defaultProps = { + confirmVariant: 'outline-brand', + confirmButtonClass: '', + cancelButtonClass: '', +}; + +export default React.memo(TypeXToConfirmPopup); diff --git a/src/taxonomy/tag-list/hooks.ts b/src/taxonomy/tag-list/hooks.ts index a1518b9de9..03825920a8 100644 --- a/src/taxonomy/tag-list/hooks.ts +++ b/src/taxonomy/tag-list/hooks.ts @@ -4,7 +4,7 @@ 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, TreeRowData } from '@src/taxonomy/tree-table/types'; import { TagTree } from './tagTree'; import { TagListTableError } from './errors'; import { @@ -15,6 +15,9 @@ import { } from './constants'; import messages from './messages'; +import { start } from '@src/library-authoring/__mocks__/contentLibrariesListV2'; +import { getTagListRowData } from './utils'; +import { Row } from '@tanstack/react-table'; /** Interface for table mode actions for React's `useReducer` hook. * @@ -41,13 +44,18 @@ interface UseEditActionsParams { setTagTree: React.Dispatch>; setDraftError: React.Dispatch>; createTagMutation: ReturnType; + enterDraftMode: () => void; enterPreviewMode: () => void; + enterViewMode: () => void; setToast: React.Dispatch>; setIsCreatingTopTag: React.Dispatch>; setCreatingParentId: React.Dispatch>; exitDraftWithoutSave: () => void; setEditingRowId: React.Dispatch>; updateTagMutation: ReturnType; + setActiveActionMenuRowId: React.Dispatch>; + setConfirmDeleteDialogOpen: React.Dispatch>; + setConfirmDeleteDialogContext: React.Dispatch | null>>; } const getInlineValidationMessage = (value: string, intl: ReturnType): string => { @@ -111,16 +119,20 @@ const useTableModes = (): UseTableModesReturn => { }; const useEditActions = ({ + enterDraftMode, + enterPreviewMode, + enterViewMode, setTagTree, setDraftError, createTagMutation, - enterPreviewMode, setToast, setIsCreatingTopTag, setCreatingParentId, exitDraftWithoutSave, setEditingRowId, updateTagMutation, + setConfirmDeleteDialogOpen, + setConfirmDeleteDialogContext, }: UseEditActionsParams) => { const intl = useIntl(); @@ -255,11 +267,54 @@ const useEditActions = ({ } }; + const startSubtagDraft = (row: Row) => { + const rowData = getTagListRowData(row); + enterDraftMode(); + setDraftError(''); + setCreatingParentId(rowData.id); + row.toggleExpanded(true); + }; + + const startEditTag = (row: Row) => { + const rowData = getTagListRowData(row); + enterDraftMode(); + setDraftError(''); + setEditingRowId(`${rowData.id}:${rowData.value}`); + }; + + const startDeleteTag = (row: Row) => { + setConfirmDeleteDialogOpen(true); + setConfirmDeleteDialogContext(row); + }; + + const someDeleteTagAPICall = async (value: string) => { + // Placeholder for actual delete API call + return new Promise((resolve) => setTimeout(resolve, 1000)); + } + const handleDeleteTag = async (row: Row) => { + const rowData = getTagListRowData(row); + try { + enterViewMode(); + await someDeleteTagAPICall(rowData.value); // Replace with actual delete API call + setToast({ + show: true, + message: intl.formatMessage(messages.tagDeleteSuccessMessage, { name: rowData.value }), + }); + } catch (error) { + const errorMessage = getErrorMessage(error); + setToast({ show: true, message: intl.formatMessage(messages.tagDeleteErrorMessage, { errorMessage }) }); + } + }; + return { updateTableWithoutDataReload, handleCreateTag, handleUpdateTag, + startSubtagDraft, + startEditTag, + startDeleteTag, validate, + handleDeleteTag, }; }; diff --git a/src/taxonomy/tag-list/messages.ts b/src/taxonomy/tag-list/messages.ts index bd3d2e7be7..4edf43b38b 100644 --- a/src/taxonomy/tag-list/messages.ts +++ b/src/taxonomy/tag-list/messages.ts @@ -73,6 +73,18 @@ const messages = defineMessages({ id: 'course-authoring.tag-list.delete-tag-disabled-tooltip', defaultMessage: 'This tag does not allow deletion', }, + tagEditForbidden: { + id: 'course-authoring.tag-list.system-defined-tag-edit-disabled', + defaultMessage: 'Disabled because this is not allowed to be changed', + }, + tagDeleteForbidden: { + id: 'course-authoring.tag-list.system-defined-tag-delete-disabled', + defaultMessage: 'Disabled because this is not allowed to be deleted', + }, + hasOpenDraft: { + id: 'course-authoring.tag-list.has-open-draft', + defaultMessage: 'Disabled because tag creation or edit is in progress', + }, }); export default messages; diff --git a/src/taxonomy/tag-list/tagColumns.tsx b/src/taxonomy/tag-list/tagColumns.tsx index 15dab666b0..86c26d5d2c 100644 --- a/src/taxonomy/tag-list/tagColumns.tsx +++ b/src/taxonomy/tag-list/tagColumns.tsx @@ -1,16 +1,4 @@ -import { - Icon, - IconButton, - IconButtonWithTooltip, - Dropdown, - OverlayTrigger, - Tooltip, -} from '@openedx/paragon'; -import { - AddCircle, - MoreVert, -} from '@openedx/paragon/icons'; -import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n'; +import { FormattedMessage } from '@edx/frontend-platform/i18n'; import type { Row } from '@tanstack/react-table'; import type { @@ -18,17 +6,16 @@ import type { TreeColumnDef, TreeRowData, } from '@src/taxonomy/tree-table/types'; -import type { TagListRowData } from './types'; import messages from './messages'; import OptionalExpandLink from './OptionalExpandLink'; import UsageCountDisplay from './UsageCountDisplay'; import { getTagListRowData } from './utils'; +import Actions from './Actions'; const EDITABLE_COLUMNS = ['value']; interface GetColumnsArgs { setIsCreatingTopRow: (isCreating: boolean) => void; - setCreatingParentId: (id: RowId | null) => void; setEditingRowId: (id: RowId | null) => void; onStartDraft: () => void; setActiveActionMenuRowId: (id: RowId | null) => void; @@ -36,133 +23,18 @@ interface GetColumnsArgs { canAddTag: boolean; setDraftError: (error: string) => void; maxDepth: number; + startSubtagDraft: (row: Row) => void; + startEditTag: (row: Row) => void; + startDeleteTag: (row: Row) => void; } -interface ActionsHeaderProps { - onStartDraft: () => void; - setDraftError: (error: string) => void; - setIsCreatingTopRow: (isCreating: boolean) => void; - setEditingRowId: (id: RowId | null) => void; - setActiveActionMenuRowId: (id: RowId | null) => void; - hasOpenDraft: boolean; - draftInProgressHintId: string; - canAddTag: boolean; -} - -const ActionsHeader = ({ - onStartDraft, - setDraftError, - setIsCreatingTopRow, - setEditingRowId, - setActiveActionMenuRowId, - hasOpenDraft, - canAddTag, - draftInProgressHintId, -}: ActionsHeaderProps) => { - const intl = useIntl(); - return ( -
- {intl.formatMessage(messages.createNewTagTooltip)}
} - src={AddCircle} - alt={intl.formatMessage(messages.createTagButtonLabel)} - size="inline" - onClick={() => { - onStartDraft(); - setDraftError(''); - setIsCreatingTopRow(true); - setEditingRowId(null); - setActiveActionMenuRowId(null); - }} - disabled={hasOpenDraft || !canAddTag} - aria-describedby={hasOpenDraft ? draftInProgressHintId : undefined} - /> - - ); -}; - -interface ActionsMenuProps { - rowData: TagListRowData; - startSubtagDraft: () => void; - disableAddSubtag: boolean; - editTag: () => void; - disableEditTag: boolean; - reachedMaxDepth: (row: Row) => boolean; - deleteTag: () => void; - disableDeleteTag: boolean; - - row: Row; -} - -const ActionsMenu = ({ - rowData, - row, - startSubtagDraft, - disableAddSubtag, - editTag, - disableEditTag, - reachedMaxDepth, - deleteTag, - disableDeleteTag, -}: ActionsMenuProps) => { - const intl = useIntl(); - - const deleteTagMenuItem = ( - - {intl.formatMessage(messages.deleteTag)} - - ) - - return ( - - - - - {intl.formatMessage(messages.addSubtag)} - - - {intl.formatMessage(messages.renameTag)} - - {disableDeleteTag ? ( - - {intl.formatMessage(messages.deleteTagDisabledTooltip)} - - } - > - {deleteTagMenuItem} - - ) : deleteTagMenuItem} - - - ); -}; - function getColumns({ setIsCreatingTopRow, - setCreatingParentId, setEditingRowId, onStartDraft, + startSubtagDraft, + startEditTag, + startDeleteTag, setActiveActionMenuRowId, hasOpenDraft, canAddTag, @@ -197,7 +69,7 @@ function getColumns({ { id: 'actions', header: () => ( - { - onStartDraft(); - setDraftError(''); - setCreatingParentId(rowData.id); - setEditingRowId(null); - setIsCreatingTopRow(false); - setActiveActionMenuRowId(null); - row.toggleExpanded(true); - }; - - const editTag = () => { - onStartDraft(); - setDraftError(''); - setEditingRowId(`${rowData.id}:${rowData.value}`); - setCreatingParentId(null); - setIsCreatingTopRow(false); - setActiveActionMenuRowId(null); - }; - - const deleteTag = () => { - // todo: implement - }; - return (
- startSubtagDraft(row)} disableAddSubtag={disableAddSubtag} - editTag={editTag} + startEditTag={() => startEditTag(row)} disableEditTag={disableEditTag} reachedMaxDepth={reachedMaxDepth} - deleteTag={deleteTag} + startDeleteTag={() => startDeleteTag(row)} disableDeleteTag={disableDeleteTag} />
diff --git a/src/taxonomy/tree-table/TableView.test.tsx b/src/taxonomy/tree-table/TableView.test.tsx index 1157713339..d4467e483f 100644 --- a/src/taxonomy/tree-table/TableView.test.tsx +++ b/src/taxonomy/tree-table/TableView.test.tsx @@ -48,12 +48,13 @@ const baseContextValue = () => ({ const renderTableView = ( contextValue = baseContextValue(), props: React.ComponentProps = {}, -) => render( - - - , - { wrapper }, -); +) => + render( + + + , + { wrapper }, + ); describe('TableView', () => { it('shows and dismisses save error banner', () => { diff --git a/src/taxonomy/tree-table/TreeTableContext.tsx b/src/taxonomy/tree-table/TreeTableContext.tsx index b2ad9c2f3f..02f0b4aeea 100644 --- a/src/taxonomy/tree-table/TreeTableContext.tsx +++ b/src/taxonomy/tree-table/TreeTableContext.tsx @@ -33,6 +33,8 @@ export interface TreeTableContextValue { handleUpdateRow: (value: string, originalValue: string) => void; editingRowId: RowId | null; setEditingRowId: (id: RowId | null) => void; + confirmDeleteDialogOpen: boolean; + setConfirmDeleteDialogOpen: Dispatch>; } export const TreeTableContext = createContext({ @@ -58,4 +60,6 @@ export const TreeTableContext = createContext({ handleUpdateRow: () => {}, editingRowId: null, setEditingRowId: () => {}, -}); \ No newline at end of file + confirmDeleteDialogOpen: false, + setConfirmDeleteDialogOpen: () => {}, +}); diff --git a/src/taxonomy/tree-table/index.ts b/src/taxonomy/tree-table/index.ts index d44d8f393f..44b27a7709 100644 --- a/src/taxonomy/tree-table/index.ts +++ b/src/taxonomy/tree-table/index.ts @@ -1,3 +1,3 @@ export { EditableCell } from './EditableCell'; -export { TreeTableContext } from './TreeTableContext'; export { TableView } from './TableView'; +export { TreeTableContext } from './TreeTableContext'; From cff509094cbf09f9b95b099f366f44ebd47710c8 Mon Sep 17 00:00:00 2001 From: Jesper Hodge Date: Tue, 21 Apr 2026 11:17:47 -0400 Subject: [PATCH 06/16] fix: tag count + confirm modal disable --- src/generic/TypeXToConfirmModal.tsx | 96 +++++++++++++++++++ src/taxonomy/tag-list/TagListTable.tsx | 8 +- src/taxonomy/tag-list/TypeXToConfirmPopup.tsx | 70 -------------- src/taxonomy/tag-list/hooks.ts | 14 ++- src/taxonomy/tag-list/messages.ts | 8 ++ tsconfig.json | 2 +- 6 files changed, 120 insertions(+), 78 deletions(-) create mode 100644 src/generic/TypeXToConfirmModal.tsx delete mode 100644 src/taxonomy/tag-list/TypeXToConfirmPopup.tsx diff --git a/src/generic/TypeXToConfirmModal.tsx b/src/generic/TypeXToConfirmModal.tsx new file mode 100644 index 0000000000..b7e7e1bad3 --- /dev/null +++ b/src/generic/TypeXToConfirmModal.tsx @@ -0,0 +1,96 @@ +import React, { useEffect } from 'react'; +import { Button, Form, ModalDialog } from '@openedx/paragon'; + +interface TypeXToConfirmModalProps { + label: string; + bodyText: string; + confirmLabel: string; + cancelLabel: string; + X: string; + context?: any; + isOpen: boolean; + onConfirm: (context?: any) => void; + onCancel: () => void; +} + +const TypeXToConfirmModal: React.FC = ({ + label, + X, + bodyText, + confirmLabel, + cancelLabel, + isOpen, + context, + onConfirm, + onCancel, +}) => { + const [confirmedByTyping, setConfirmedByTyping] = React.useState(false); + + const handleKeyDown = (e: React.KeyboardEvent) => { + if (!confirmedByTyping) return; + if (e.key === 'Enter') { + onConfirm(context); + } + }; + + const handleConfirm = () => { + if (!confirmedByTyping) return; + setConfirmedByTyping(false); + onConfirm(context); + }; + + const handleCancel = () => { + setConfirmedByTyping(false); + onCancel(); + }; + + const handleChange = (e: React.ChangeEvent) => { + if (e.target.value === X) { + setConfirmedByTyping(true); + } else { + setConfirmedByTyping(false); + } + }; + + // Don't remove. This is necessary to prevent an old state from erroneously enabling the confirm button + useEffect(() => { + if (!isOpen) { + setConfirmedByTyping(false); + } + }, [X, isOpen]); + + return ( + + + {label} + + +
{bodyText}
+
+
+ Type {X} to confirm +
+ +
+ + + + + + + ); +}; + +export default React.memo(TypeXToConfirmModal); diff --git a/src/taxonomy/tag-list/TagListTable.tsx b/src/taxonomy/tag-list/TagListTable.tsx index d3a4d7ba0f..f924820c67 100644 --- a/src/taxonomy/tag-list/TagListTable.tsx +++ b/src/taxonomy/tag-list/TagListTable.tsx @@ -3,7 +3,7 @@ import React, { useMemo, useEffect, } from 'react'; -import type { PaginationState } from '@tanstack/react-table'; +import type { PaginationState, Row } from '@tanstack/react-table'; import { useTagListData, useCreateTag, useUpdateTag } from '@src/taxonomy/data/apiHooks'; import { TableView, TreeTableContext } from '@src/taxonomy/tree-table'; import type { @@ -16,7 +16,7 @@ import { TABLE_MODES, } from './constants'; import { useTableModes, useEditActions } from './hooks'; -import TypeXToConfirmPopup from './TypeXToConfirmPopup'; +import TypeXToConfirmModal from '@src/generic/TypeXToConfirmModal'; interface TagListTableProps { taxonomyId: number; @@ -88,8 +88,6 @@ const TagListTable = ({ taxonomyId, maxDepth }: TagListTableProps) => { const pageCount = tagList?.numPages ?? -1; const canAddTag = tagList?.canAddTag !== false; - // 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, startSubtagDraft, startEditTag, startDeleteTag, handleDeleteTag } = useEditActions( @@ -168,7 +166,7 @@ const TagListTable = ({ taxonomyId, maxDepth }: TagListTableProps) => { return ( - { - const [confirmedByTyping, setConfirmedByTyping] = React.useState(false); - - return ( - - - {label} - - -
{bodyText}
-
-
- Type {X} to confirm -
- e.target.value === X ? setConfirmedByTyping(true) : setConfirmedByTyping(false)} - /> -
- - - - - - - ); -}; - -TypeXToConfirmPopup.propTypes = { - label: PropTypes.string.isRequired, - bodyText: PropTypes.string.isRequired, - onConfirm: PropTypes.func.isRequired, - onCancel: PropTypes.func.isRequired, - confirmLabel: PropTypes.string.isRequired, - cancelLabel: PropTypes.string.isRequired, - X: PropTypes.string.isRequired, - context: PropTypes.any, - confirmButtonClass: PropTypes.string, - cancelButtonClass: PropTypes.string, - confirmVariant: PropTypes.string, -}; -TypeXToConfirmPopup.defaultProps = { - confirmVariant: 'outline-brand', - confirmButtonClass: '', - cancelButtonClass: '', -}; - -export default React.memo(TypeXToConfirmPopup); diff --git a/src/taxonomy/tag-list/hooks.ts b/src/taxonomy/tag-list/hooks.ts index 03825920a8..2b03855456 100644 --- a/src/taxonomy/tag-list/hooks.ts +++ b/src/taxonomy/tag-list/hooks.ts @@ -118,6 +118,13 @@ const useTableModes = (): UseTableModesReturn => { }; }; +const getTagWithDescendantsCount = (rowData: TreeRowData): number => { + if (!rowData.subRows || rowData.subRows.length === 0) { + return 1; + } + return rowData.subRows.reduce((count, subRow) => count + getTagWithDescendantsCount(subRow), 1); +}; + const useEditActions = ({ enterDraftMode, enterPreviewMode, @@ -290,15 +297,18 @@ const useEditActions = ({ const someDeleteTagAPICall = async (value: string) => { // Placeholder for actual delete API call return new Promise((resolve) => setTimeout(resolve, 1000)); - } + }; + const handleDeleteTag = async (row: Row) => { const rowData = getTagListRowData(row); + const count = getTagWithDescendantsCount(rowData); try { + // In view mode, the table reloads on change, reflecting the deletion without needing to manually update the table state enterViewMode(); await someDeleteTagAPICall(rowData.value); // Replace with actual delete API call setToast({ show: true, - message: intl.formatMessage(messages.tagDeleteSuccessMessage, { name: rowData.value }), + message: intl.formatMessage(messages.tagsDeleteSuccessMessage, { count }), }); } catch (error) { const errorMessage = getErrorMessage(error); diff --git a/src/taxonomy/tag-list/messages.ts b/src/taxonomy/tag-list/messages.ts index 4edf43b38b..ec889fa4fd 100644 --- a/src/taxonomy/tag-list/messages.ts +++ b/src/taxonomy/tag-list/messages.ts @@ -85,6 +85,14 @@ const messages = defineMessages({ id: 'course-authoring.tag-list.has-open-draft', defaultMessage: 'Disabled because tag creation or edit is in progress', }, + tagsDeleteSuccessMessage: { + id: 'course-authoring.tag-list.delete-success', + defaultMessage: '{count} tag(s) deleted. This change will be applied across all tagged content.', + }, + tagDeleteErrorMessage: { + id: 'course-authoring.tag-list.delete-error', + defaultMessage: 'Error deleting tag: {errorMessage}', + }, }); export default messages; diff --git a/tsconfig.json b/tsconfig.json index e529f8012f..b924613492 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -14,6 +14,6 @@ ".eslintrc.js", "src/**/*", "plugins/**/*" - ], +, "src/generic/TypeXToConfirmModal.tsx" ], "exclude": ["dist", "node_modules"] } From d5d00cdced0160d9c8c6c76d3242f49e286457dc Mon Sep 17 00:00:00 2001 From: Jesper Hodge Date: Tue, 21 Apr 2026 12:29:41 -0400 Subject: [PATCH 07/16] fix: internationalization --- src/generic/TypeXToConfirmModal.tsx | 17 ++++-- src/taxonomy/tag-list/Actions.tsx | 32 ++++++------ src/taxonomy/tag-list/DeleteModal.tsx | 55 ++++++++++++++++++++ src/taxonomy/tag-list/TagListTable.tsx | 25 ++------- src/taxonomy/tag-list/hooks.ts | 19 +++---- src/taxonomy/tag-list/messages.ts | 28 ++++++++++ src/taxonomy/tag-list/tagColumns.tsx | 20 +++---- src/taxonomy/tag-list/utils.ts | 7 +++ src/taxonomy/tree-table/TreeTableContext.tsx | 12 ++++- 9 files changed, 149 insertions(+), 66 deletions(-) create mode 100644 src/taxonomy/tag-list/DeleteModal.tsx diff --git a/src/generic/TypeXToConfirmModal.tsx b/src/generic/TypeXToConfirmModal.tsx index b7e7e1bad3..5ea3b48fb6 100644 --- a/src/generic/TypeXToConfirmModal.tsx +++ b/src/generic/TypeXToConfirmModal.tsx @@ -7,10 +7,12 @@ interface TypeXToConfirmModalProps { confirmLabel: string; cancelLabel: string; X: string; - context?: any; + // any additional context that the caller wants to pass to the onConfirm callback; not a React context. + context?: Record | null; isOpen: boolean; - onConfirm: (context?: any) => void; + onConfirm: (context?: Record | null) => void; onCancel: () => void; + setContext?: (context: Record | null) => void; } const TypeXToConfirmModal: React.FC = ({ @@ -23,18 +25,19 @@ const TypeXToConfirmModal: React.FC = ({ context, onConfirm, onCancel, + setContext, }) => { const [confirmedByTyping, setConfirmedByTyping] = React.useState(false); const handleKeyDown = (e: React.KeyboardEvent) => { - if (!confirmedByTyping) return; + if (!confirmedByTyping) { return; } if (e.key === 'Enter') { onConfirm(context); } }; const handleConfirm = () => { - if (!confirmedByTyping) return; + if (!confirmedByTyping) { return; } setConfirmedByTyping(false); onConfirm(context); }; @@ -56,8 +59,12 @@ const TypeXToConfirmModal: React.FC = ({ useEffect(() => { if (!isOpen) { setConfirmedByTyping(false); + if (setContext) { + // reset onConfirm callback context when modal is closed + setContext(null); + } } - }, [X, isOpen]); + }, [X, isOpen, context, setContext]); return ( void; disableAddSubtag: boolean; - startEditTag: () => void; - disableEditTag: boolean; + startEditRow: () => void; + disableEditRow: boolean; reachedMaxDepth: (row: Row) => boolean; - startDeleteTag: (row: Row) => void; - disableDeleteTag: boolean; + startDeleteRow: (row: Row) => void; + disableDeleteRow: boolean; row: Row; } @@ -79,27 +79,27 @@ const ActionsMenu = ({ row, startSubtagDraft, disableAddSubtag, - startEditTag, - disableEditTag, + startEditRow, + disableEditRow, reachedMaxDepth, - startDeleteTag, - disableDeleteTag, + startDeleteRow, + disableDeleteRow, }: ActionsMenuProps) => { const intl = useIntl(); - const deleteTagMenuItem = ( + const deleteRowMenuItem = ( startDeleteTag(row)} - disabled={disableDeleteTag} + onClick={() => startDeleteRow(row)} + disabled={disableDeleteRow} > {intl.formatMessage(messages.deleteTag)} ); - const editTagMenuItem = ( + const editRowMenuItem = ( {intl.formatMessage(messages.renameTag)} @@ -123,8 +123,8 @@ const ActionsMenu = ({ > {intl.formatMessage(messages.addSubtag)} - {editTagMenuItem} - {deleteTagMenuItem} + {editRowMenuItem} + {deleteRowMenuItem} ); diff --git a/src/taxonomy/tag-list/DeleteModal.tsx b/src/taxonomy/tag-list/DeleteModal.tsx new file mode 100644 index 0000000000..58349ca870 --- /dev/null +++ b/src/taxonomy/tag-list/DeleteModal.tsx @@ -0,0 +1,55 @@ +import { useContext, useMemo } from 'react'; +import { Row } from '@tanstack/react-table'; +import { useIntl } from '@edx/frontend-platform/i18n'; + +import TypeXToConfirmModal from '@src/generic/TypeXToConfirmModal'; +import { TreeTableContext } from '@src/taxonomy/tree-table'; +import { TreeRowData } from '@src/taxonomy/tree-table/types'; +import messages from './messages'; +import { getTagListRowData, getTagWithDescendantsCount } from './utils'; + +const DeleteModal = () => { + const { + confirmDeleteDialogOpen, + setConfirmDeleteDialogOpen, + confirmDeleteDialogContext, + setConfirmDeleteDialogContext, + handleDeleteTag, + } = useContext(TreeTableContext); + const intl = useIntl(); + + const handleConfirm = (row: Row) => { + handleDeleteTag(row); + setConfirmDeleteDialogOpen(false); + setConfirmDeleteDialogContext(null); + }; + + const handleCancel = () => { + setConfirmDeleteDialogOpen(false); + setConfirmDeleteDialogContext(null); + }; + + const rowData = confirmDeleteDialogContext ? getTagListRowData(confirmDeleteDialogContext) : null; + const count = useMemo(() => rowData ? getTagWithDescendantsCount(rowData) : 0, [confirmDeleteDialogContext]); + + const hasSubtags = count > 1; + const bodyText = hasSubtags ? intl.formatMessage(messages.deleteTagWithSubtagsConfirmation, { count }) : intl.formatMessage(messages.deleteTagConfirmation); + const typeToDeleteText = hasSubtags ? intl.formatMessage(messages.typeToConfirmDeleteTagWithSubtags) : intl.formatMessage(messages.typeToConfirmDeleteOneTag); + + return ( + + ); +}; + +export default DeleteModal; diff --git a/src/taxonomy/tag-list/TagListTable.tsx b/src/taxonomy/tag-list/TagListTable.tsx index f924820c67..b69d99ae6e 100644 --- a/src/taxonomy/tag-list/TagListTable.tsx +++ b/src/taxonomy/tag-list/TagListTable.tsx @@ -17,6 +17,7 @@ import { } from './constants'; import { useTableModes, useEditActions } from './hooks'; import TypeXToConfirmModal from '@src/generic/TypeXToConfirmModal'; +import DeleteModal from './DeleteModal'; interface TagListTableProps { taxonomyId: number; @@ -90,7 +91,7 @@ const TagListTable = ({ taxonomyId, maxDepth }: TagListTableProps) => { // 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, startSubtagDraft, startEditTag, startDeleteTag, handleDeleteTag } = useEditActions( + const editActions = useEditActions( { enterDraftMode, enterPreviewMode, @@ -124,6 +125,7 @@ const TagListTable = ({ taxonomyId, maxDepth }: TagListTableProps) => { // TreeTable context const contextValueArgs = { + ...editActions, treeData, pageCount, pagination, @@ -137,12 +139,9 @@ const TagListTable = ({ taxonomyId, maxDepth }: TagListTableProps) => { setToast, setIsCreatingTopRow: setIsCreatingTopTag, exitDraftWithoutSave, - handleCreateRow: handleCreateTag, creatingParentId, setCreatingParentId, setDraftError, - validate, - handleUpdateRow: handleUpdateTag, editingRowId, setEditingRowId, onStartDraft: enterDraftMode, @@ -150,9 +149,6 @@ const TagListTable = ({ taxonomyId, maxDepth }: TagListTableProps) => { hasOpenDraft, canAddTag, maxDepth, - startSubtagDraft, - startEditTag, - startDeleteTag, confirmDeleteDialogOpen, setConfirmDeleteDialogOpen, confirmDeleteDialogContext, @@ -166,20 +162,7 @@ const TagListTable = ({ taxonomyId, maxDepth }: TagListTableProps) => { return ( - { handleDeleteTag(row); setConfirmDeleteDialogOpen(false); setConfirmDeleteDialogContext(null); }} - onCancel={() => { - setConfirmDeleteDialogOpen(false); - setConfirmDeleteDialogContext(null); - }} - /> + ); }; diff --git a/src/taxonomy/tag-list/hooks.ts b/src/taxonomy/tag-list/hooks.ts index 2b03855456..156fd67d63 100644 --- a/src/taxonomy/tag-list/hooks.ts +++ b/src/taxonomy/tag-list/hooks.ts @@ -16,7 +16,7 @@ import { import messages from './messages'; import { start } from '@src/library-authoring/__mocks__/contentLibrariesListV2'; -import { getTagListRowData } from './utils'; +import { getTagListRowData, getTagWithDescendantsCount } from './utils'; import { Row } from '@tanstack/react-table'; /** Interface for table mode actions for React's `useReducer` hook. @@ -118,13 +118,6 @@ const useTableModes = (): UseTableModesReturn => { }; }; -const getTagWithDescendantsCount = (rowData: TreeRowData): number => { - if (!rowData.subRows || rowData.subRows.length === 0) { - return 1; - } - return rowData.subRows.reduce((count, subRow) => count + getTagWithDescendantsCount(subRow), 1); -}; - const useEditActions = ({ enterDraftMode, enterPreviewMode, @@ -318,13 +311,13 @@ const useEditActions = ({ return { updateTableWithoutDataReload, - handleCreateTag, - handleUpdateTag, + handleCreateRow: handleCreateTag, + handleUpdateRow: handleUpdateTag, startSubtagDraft, - startEditTag, - startDeleteTag, + startEditRow: startEditTag, + startDeleteRow: startDeleteTag, validate, - handleDeleteTag, + handleDeleteRow: handleDeleteTag, }; }; diff --git a/src/taxonomy/tag-list/messages.ts b/src/taxonomy/tag-list/messages.ts index ec889fa4fd..20d48160a3 100644 --- a/src/taxonomy/tag-list/messages.ts +++ b/src/taxonomy/tag-list/messages.ts @@ -93,6 +93,34 @@ const messages = defineMessages({ id: 'course-authoring.tag-list.delete-error', defaultMessage: 'Error deleting tag: {errorMessage}', }, + confirmDeleteTitle: { + id: 'course-authoring.tag-list.confirm-delete-title', + defaultMessage: 'Delete "{tagName}"', + }, + typeToConfirmDeleteOneTag: { + id: 'course-authoring.tag-list.delete-one-tag-type-to-confirm', + defaultMessage: 'DELETE', + }, + deleteTagConfirmation: { + id: 'course-authoring.tag-list.delete-tag-confirmation', + defaultMessage: 'Warning! You are about to delete a tag. If the tag is applied to course content it will be removed.', + }, + deleteLabel: { + id: 'course-authoring.tag-list.delete-label', + defaultMessage: 'Delete', + }, + cancelLabel: { + id: 'course-authoring.tag-list.cancel-label', + defaultMessage: 'Cancel', + }, + typeToConfirmDeleteTagWithSubtags: { + id: 'course-authoring.tag-list.delete-tag-with-subtags-type-to-confirm', + defaultMessage: 'DELETE WITH SUB-TAGS', + }, + deleteTagWithSubtagsConfirmation: { + id: 'course-authoring.tag-list.delete-tag-with-subtags-confirmation', + defaultMessage: 'Warning! You are about to delete a tag containing sub-tags. If you proceed, {count} sub-tags that you did not directly select will also be deleted. Any tags applied to course content will be removed.', + }, }); export default messages; diff --git a/src/taxonomy/tag-list/tagColumns.tsx b/src/taxonomy/tag-list/tagColumns.tsx index 86c26d5d2c..800355a761 100644 --- a/src/taxonomy/tag-list/tagColumns.tsx +++ b/src/taxonomy/tag-list/tagColumns.tsx @@ -24,8 +24,8 @@ interface GetColumnsArgs { setDraftError: (error: string) => void; maxDepth: number; startSubtagDraft: (row: Row) => void; - startEditTag: (row: Row) => void; - startDeleteTag: (row: Row) => void; + startEditRow: (row: Row) => void; + startDeleteRow: (row: Row) => void; } function getColumns({ @@ -33,8 +33,8 @@ function getColumns({ setEditingRowId, onStartDraft, startSubtagDraft, - startEditTag, - startDeleteTag, + startEditRow, + startDeleteRow, setActiveActionMenuRowId, hasOpenDraft, canAddTag, @@ -88,8 +88,8 @@ function getColumns({ } const disableAddSubtag = hasOpenDraft || !canAddTag; - const disableEditTag = hasOpenDraft || rowData.canChangeTag === false; - const disableDeleteTag = hasOpenDraft || rowData.canDeleteTag === false; + const disableEditRow = hasOpenDraft || rowData.canChangeTag === false; + const disableDeleteRow = hasOpenDraft || rowData.canDeleteTag === false; return (
@@ -98,11 +98,11 @@ function getColumns({ row={row} startSubtagDraft={() => startSubtagDraft(row)} disableAddSubtag={disableAddSubtag} - startEditTag={() => startEditTag(row)} - disableEditTag={disableEditTag} + startEditRow={() => startEditRow(row)} + disableEditRow={disableEditRow} reachedMaxDepth={reachedMaxDepth} - startDeleteTag={() => startDeleteTag(row)} - disableDeleteTag={disableDeleteTag} + startDeleteRow={() => startDeleteRow(row)} + disableDeleteRow={disableDeleteRow} />
); diff --git a/src/taxonomy/tag-list/utils.ts b/src/taxonomy/tag-list/utils.ts index 012ca2b824..9f0cbbbc19 100644 --- a/src/taxonomy/tag-list/utils.ts +++ b/src/taxonomy/tag-list/utils.ts @@ -10,3 +10,10 @@ import { TagListRowData } from './types'; export const getTagListRowData = (row: Row): TagListRowData => ( row.original as unknown as TagListRowData ); + +export const getTagWithDescendantsCount = (rowData: TreeRowData): number => { + if (!rowData.subRows || rowData.subRows.length === 0) { + return 1; + } + return rowData.subRows.reduce((count, subRow) => count + getTagWithDescendantsCount(subRow), 1); +}; diff --git a/src/taxonomy/tree-table/TreeTableContext.tsx b/src/taxonomy/tree-table/TreeTableContext.tsx index 02f0b4aeea..55f1baa048 100644 --- a/src/taxonomy/tree-table/TreeTableContext.tsx +++ b/src/taxonomy/tree-table/TreeTableContext.tsx @@ -1,6 +1,6 @@ import { createContext } from 'react'; import type { Dispatch, SetStateAction } from 'react'; -import type { OnChangeFn, PaginationState } from '@tanstack/react-table'; +import type { OnChangeFn, PaginationState, Row } from '@tanstack/react-table'; import type { CreateRowMutationState, @@ -35,6 +35,11 @@ export interface TreeTableContextValue { setEditingRowId: (id: RowId | null) => void; confirmDeleteDialogOpen: boolean; setConfirmDeleteDialogOpen: Dispatch>; + confirmDeleteDialogContext: Row | null; + setConfirmDeleteDialogContext: Dispatch | null>>; + handleDeleteRow: (row: Row) => void; + startEditRow: (row: Row) => void; + startDeleteRow: (row: Row) => void; } export const TreeTableContext = createContext({ @@ -62,4 +67,9 @@ export const TreeTableContext = createContext({ setEditingRowId: () => {}, confirmDeleteDialogOpen: false, setConfirmDeleteDialogOpen: () => {}, + confirmDeleteDialogContext: null, + setConfirmDeleteDialogContext: () => {}, + handleDeleteRow: () => {}, + startEditRow: () => {}, + startDeleteRow: () => {}, }); From 6736701384c68f7732953b0bde91b80597e8004f Mon Sep 17 00:00:00 2001 From: Jesper Hodge Date: Wed, 22 Apr 2026 11:26:23 -0400 Subject: [PATCH 08/16] feat: style delete modal --- src/generic/TypeXToConfirmModal.tsx | 34 +++++++++++++++++++++----- src/generic/messages.ts | 10 ++++++++ src/taxonomy/data/api.ts | 1 + src/taxonomy/data/apiHooks.ts | 20 +++++++++++++++ src/taxonomy/tag-list/DeleteModal.tsx | 24 ++++++++++++++---- src/taxonomy/tag-list/TagListTable.tsx | 4 ++- src/taxonomy/tag-list/hooks.ts | 18 ++++++++------ src/taxonomy/tag-list/messages.ts | 18 ++++++++++---- src/taxonomy/tree-table/TableView.tsx | 1 + 9 files changed, 105 insertions(+), 25 deletions(-) create mode 100644 src/generic/messages.ts diff --git a/src/generic/TypeXToConfirmModal.tsx b/src/generic/TypeXToConfirmModal.tsx index 5ea3b48fb6..f691df13a7 100644 --- a/src/generic/TypeXToConfirmModal.tsx +++ b/src/generic/TypeXToConfirmModal.tsx @@ -1,9 +1,12 @@ import React, { useEffect } from 'react'; -import { Button, Form, ModalDialog } from '@openedx/paragon'; +import { Button, Card, Form, Icon, ModalDialog } from '@openedx/paragon'; +import { WarningFilled } from '@openedx/paragon/icons'; +import { useIntl } from '@edx/frontend-platform/i18n'; +import messages from './messages'; interface TypeXToConfirmModalProps { label: string; - bodyText: string; + bodyText: string | React.ReactNode; confirmLabel: string; cancelLabel: string; X: string; @@ -28,6 +31,7 @@ const TypeXToConfirmModal: React.FC = ({ setContext, }) => { const [confirmedByTyping, setConfirmedByTyping] = React.useState(false); + const intl = useIntl(); const handleKeyDown = (e: React.KeyboardEvent) => { if (!confirmedByTyping) { return; } @@ -77,21 +81,39 @@ const TypeXToConfirmModal: React.FC = ({ {label} -
{bodyText}
-
+ + +
+ +
{bodyText}
+
+ + +
- Type {X} to confirm + {(() => { + const messageText = intl.formatMessage(messages.typeToConfirmInstruction, { X }); + const parts = messageText.split(X); + return ( + <> + {parts[0]} + {X} + {parts[1]} + + ); + })()}
- diff --git a/src/generic/messages.ts b/src/generic/messages.ts new file mode 100644 index 0000000000..05b8ed160d --- /dev/null +++ b/src/generic/messages.ts @@ -0,0 +1,10 @@ +import { defineMessages } from '@edx/frontend-platform/i18n'; + +const messages = defineMessages({ + typeToConfirmInstruction: { + id: 'course-authoring.generic.type-to-confirm-instruction', + defaultMessage: 'Type {X} to confirm', + }, +}); + +export default messages; diff --git a/src/taxonomy/data/api.ts b/src/taxonomy/data/api.ts index f2de425ed5..4d552f9da3 100644 --- a/src/taxonomy/data/api.ts +++ b/src/taxonomy/data/api.ts @@ -99,6 +99,7 @@ export const apiUrls = { tagsPlanImport: (taxonomyId: number) => makeUrl(`${taxonomyId}/tags/import/plan/`), createTag: (taxonomyId: number) => makeUrl(`${taxonomyId}/tags/`), updateTag: (taxonomyId: number) => makeUrl(`${taxonomyId}/tags/`), + deleteTag: (taxonomyId: number) => makeUrl(`${taxonomyId}/tags/`), } satisfies Record string>; /** diff --git a/src/taxonomy/data/apiHooks.ts b/src/taxonomy/data/apiHooks.ts index b47ee23f1b..fd40a10097 100644 --- a/src/taxonomy/data/apiHooks.ts +++ b/src/taxonomy/data/apiHooks.ts @@ -269,3 +269,23 @@ export const useUpdateTag = (taxonomyId: number) => { }, }); }; + +export const useDeleteTag = (taxonomyId: number) => { + const queryClient = useQueryClient(); + + return useMutation({ + mutationFn: async ({ value, withSubtags }: { value: string; withSubtags: boolean; }) => { + const body = { tags: [value], with_subtags: withSubtags }; + await getAuthenticatedHttpClient().delete(apiUrls.deleteTag(taxonomyId), { + data: body, + }); + }, + onSuccess: () => { + queryClient.invalidateQueries({ + queryKey: taxonomyQueryKeys.taxonomyTagList(taxonomyId), + }); + // In the metadata, 'tagsCount' (and possibly other fields) will have changed: + queryClient.invalidateQueries({ queryKey: taxonomyQueryKeys.taxonomyMetadata(taxonomyId) }); + }, + }); +}; diff --git a/src/taxonomy/tag-list/DeleteModal.tsx b/src/taxonomy/tag-list/DeleteModal.tsx index 58349ca870..f33d0d2396 100644 --- a/src/taxonomy/tag-list/DeleteModal.tsx +++ b/src/taxonomy/tag-list/DeleteModal.tsx @@ -14,12 +14,12 @@ const DeleteModal = () => { setConfirmDeleteDialogOpen, confirmDeleteDialogContext, setConfirmDeleteDialogContext, - handleDeleteTag, + handleDeleteRow, } = useContext(TreeTableContext); const intl = useIntl(); const handleConfirm = (row: Row) => { - handleDeleteTag(row); + handleDeleteRow(row); setConfirmDeleteDialogOpen(false); setConfirmDeleteDialogContext(null); }; @@ -33,15 +33,29 @@ const DeleteModal = () => { const count = useMemo(() => rowData ? getTagWithDescendantsCount(rowData) : 0, [confirmDeleteDialogContext]); const hasSubtags = count > 1; - const bodyText = hasSubtags ? intl.formatMessage(messages.deleteTagWithSubtagsConfirmation, { count }) : intl.formatMessage(messages.deleteTagConfirmation); - const typeToDeleteText = hasSubtags ? intl.formatMessage(messages.typeToConfirmDeleteTagWithSubtags) : intl.formatMessage(messages.typeToConfirmDeleteOneTag); + // const bodyText = hasSubtags ? intl.formatMessage(messages.deleteTagWithSubtagsConfirmation, { count }) : intl.formatMessage(messages.deleteTagConfirmation, { count }); + const typeToDeleteText = hasSubtags ? intl.formatMessage(messages.typeToConfirmDeleteTagWithSubtags, { count }) : intl.formatMessage(messages.typeToConfirmDeleteOneTag); + const messageText = hasSubtags ? intl.formatMessage(messages.deleteTagWithSubtagsConfirmation, { count }) : intl.formatMessage(messages.deleteTagConfirmation, { count }); + const parts = messageText.split(String(count)); + const bodyText = ( + <> +
+ {parts[0]} + {count} + {parts[1]} +
+
+ {intl.formatMessage(messages.deleteTagConfirmationEmphasizedPart)} +
+ + ); return ( 1 ? messages.deleteLabelPlural : messages.deleteLabelSingular)} cancelLabel={intl.formatMessage(messages.cancelLabel)} isOpen={confirmDeleteDialogOpen} context={confirmDeleteDialogContext} diff --git a/src/taxonomy/tag-list/TagListTable.tsx b/src/taxonomy/tag-list/TagListTable.tsx index b69d99ae6e..2c7b6948e8 100644 --- a/src/taxonomy/tag-list/TagListTable.tsx +++ b/src/taxonomy/tag-list/TagListTable.tsx @@ -4,7 +4,7 @@ import React, { useEffect, } from 'react'; import type { PaginationState, Row } from '@tanstack/react-table'; -import { useTagListData, useCreateTag, useUpdateTag } from '@src/taxonomy/data/apiHooks'; +import { useTagListData, useCreateTag, useUpdateTag, useDeleteTag } from '@src/taxonomy/data/apiHooks'; import { TableView, TreeTableContext } from '@src/taxonomy/tree-table'; import type { RowId, @@ -86,6 +86,7 @@ const TagListTable = ({ taxonomyId, maxDepth }: TagListTableProps) => { }); const createTagMutation = useCreateTag(taxonomyId); const updateTagMutation = useUpdateTag(taxonomyId); + const deleteTagMutation = useDeleteTag(taxonomyId); const pageCount = tagList?.numPages ?? -1; const canAddTag = tagList?.canAddTag !== false; @@ -108,6 +109,7 @@ const TagListTable = ({ taxonomyId, maxDepth }: TagListTableProps) => { setActiveActionMenuRowId, setConfirmDeleteDialogOpen, setConfirmDeleteDialogContext, + deleteTagMutation, }, ); diff --git a/src/taxonomy/tag-list/hooks.ts b/src/taxonomy/tag-list/hooks.ts index 156fd67d63..b6cc00736e 100644 --- a/src/taxonomy/tag-list/hooks.ts +++ b/src/taxonomy/tag-list/hooks.ts @@ -3,7 +3,7 @@ import { AxiosError } from 'axios'; import { useIntl } from '@edx/frontend-platform/i18n'; import globalMessages from '@src/messages'; -import { useCreateTag, useUpdateTag } from '@src/taxonomy/data/apiHooks'; +import { useCreateTag, useDeleteTag, useUpdateTag } from '@src/taxonomy/data/apiHooks'; import type { RowId, TreeRowData } from '@src/taxonomy/tree-table/types'; import { TagTree } from './tagTree'; import { TagListTableError } from './errors'; @@ -56,6 +56,7 @@ interface UseEditActionsParams { setActiveActionMenuRowId: React.Dispatch>; setConfirmDeleteDialogOpen: React.Dispatch>; setConfirmDeleteDialogContext: React.Dispatch | null>>; + deleteTagMutation: ReturnType; } const getInlineValidationMessage = (value: string, intl: ReturnType): string => { @@ -131,6 +132,7 @@ const useEditActions = ({ exitDraftWithoutSave, setEditingRowId, updateTagMutation, + deleteTagMutation, setConfirmDeleteDialogOpen, setConfirmDeleteDialogContext, }: UseEditActionsParams) => { @@ -287,18 +289,18 @@ const useEditActions = ({ setConfirmDeleteDialogContext(row); }; - const someDeleteTagAPICall = async (value: string) => { - // Placeholder for actual delete API call - return new Promise((resolve) => setTimeout(resolve, 1000)); - }; - const handleDeleteTag = async (row: Row) => { const rowData = getTagListRowData(row); const count = getTagWithDescendantsCount(rowData); + // If the tag in the frontend state does not have subtags, + // don't allow the backend to delete subtags. + // That prevents problems in case of stale frontend state. + const shouldDeleteSubtags = count > 1; try { - // In view mode, the table reloads on change, reflecting the deletion without needing to manually update the table state + // In view mode, the table reloads on change, reflecting the deletion + // without needing to manually update the table state enterViewMode(); - await someDeleteTagAPICall(rowData.value); // Replace with actual delete API call + await deleteTagMutation.mutateAsync({ value: rowData.value, withSubtags: shouldDeleteSubtags }); setToast({ show: true, message: intl.formatMessage(messages.tagsDeleteSuccessMessage, { count }), diff --git a/src/taxonomy/tag-list/messages.ts b/src/taxonomy/tag-list/messages.ts index 20d48160a3..a34609db12 100644 --- a/src/taxonomy/tag-list/messages.ts +++ b/src/taxonomy/tag-list/messages.ts @@ -103,11 +103,15 @@ const messages = defineMessages({ }, deleteTagConfirmation: { id: 'course-authoring.tag-list.delete-tag-confirmation', - defaultMessage: 'Warning! You are about to delete a tag. If the tag is applied to course content it will be removed.', + defaultMessage: 'Warning! You are about to delete {count} tag(s).', }, - deleteLabel: { + deleteLabelPlural: { id: 'course-authoring.tag-list.delete-label', - defaultMessage: 'Delete', + defaultMessage: 'Delete Tags', + }, + deleteLabelSingular: { + id: 'course-authoring.tag-list.delete-label-singular', + defaultMessage: 'Delete Tag', }, cancelLabel: { id: 'course-authoring.tag-list.cancel-label', @@ -115,11 +119,15 @@ const messages = defineMessages({ }, typeToConfirmDeleteTagWithSubtags: { id: 'course-authoring.tag-list.delete-tag-with-subtags-type-to-confirm', - defaultMessage: 'DELETE WITH SUB-TAGS', + defaultMessage: 'DELETE ALL {count} TAGS', }, deleteTagWithSubtagsConfirmation: { id: 'course-authoring.tag-list.delete-tag-with-subtags-confirmation', - defaultMessage: 'Warning! You are about to delete a tag containing sub-tags. If you proceed, {count} sub-tags that you did not directly select will also be deleted. Any tags applied to course content will be removed.', + defaultMessage: 'Warning! You are about to delete a tag containing sub-tags. If you proceed, {count} tags will be deleted.', + }, + deleteTagConfirmationEmphasizedPart: { + id: 'course-authoring.tag-list.delete-tag-confirmation-bold-part', + defaultMessage: 'Any tags applied to course content will be removed across all assigned organizations.', }, }); diff --git a/src/taxonomy/tree-table/TableView.tsx b/src/taxonomy/tree-table/TableView.tsx index 4e53fe3c3c..c78b2f07da 100644 --- a/src/taxonomy/tree-table/TableView.tsx +++ b/src/taxonomy/tree-table/TableView.tsx @@ -18,6 +18,7 @@ import { import { ArrowDropUpDown } from '@openedx/paragon/icons'; import { useIntl } from '@edx/frontend-platform/i18n'; import TableBody from './TableBody'; +// @ts-ignore import './TableView.scss'; import messages from './messages'; import SaveErrorAlert from './SaveErrorAlert'; From cc959454967b98f29106b823613c189fbeea24f9 Mon Sep 17 00:00:00 2001 From: Jesper Hodge Date: Wed, 22 Apr 2026 11:50:42 -0400 Subject: [PATCH 09/16] feat: open alert on tag delete error --- src/taxonomy/tag-list/TagListTable.tsx | 1 + src/taxonomy/tag-list/hooks.ts | 1 + src/taxonomy/tree-table/SaveErrorAlert.tsx | 8 +++++--- src/taxonomy/tree-table/TableView.tsx | 4 +++- src/taxonomy/tree-table/TreeTableContext.tsx | 2 ++ 5 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/taxonomy/tag-list/TagListTable.tsx b/src/taxonomy/tag-list/TagListTable.tsx index 2c7b6948e8..51bfd84d7c 100644 --- a/src/taxonomy/tag-list/TagListTable.tsx +++ b/src/taxonomy/tag-list/TagListTable.tsx @@ -137,6 +137,7 @@ const TagListTable = ({ taxonomyId, maxDepth }: TagListTableProps) => { draftError, createRowMutation: createTagMutation, updateRowMutation: updateTagMutation, + deleteRowMutation: deleteTagMutation, toast, setToast, setIsCreatingTopRow: setIsCreatingTopTag, diff --git a/src/taxonomy/tag-list/hooks.ts b/src/taxonomy/tag-list/hooks.ts index b6cc00736e..7f109974dd 100644 --- a/src/taxonomy/tag-list/hooks.ts +++ b/src/taxonomy/tag-list/hooks.ts @@ -307,6 +307,7 @@ const useEditActions = ({ }); } catch (error) { const errorMessage = getErrorMessage(error); + setDraftError(errorMessage); setToast({ show: true, message: intl.formatMessage(messages.tagDeleteErrorMessage, { errorMessage }) }); } }; diff --git a/src/taxonomy/tree-table/SaveErrorAlert.tsx b/src/taxonomy/tree-table/SaveErrorAlert.tsx index ef0788140f..c283151ca7 100644 --- a/src/taxonomy/tree-table/SaveErrorAlert.tsx +++ b/src/taxonomy/tree-table/SaveErrorAlert.tsx @@ -5,6 +5,7 @@ import { import { Info } from '@openedx/paragon/icons'; import { useIntl } from '@edx/frontend-platform/i18n'; +// @ts-ignore import './TableView.scss'; import messages from './messages'; @@ -12,15 +13,16 @@ interface SaveErrorAlertProps { draftError: string | undefined; isError: boolean | undefined; isUpdateError: boolean | undefined; + isDeleteError: boolean | undefined; } -const SaveErrorAlert = ({ draftError, isError, isUpdateError }: SaveErrorAlertProps) => { +const SaveErrorAlert = ({ draftError, isError, isUpdateError, isDeleteError }: SaveErrorAlertProps) => { const intl = useIntl(); - const hasError: boolean = Boolean((isError || isUpdateError) && !!draftError); + const hasError: boolean = Boolean((isError || isUpdateError || isDeleteError) && !!draftError); const [alertOpen, setAlertOpen] = React.useState(hasError); useEffect(() => { setAlertOpen(hasError); - }, [hasError, isError, isUpdateError, draftError]); + }, [hasError, isError, isUpdateError, isDeleteError, draftError]); if (!alertOpen) { return null; } diff --git a/src/taxonomy/tree-table/TableView.tsx b/src/taxonomy/tree-table/TableView.tsx index c78b2f07da..0d3d2bda01 100644 --- a/src/taxonomy/tree-table/TableView.tsx +++ b/src/taxonomy/tree-table/TableView.tsx @@ -43,6 +43,7 @@ const TableView = ({ draftError, createRowMutation, updateRowMutation, + deleteRowMutation, handleCreateRow, toast, setToast, @@ -75,10 +76,11 @@ const TableView = ({ const { isError } = createRowMutation; const { isError: isUpdateError } = updateRowMutation; + const { isError: isDeleteError } = deleteRowMutation; return ( <> - +
diff --git a/src/taxonomy/tree-table/TreeTableContext.tsx b/src/taxonomy/tree-table/TreeTableContext.tsx index 55f1baa048..bcb085cdfa 100644 --- a/src/taxonomy/tree-table/TreeTableContext.tsx +++ b/src/taxonomy/tree-table/TreeTableContext.tsx @@ -29,6 +29,7 @@ export interface TreeTableContextValue { creatingParentId: RowId | null; setCreatingParentId: (id: RowId | null) => void; setDraftError: (error: string) => void; + deleteRowMutation: CreateRowMutationState; validate: (value: string, mode?: 'soft' | 'hard') => boolean; handleUpdateRow: (value: string, originalValue: string) => void; editingRowId: RowId | null; @@ -53,6 +54,7 @@ export const TreeTableContext = createContext({ draftError: '', createRowMutation: {}, updateRowMutation: {}, + deleteRowMutation: {}, toast: { show: false, message: '', variant: 'success' }, setToast: () => {}, setIsCreatingTopRow: () => {}, From 2ce090987b63e155f85c635c700a31f2a8918848 Mon Sep 17 00:00:00 2001 From: Jesper Hodge Date: Thu, 23 Apr 2026 18:28:21 -0400 Subject: [PATCH 10/16] test: tag list delete --- src/taxonomy/tag-list/TagListTable.test.jsx | 387 +++++++++++++++++--- 1 file changed, 330 insertions(+), 57 deletions(-) diff --git a/src/taxonomy/tag-list/TagListTable.test.jsx b/src/taxonomy/tag-list/TagListTable.test.jsx index 7d0b11c184..96587a4e2d 100644 --- a/src/taxonomy/tag-list/TagListTable.test.jsx +++ b/src/taxonomy/tag-list/TagListTable.test.jsx @@ -1,5 +1,6 @@ import React from 'react'; import { AxiosError } from 'axios'; +import userEvent from "@testing-library/user-event"; import { render, waitFor, @@ -124,6 +125,7 @@ const subTagsResponse = { const subTagsUrl = 'http://localhost:18010/api/content_tagging/v1/taxonomies/1/tags/?full_depth_threshold=10000&parent_tag=root+tag+1'; const createTagUrl = 'http://localhost:18010/api/content_tagging/v1/taxonomies/1/tags/'; +const deleteTagUrl = createTagUrl; const renderTagListTable = (maxDepth = 3) => render(); @@ -198,6 +200,46 @@ const openRenameDraftRow = async (tagName = 'root tag 1') => { }; }; +const buildTagsResponse = (results) => ({ + ...mockTagsResponse, + count: results.filter((tag) => tag.depth === 0).length, + results, +}); + +const openDeleteDialogForTag = async ({ + tagName, + actionButtonName = /actions/i, +} = {}) => { + openActionsMenuForTag(tagName, actionButtonName); + fireEvent.click(screen.getByRole("button", { name: /^Delete$/i })); + const dialog = await screen.findByRole("dialog"); + expect(within(dialog).getByText(`Delete "${tagName}"`)).toBeInTheDocument(); + const input = within(dialog).getByRole("textbox"); + const cancelButton = within(dialog).getByRole("button", { name: "Cancel" }); + const deleteButton = within(dialog).getByRole("button", { + name: /Delete Tag|Delete Tags/i, + }); + return { + dialog, + input, + cancelButton, + deleteButton, + }; +}; + +const expectDeleteRequest = async ({ tagName, withSubtags }) => { + await waitFor(() => { + expect(axiosMock.history.delete.length).toBe(1); + expect(axiosMock.history.delete[0].url).toBe(deleteTagUrl); + expect(axiosMock.history.delete[0].data).toEqual( + JSON.stringify({ + tags: [tagName], + with_subtags: withSubtags, + }), + ); + }); +}; + describe('', () => { beforeEach(async () => { ({ axiosMock } = initializeMocks({ user: adminUser })); @@ -281,24 +323,6 @@ describe('', () => { }); }); - // temporarily skipped because pagination is not implemented yet - it.skip('should render pagination footer', async () => { - axiosMock.onGet(rootTagsListUrl).reply(200, mockTagsPaginationResponse); - renderTagListTable(); - const tableFooter = await screen.findAllByRole('navigation', { - name: /table pagination/i, - }); - expect(tableFooter[0]).toBeInTheDocument(); - }); - - // temporarily skipped because pagination is not implemented yet - it.skip('should render correct number of items in pagination footer', async () => { - axiosMock.onGet(rootTagsListUrl).reply(200, mockTagsPaginationResponse); - renderTagListTable(); - const paginationButtons = await screen.findByText('Page 1 of 2'); - expect(paginationButtons).toBeInTheDocument(); - }); - describe('Create a new top-level tag', () => { it('should disable tag creation buttons if the taxonomy includes `can_add_tag: false`', async () => { axiosMock.onGet(rootTagsListUrl).reply(200, { @@ -460,38 +484,6 @@ describe('', () => { expect(temporaryRow).toBeInTheDocument(); }); - // temporarily skipped because pagination is not implemented yet - it.skip('should refresh the table and remove the temporary row when a pagination button is clicked', async () => { - axiosMock.onPost(createTagUrl).reply(201, { - ...tagDefaults, - value: 'xyz tag', - child_count: 0, - _id: 1234, - }); - const { creatingRow, input } = await openTopLevelDraftRow(); - - fireEvent.change(input, { target: { value: 'xyz tag' } }); - const saveButton = within(creatingRow).getByRole('button', { name: 'Save' }); - fireEvent.click(saveButton); - const temporaryRow = await screen.findByText('xyz tag'); - // temporaryRow should be at the top of the table, that is, the first row after the header - const rows = screen.getAllByRole('row'); - expect(rows[1]).toContainElement(temporaryRow); - - // Simulate clicking a pagination button - const paginationButton = await screen.findByRole('button', { name: 'Go to page 2' }); - fireEvent.click(paginationButton); - - await waitFor(() => { - // A get request should have refreshed the table data - expect(axiosMock.history.get.length).toBeGreaterThan(1); - const xyzTagRow = screen.queryByText('xyz tag'); - expect(xyzTagRow).toBeInTheDocument(); - // expect the row to not be the first row after the header - expect(rows[1]).not.toContainElement(xyzTagRow); - }); - }); - // a bit flaky when ran together with other tests - any way to improve this? it('should allow adding multiple tags consecutively without a page refresh', async () => { // clear axios mock history @@ -1008,25 +1000,306 @@ describe('', () => { axiosMock.resetHistory(); }); - it('should disable delete action and show tooltip if tag includes `can_delete: false`', async () => { + it("should disable delete action if tag includes `can_delete: false`", async () => { axiosMock.reset(); - axiosMock.onGet(rootTagsListUrl).reply(200, mockTagResponseDisallowingEdits); + axiosMock + .onGet(rootTagsListUrl) + .reply(200, mockTagResponseDisallowingEdits); axiosMock.onGet(subTagsUrl).reply(200, subTagsResponse); cleanup(); ({ axiosMock } = initializeMocks({ user: adminUser })); - axiosMock.onGet(rootTagsListUrl).reply(200, mockTagResponseDisallowingEdits); + axiosMock + .onGet(rootTagsListUrl) + .reply(200, mockTagResponseDisallowingEdits); axiosMock.onGet(subTagsUrl).reply(200, subTagsResponse); renderTagListTable(); await waitForRootTag(); openActionsMenuForTag(tagName); - const deleteButton = screen.getByRole('button', { name: /Delete/i }); + const deleteButton = screen.getByRole("button", { name: /Delete/i }); expect(deleteButton).toBeInTheDocument(); - expect(deleteButton).toHaveAttribute('aria-disabled', 'true'); - fireEvent.mouseOver(deleteButton); - expect(screen.getByText(/This tag does not allow deletion/i)).toBeInTheDocument(); + expect(deleteButton).toHaveAttribute("aria-disabled", "true"); + }); + }); + }); + + it('opens the delete confirmation dialog for a leaf tag from the actions menu and requires typing "DELETE" before deletion can proceed', async () => { + fireEvent.click(screen.getAllByText("Expand All")[0]); + await screen.findByText("the grandchild tag"); + + const { dialog, input, cancelButton, deleteButton } = + await openDeleteDialogForTag({ + tagName: "the grandchild tag", + actionButtonName: /more actions for tag "the grandchild tag"/i, }); + + expect(dialog).toHaveTextContent( + "Warning! You are about to delete 1 tag(s).", + ); + expect(dialog).toHaveTextContent( + "Any tags applied to course content will be removed across all assigned organizations.", + ); + expect(dialog).toHaveTextContent("Type DELETE to confirm"); + expect(cancelButton).toBeInTheDocument(); + expect(deleteButton).toHaveTextContent("Delete Tag"); + expect(deleteButton).toBeDisabled(); + + fireEvent.change(input, { target: { value: "DELETE ALL 2 TAGS" } }); + expect(deleteButton).toBeDisabled(); + + fireEvent.change(input, { target: { value: "DELETE" } }); + expect(deleteButton).toBeEnabled(); + }); + + it("opens the stronger delete confirmation dialog for a parent tag and requires the exact descendant-aware confirmation phrase before enabling delete", async () => { + fireEvent.click(screen.getAllByText("Expand All")[0]); + await screen.findByText("the child tag"); + + const { dialog, input, deleteButton } = await openDeleteDialogForTag({ + tagName: "the child tag", + actionButtonName: /more actions for tag "the child tag"/i, + }); + + expect(dialog).toHaveTextContent( + "Warning! You are about to delete a tag containing sub-tags. If you proceed, 2 tags will be deleted.", + ); + expect(dialog).toHaveTextContent("Type DELETE ALL 2 TAGS to confirm"); + expect(deleteButton).toHaveTextContent("Delete Tags"); + expect(deleteButton).toBeDisabled(); + + fireEvent.change(input, { target: { value: "DELETE" } }); + expect(deleteButton).toBeDisabled(); + + fireEvent.change(input, { target: { value: "DELETE ALL 2 TAGS" } }); + expect(deleteButton).toBeEnabled(); + }); + + it("shows the descendant deletion warning and the total recursive tag count when deleting a tag with nested subtags", async () => { + const { dialog, deleteButton } = await openDeleteDialogForTag({ + tagName: "root tag 1", + }); + + expect(dialog).toHaveTextContent( + "Warning! You are about to delete a tag containing sub-tags. If you proceed, 3 tags will be deleted.", + ); + expect(dialog).toHaveTextContent("Type DELETE ALL 3 TAGS to confirm"); + expect(deleteButton).toHaveTextContent("Delete Tags"); + }); + + it("deletes a leaf tag after typed confirmation and shows the success toast describing that tagged content will be updated", async () => { + axiosMock.reset(); + axiosMock.onDelete(deleteTagUrl).reply(204); + axiosMock + .onGet(rootTagsListUrl) + .reply( + 200, + buildTagsResponse( + mockTagsResponse.results.filter( + (tag) => tag.value !== "root tag 2", + ), + ), + ); + + expect(screen.queryByText("root tag 2")).toBeInTheDocument(); + + const { input, deleteButton } = await openDeleteDialogForTag({ + tagName: "root tag 2", + }); + fireEvent.change(input, { target: { value: "DELETE" } }); + fireEvent.click(deleteButton); + + await expectDeleteRequest({ tagName: "root tag 2", withSubtags: false }); + expect( + await screen.findByText( + "1 tag(s) deleted. This change will be applied across all tagged content.", + ), + ).toBeInTheDocument(); + await waitFor(() => { + expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); + expect(screen.queryByText("root tag 2")).not.toBeInTheDocument(); + }); + }); + + it("deletes a parent tag together with all rendered descendants after typed confirmation and shows the success toast with the total deleted count", async () => { + fireEvent.click(screen.getAllByText("Expand All")[0]); + await screen.findByText("the child tag"); + await screen.findByText("the grandchild tag"); + + axiosMock.reset(); + axiosMock.onDelete(deleteTagUrl).reply(204); + axiosMock + .onGet(rootTagsListUrl) + .reply( + 200, + buildTagsResponse( + mockTagsResponse.results.filter( + (tag) => + !["root tag 1", "the child tag", "the grandchild tag"].includes( + tag.value, + ), + ), + ), + ); + + const { input, deleteButton } = await openDeleteDialogForTag({ + tagName: "root tag 1", + }); + fireEvent.change(input, { target: { value: "DELETE ALL 3 TAGS" } }); + fireEvent.click(deleteButton); + + await expectDeleteRequest({ tagName: "root tag 1", withSubtags: true }); + expect( + await screen.findByText( + "3 tag(s) deleted. This change will be applied across all tagged content.", + ), + ).toBeInTheDocument(); + await waitFor(() => { + expect(screen.queryByText("root tag 1")).not.toBeInTheDocument(); + expect(screen.queryByText("the child tag")).not.toBeInTheDocument(); + expect( + screen.queryByText("the grandchild tag"), + ).not.toBeInTheDocument(); + expect(screen.getByText("root tag 2")).toBeInTheDocument(); + }); + }); + + it("does not issue a delete request when the dialog is canceled and leaves the table unchanged", async () => { + const { cancelButton } = await openDeleteDialogForTag({ + tagName: "root tag 1", + }); + fireEvent.click(cancelButton); + + await waitFor(() => { + expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); + }); + expect(axiosMock.history.delete.length).toBe(0); + expect(screen.getByText("root tag 1")).toBeInTheDocument(); + }); + + it("does not keep a previous typed confirmation when the delete dialog is closed and reopened for the same tag", async () => { + const firstOpen = await openDeleteDialogForTag({ tagName: "root tag 1" }); + fireEvent.change(firstOpen.input, { + target: { value: "DELETE ALL 3 TAGS" }, + }); + expect(firstOpen.deleteButton).toBeEnabled(); + fireEvent.click(firstOpen.cancelButton); + + await waitFor(() => { + expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); + }); + + const secondOpen = await openDeleteDialogForTag({ + tagName: "root tag 1", + }); + expect(secondOpen.input.value).toEqual(""); + expect(secondOpen.deleteButton).toBeDisabled(); + }); + + it("completes the delete workflow with keyboard only by opening the menu, selecting Delete, typing the confirmation phrase, and pressing Enter", async () => { + const user = userEvent.setup(); + axiosMock.reset(); + axiosMock.onDelete(deleteTagUrl).reply(204); + axiosMock + .onGet(rootTagsListUrl) + .reply( + 200, + buildTagsResponse( + mockTagsResponse.results.filter( + (tag) => tag.value !== "root tag 2", + ), + ), + ); + + const row = screen.getByText("root tag 2").closest("tr"); + const actionsButton = within(row).getByRole("button", { + name: /more actions for tag "root tag 2"/i, + }); + actionsButton.focus(); + expect(actionsButton).toHaveFocus(); + + await user.keyboard("{Enter}"); + + const deleteMenuItem = await screen.findByRole("button", { + name: /^Delete$/i, + }); + deleteMenuItem.focus(); + expect(deleteMenuItem).toHaveFocus(); + await user.keyboard("{Enter}"); + + const dialog = await screen.findByRole("dialog"); + const input = within(dialog).getByRole("textbox"); + await user.type(input, "DELETE"); + await user.keyboard("{Enter}"); + + await expectDeleteRequest({ tagName: "root tag 2", withSubtags: false }); + expect( + await screen.findByText( + "1 tag(s) deleted. This change will be applied across all tagged content.", + ), + ).toBeInTheDocument(); + await waitFor(() => { + expect(screen.queryByText("root tag 2")).not.toBeInTheDocument(); + }); + }); + + it("cancels the delete workflow with keyboard only by pressing Escape and does not send a delete request", async () => { + const user = userEvent.setup(); + const row = screen.getByText("root tag 1").closest("tr"); + const actionsButton = within(row).getByRole("button", { + name: /more actions for tag "root tag 1"/i, + }); + actionsButton.focus(); + expect(actionsButton).toHaveFocus(); + + await user.keyboard("{Enter}"); + + const deleteMenuItem = await screen.findByRole("button", { + name: /^Delete$/i, + }); + deleteMenuItem.focus(); + expect(deleteMenuItem).toHaveFocus(); + await user.keyboard("{Enter}"); + + const dialog = await screen.findByRole("dialog"); + expect( + within(dialog).getByText('Delete "root tag 1"'), + ).toBeInTheDocument(); + + await user.keyboard("{Escape}"); + + await waitFor(() => { + expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); + }); + expect(axiosMock.history.delete.length).toBe(0); + expect(screen.getByText("root tag 1")).toBeInTheDocument(); + }); + + it("surfaces a failed delete through both the persistent error alert and the delete-error toast while leaving the row visible", async () => { + axiosMock.reset(); + axiosMock.onDelete(deleteTagUrl).reply(() => { + const error = new AxiosError("Request failed with status code 500"); + error.response = { + data: { + value: ["Delete failed"], + }, + }; + return Promise.reject(error); + }); + + const { input, deleteButton } = await openDeleteDialogForTag({ + tagName: "root tag 1", }); + fireEvent.change(input, { target: { value: "DELETE ALL 3 TAGS" } }); + fireEvent.click(deleteButton); + + await expectDeleteRequest({ tagName: "root tag 1", withSubtags: true }); + expect( + await screen.findByText("Error saving changes"), + ).toBeInTheDocument(); + expect( + await screen.findByText("Error deleting tag: Delete failed"), + ).toBeInTheDocument(); + expect(screen.getByText("root tag 1")).toBeInTheDocument(); }); }); }); From 40475458deda2a86d79e0a0d89495da7032ecd1d Mon Sep 17 00:00:00 2001 From: Jesper Hodge Date: Thu, 23 Apr 2026 18:31:49 -0400 Subject: [PATCH 11/16] test: tag list delete --- src/generic/TypeXToConfirmModal.test.tsx | 105 ++++++++++++++ src/taxonomy/tag-list/DeleteModal.test.tsx | 160 +++++++++++++++++++++ src/taxonomy/tag-list/utils.test.ts | 33 +++++ src/taxonomy/tree-table/TableView.test.tsx | 43 ++++++ 4 files changed, 341 insertions(+) create mode 100644 src/generic/TypeXToConfirmModal.test.tsx create mode 100644 src/taxonomy/tag-list/DeleteModal.test.tsx create mode 100644 src/taxonomy/tag-list/utils.test.ts diff --git a/src/generic/TypeXToConfirmModal.test.tsx b/src/generic/TypeXToConfirmModal.test.tsx new file mode 100644 index 0000000000..025232bdeb --- /dev/null +++ b/src/generic/TypeXToConfirmModal.test.tsx @@ -0,0 +1,105 @@ +import React from 'react'; +import userEvent from '@testing-library/user-event'; +import { IntlProvider } from '@edx/frontend-platform/i18n'; +import { fireEvent, render, screen } from '@testing-library/react'; + +import TypeXToConfirmModal from './TypeXToConfirmModal'; + +const defaultProps = () => ({ + label: 'Delete item', + bodyText: 'Dangerous action', + confirmLabel: 'Delete', + cancelLabel: 'Cancel', + X: 'DELETE', + context: { id: 7 }, + isOpen: true, + onConfirm: jest.fn(), + onCancel: jest.fn(), + setContext: jest.fn(), +}); + +const renderModal = (props = defaultProps()) => render( + + + , +); + +describe('TypeXToConfirmModal', () => { + it('keeps the destructive confirm button disabled until the typed value exactly matches the required confirmation phrase', () => { + renderModal(); + + const input = screen.getByRole('textbox'); + const confirmButton = screen.getByRole('button', { name: 'Delete' }); + + expect(confirmButton).toBeDisabled(); + fireEvent.change(input, { target: { value: 'DEL' } }); + expect(confirmButton).toBeDisabled(); + fireEvent.change(input, { target: { value: 'DELETE' } }); + expect(confirmButton).toBeEnabled(); + }); + + it('does not enable confirmation for partial, differently cased, or whitespace-padded confirmation text', () => { + renderModal(); + + const input = screen.getByRole('textbox'); + const confirmButton = screen.getByRole('button', { name: 'Delete' }); + + ['DEL', 'delete', ' DELETE', 'DELETE ', ' Delete '].forEach(value => { + fireEvent.change(input, { target: { value } }); + expect(confirmButton).toBeDisabled(); + }); + }); + + it('submits on Enter only after the exact confirmation phrase has been entered', async () => { + const user = userEvent.setup(); + const props = defaultProps(); + renderModal(props); + + const input = screen.getByRole('textbox'); + + await user.click(input); + await user.keyboard('{Enter}'); + expect(props.onConfirm).not.toHaveBeenCalled(); + + await user.type(input, 'DELETE'); + await user.keyboard('{Enter}'); + expect(props.onConfirm).toHaveBeenCalledWith(props.context); + }); + + it('resets confirmation state when the modal closes so a reopened dialog starts disabled again', async () => { + const user = userEvent.setup(); + const props = defaultProps(); + const { rerender } = renderModal(props); + + await user.type(screen.getByRole('textbox'), 'DELETE'); + expect(screen.getByRole('button', { name: 'Delete' })).toBeEnabled(); + + rerender( + + + , + ); + + rerender( + + + , + ); + + expect(screen.getByRole('button', { name: 'Delete' })).toBeDisabled(); + }); + + it('clears the provided context when the modal is closed without confirming', () => { + const props = defaultProps(); + const { rerender } = renderModal(props); + + rerender( + + + , + ); + + expect(props.setContext).toHaveBeenCalledWith(null); + expect(props.onConfirm).not.toHaveBeenCalled(); + }); +}); diff --git a/src/taxonomy/tag-list/DeleteModal.test.tsx b/src/taxonomy/tag-list/DeleteModal.test.tsx new file mode 100644 index 0000000000..e53fa7ea6b --- /dev/null +++ b/src/taxonomy/tag-list/DeleteModal.test.tsx @@ -0,0 +1,160 @@ +import React from 'react'; +import userEvent from '@testing-library/user-event'; +import { IntlProvider } from '@edx/frontend-platform/i18n'; +import { render, screen, within } from '@testing-library/react'; + +import { TreeTableContext } from '@src/taxonomy/tree-table'; +import DeleteModal from './DeleteModal'; + +const createRow = (rowData) => ({ + id: String(rowData.id), + original: rowData, +}) as any; + +const leafRowData = { + id: 101, + value: 'leaf tag', + depth: 0, + childCount: 0, + subRows: [], +}; + +const nestedRowData = { + id: 201, + value: 'parent tag', + depth: 0, + childCount: 1, + subRows: [ + { + id: 202, + value: 'child tag', + depth: 1, + childCount: 1, + subRows: [ + { + id: 203, + value: 'grandchild tag', + depth: 2, + childCount: 0, + subRows: [], + }, + ], + }, + ], +}; + +const baseContextValue = (overrides = {}) => ({ + treeData: [], + columns: [], + pageCount: -1, + pagination: { pageIndex: 0, pageSize: 10 }, + handlePaginationChange: jest.fn(), + isLoading: false, + isCreatingTopRow: false, + draftError: '', + createRowMutation: {}, + updateRowMutation: {}, + deleteRowMutation: {}, + 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(), + confirmDeleteDialogOpen: true, + setConfirmDeleteDialogOpen: jest.fn(), + confirmDeleteDialogContext: createRow(leafRowData), + setConfirmDeleteDialogContext: jest.fn(), + handleDeleteRow: jest.fn(), + startEditRow: jest.fn(), + startDeleteRow: jest.fn(), + ...overrides, +}); + +const renderDeleteModal = (contextValue = baseContextValue()) => render( + + + + + , +); + +describe('DeleteModal', () => { + it('renders a singular delete title and "Delete Tag" action label when the selected row has no descendants', () => { + renderDeleteModal(); + + const dialog = screen.getByRole('dialog'); + expect(dialog).toHaveTextContent('Delete "leaf tag"'); + expect(dialog).toHaveTextContent('Warning! You are about to delete 1 tag(s).'); + expect(dialog).toHaveTextContent('Type DELETE to confirm'); + expect(within(dialog).getByRole('button', { name: 'Delete Tag' })).toBeDisabled(); + expect(within(dialog).getByRole('button', { name: 'Cancel' })).toBeInTheDocument(); + }); + + it('renders a plural delete action label and descendant warning copy when the selected row has one or more descendants', () => { + renderDeleteModal(baseContextValue({ confirmDeleteDialogContext: createRow(nestedRowData) })); + + const dialog = screen.getByRole('dialog'); + expect(dialog).toHaveTextContent('Delete "parent tag"'); + expect(dialog).toHaveTextContent('Warning! You are about to delete a tag containing sub-tags.'); + expect(dialog).toHaveTextContent('If you proceed, 3 tags will be deleted.'); + expect(dialog).toHaveTextContent('Type DELETE ALL 3 TAGS to confirm'); + expect(within(dialog).getByRole('button', { name: 'Delete Tags' })).toBeDisabled(); + }); + + it('computes the required confirmation phrase from the recursive descendant count instead of only the immediate child count', () => { + renderDeleteModal(baseContextValue({ confirmDeleteDialogContext: createRow(nestedRowData) })); + + const dialog = screen.getByRole('dialog'); + expect(dialog).toHaveTextContent('If you proceed, 3 tags will be deleted.'); + expect(dialog).toHaveTextContent('Type DELETE ALL 3 TAGS to confirm'); + expect(dialog).not.toHaveTextContent('DELETE ALL 2 TAGS'); + }); + + it('calls handleDeleteRow with the dialog row context and then closes and clears the dialog state on confirm', async () => { + const user = userEvent.setup(); + const row = createRow(leafRowData); + const handleDeleteRow = jest.fn(); + const setConfirmDeleteDialogOpen = jest.fn(); + const setConfirmDeleteDialogContext = jest.fn(); + + renderDeleteModal(baseContextValue({ + confirmDeleteDialogContext: row, + handleDeleteRow, + setConfirmDeleteDialogOpen, + setConfirmDeleteDialogContext, + })); + + await user.type(screen.getByRole('textbox'), 'DELETE'); + await user.click(screen.getByRole('button', { name: 'Delete Tag' })); + + expect(handleDeleteRow).toHaveBeenCalledWith(row); + expect(setConfirmDeleteDialogOpen).toHaveBeenCalledWith(false); + expect(setConfirmDeleteDialogContext).toHaveBeenCalledWith(null); + }); + + it('closes and clears the dialog context on cancel without invoking deletion', async () => { + const user = userEvent.setup(); + const handleDeleteRow = jest.fn(); + const setConfirmDeleteDialogOpen = jest.fn(); + const setConfirmDeleteDialogContext = jest.fn(); + + renderDeleteModal(baseContextValue({ + handleDeleteRow, + setConfirmDeleteDialogOpen, + setConfirmDeleteDialogContext, + })); + + await user.click(screen.getByRole('button', { name: 'Cancel' })); + + expect(handleDeleteRow).not.toHaveBeenCalled(); + expect(setConfirmDeleteDialogOpen).toHaveBeenCalledWith(false); + expect(setConfirmDeleteDialogContext).toHaveBeenCalledWith(null); + }); +}); \ No newline at end of file diff --git a/src/taxonomy/tag-list/utils.test.ts b/src/taxonomy/tag-list/utils.test.ts new file mode 100644 index 0000000000..923016d60a --- /dev/null +++ b/src/taxonomy/tag-list/utils.test.ts @@ -0,0 +1,33 @@ +import { getTagWithDescendantsCount } from './utils'; + +describe('getTagsWithDescendantCount', () => { + it('returns 1 for a leaf tag', () => { + expect(getTagWithDescendantsCount({ value: 'leaf', subRows: [] } as any)).toBe(1); + }); + + it('counts all descendants across multiple nested levels', () => { + const rowData = { + value: 'root', + subRows: [ + { + value: 'child 1', + subRows: [ + { value: 'grandchild 1', subRows: [] }, + { value: 'grandchild 2', subRows: [] }, + ], + }, + { + value: 'child 2', + subRows: [ + { + value: 'grandchild 3', + subRows: [{ value: 'great grandchild 1', subRows: [] }], + }, + ], + }, + ], + } as any; + + expect(getTagWithDescendantsCount(rowData)).toBe(7); + }); +}); diff --git a/src/taxonomy/tree-table/TableView.test.tsx b/src/taxonomy/tree-table/TableView.test.tsx index d4467e483f..4682ee1ae3 100644 --- a/src/taxonomy/tree-table/TableView.test.tsx +++ b/src/taxonomy/tree-table/TableView.test.tsx @@ -31,6 +31,7 @@ const baseContextValue = () => ({ draftError: '', createRowMutation: { isPending: false, isError: false }, updateRowMutation: { isPending: false, isError: false }, + deleteRowMutation: { isPending: false, isError: false }, toast: { show: false, message: '', variant: 'success' }, setToast: jest.fn(), setIsCreatingTopRow: jest.fn(), @@ -43,6 +44,13 @@ const baseContextValue = () => ({ handleUpdateRow: jest.fn(), editingRowId: null, setEditingRowId: jest.fn(), + confirmDeleteDialogOpen: false, + setConfirmDeleteDialogOpen: jest.fn(), + confirmDeleteDialogContext: null, + setConfirmDeleteDialogContext: jest.fn(), + handleDeleteRow: jest.fn(), + startEditRow: jest.fn(), + startDeleteRow: jest.fn(), }); const renderTableView = ( @@ -107,4 +115,39 @@ describe('TableView', () => { variant: 'success', }); }); + + it('shows the save error alert when a delete mutation fails and draftError is present', () => { + const contextValue = baseContextValue(); + contextValue.deleteRowMutation = { isPending: false, isError: true }; + contextValue.draftError = 'Delete request failed'; + + renderTableView(contextValue); + + expect(screen.getByText('Error saving changes')).toBeInTheDocument(); + expect(screen.getByText('Delete request failed. Please try again.')).toBeInTheDocument(); + }); + + it('reopens the save error alert when a new delete error arrives after the user previously dismissed it', () => { + const contextValue = baseContextValue(); + contextValue.deleteRowMutation = { isPending: false, isError: true }; + contextValue.draftError = 'First delete failure'; + + const { rerender } = renderTableView(contextValue); + + fireEvent.click(screen.getByRole('button', { name: /dismiss/i })); + expect(screen.queryByText('Error saving changes')).not.toBeInTheDocument(); + + const nextContextValue = baseContextValue(); + nextContextValue.deleteRowMutation = { isPending: false, isError: true }; + nextContextValue.draftError = 'Second delete failure'; + + rerender( + + + , + ); + + expect(screen.getByText('Error saving changes')).toBeInTheDocument(); + expect(screen.getByText('Second delete failure. Please try again.')).toBeInTheDocument(); + }); }); From 5756e5304965ae269152b6f0ce02bd12e247bc5f Mon Sep 17 00:00:00 2001 From: Jesper Hodge Date: Thu, 23 Apr 2026 18:37:45 -0400 Subject: [PATCH 12/16] fix: lint --- src/generic/TypeXToConfirmModal.test.tsx | 11 +- src/taxonomy/tag-list/DeleteModal.test.tsx | 26 +-- src/taxonomy/tag-list/DeleteModal.tsx | 8 +- src/taxonomy/tag-list/TagListTable.test.jsx | 212 ++++++++++---------- src/taxonomy/tag-list/TagListTable.tsx | 3 +- src/taxonomy/tag-list/hooks.ts | 1 - src/taxonomy/tree-table/TableView.tsx | 7 +- 7 files changed, 134 insertions(+), 134 deletions(-) diff --git a/src/generic/TypeXToConfirmModal.test.tsx b/src/generic/TypeXToConfirmModal.test.tsx index 025232bdeb..1d90d5d588 100644 --- a/src/generic/TypeXToConfirmModal.test.tsx +++ b/src/generic/TypeXToConfirmModal.test.tsx @@ -18,11 +18,12 @@ const defaultProps = () => ({ setContext: jest.fn(), }); -const renderModal = (props = defaultProps()) => render( - - - , -); +const renderModal = (props = defaultProps()) => + render( + + + , + ); describe('TypeXToConfirmModal', () => { it('keeps the destructive confirm button disabled until the typed value exactly matches the required confirmation phrase', () => { diff --git a/src/taxonomy/tag-list/DeleteModal.test.tsx b/src/taxonomy/tag-list/DeleteModal.test.tsx index e53fa7ea6b..c810435553 100644 --- a/src/taxonomy/tag-list/DeleteModal.test.tsx +++ b/src/taxonomy/tag-list/DeleteModal.test.tsx @@ -6,10 +6,11 @@ import { render, screen, within } from '@testing-library/react'; import { TreeTableContext } from '@src/taxonomy/tree-table'; import DeleteModal from './DeleteModal'; -const createRow = (rowData) => ({ - id: String(rowData.id), - original: rowData, -}) as any; +const createRow = (rowData) => + ({ + id: String(rowData.id), + original: rowData, + }) as any; const leafRowData = { id: 101, @@ -77,13 +78,14 @@ const baseContextValue = (overrides = {}) => ({ ...overrides, }); -const renderDeleteModal = (contextValue = baseContextValue()) => render( - - - - - , -); +const renderDeleteModal = (contextValue = baseContextValue()) => + render( + + + + + , + ); describe('DeleteModal', () => { it('renders a singular delete title and "Delete Tag" action label when the selected row has no descendants', () => { @@ -157,4 +159,4 @@ describe('DeleteModal', () => { expect(setConfirmDeleteDialogOpen).toHaveBeenCalledWith(false); expect(setConfirmDeleteDialogContext).toHaveBeenCalledWith(null); }); -}); \ No newline at end of file +}); diff --git a/src/taxonomy/tag-list/DeleteModal.tsx b/src/taxonomy/tag-list/DeleteModal.tsx index f33d0d2396..c0d59b2833 100644 --- a/src/taxonomy/tag-list/DeleteModal.tsx +++ b/src/taxonomy/tag-list/DeleteModal.tsx @@ -34,8 +34,12 @@ const DeleteModal = () => { const hasSubtags = count > 1; // const bodyText = hasSubtags ? intl.formatMessage(messages.deleteTagWithSubtagsConfirmation, { count }) : intl.formatMessage(messages.deleteTagConfirmation, { count }); - const typeToDeleteText = hasSubtags ? intl.formatMessage(messages.typeToConfirmDeleteTagWithSubtags, { count }) : intl.formatMessage(messages.typeToConfirmDeleteOneTag); - const messageText = hasSubtags ? intl.formatMessage(messages.deleteTagWithSubtagsConfirmation, { count }) : intl.formatMessage(messages.deleteTagConfirmation, { count }); + const typeToDeleteText = hasSubtags + ? intl.formatMessage(messages.typeToConfirmDeleteTagWithSubtags, { count }) + : intl.formatMessage(messages.typeToConfirmDeleteOneTag); + const messageText = hasSubtags + ? intl.formatMessage(messages.deleteTagWithSubtagsConfirmation, { count }) + : intl.formatMessage(messages.deleteTagConfirmation, { count }); const parts = messageText.split(String(count)); const bodyText = ( <> diff --git a/src/taxonomy/tag-list/TagListTable.test.jsx b/src/taxonomy/tag-list/TagListTable.test.jsx index 96587a4e2d..717dc8537f 100644 --- a/src/taxonomy/tag-list/TagListTable.test.jsx +++ b/src/taxonomy/tag-list/TagListTable.test.jsx @@ -1,6 +1,6 @@ import React from 'react'; import { AxiosError } from 'axios'; -import userEvent from "@testing-library/user-event"; +import userEvent from '@testing-library/user-event'; import { render, waitFor, @@ -87,21 +87,12 @@ const mockTagsResponse = { const mockTagResponseDisallowingEdits = { ...mockTagsResponse, - results: mockTagsResponse.results.map(tag => ({ + results: mockTagsResponse.results.map((tag) => ({ ...tag, can_change_tag: false, can_delete_tag: false, })), }; -const mockTagsPaginationResponse = { - next: null, - previous: null, - count: 103, - num_pages: 2, - current_page: 1, - start: 0, - results: [], -}; const rootTagsListUrl = 'http://localhost:18010/api/content_tagging/v1/taxonomies/1/tags/?full_depth_threshold=10000&include_counts=true'; const subTagsResponse = { @@ -211,12 +202,12 @@ const openDeleteDialogForTag = async ({ actionButtonName = /actions/i, } = {}) => { openActionsMenuForTag(tagName, actionButtonName); - fireEvent.click(screen.getByRole("button", { name: /^Delete$/i })); - const dialog = await screen.findByRole("dialog"); + fireEvent.click(screen.getByRole('button', { name: /^Delete$/i })); + const dialog = await screen.findByRole('dialog'); expect(within(dialog).getByText(`Delete "${tagName}"`)).toBeInTheDocument(); - const input = within(dialog).getByRole("textbox"); - const cancelButton = within(dialog).getByRole("button", { name: "Cancel" }); - const deleteButton = within(dialog).getByRole("button", { + const input = within(dialog).getByRole('textbox'); + const cancelButton = within(dialog).getByRole('button', { name: 'Cancel' }); + const deleteButton = within(dialog).getByRole('button', { name: /Delete Tag|Delete Tags/i, }); return { @@ -1000,7 +991,7 @@ describe('', () => { axiosMock.resetHistory(); }); - it("should disable delete action if tag includes `can_delete: false`", async () => { + it('should disable delete action if tag includes `can_delete: false`', async () => { axiosMock.reset(); axiosMock .onGet(rootTagsListUrl) @@ -1016,77 +1007,76 @@ describe('', () => { await waitForRootTag(); openActionsMenuForTag(tagName); - const deleteButton = screen.getByRole("button", { name: /Delete/i }); + const deleteButton = screen.getByRole('button', { name: /Delete/i }); expect(deleteButton).toBeInTheDocument(); - expect(deleteButton).toHaveAttribute("aria-disabled", "true"); + expect(deleteButton).toHaveAttribute('aria-disabled', 'true'); }); }); }); it('opens the delete confirmation dialog for a leaf tag from the actions menu and requires typing "DELETE" before deletion can proceed', async () => { - fireEvent.click(screen.getAllByText("Expand All")[0]); - await screen.findByText("the grandchild tag"); + fireEvent.click(screen.getAllByText('Expand All')[0]); + await screen.findByText('the grandchild tag'); - const { dialog, input, cancelButton, deleteButton } = - await openDeleteDialogForTag({ - tagName: "the grandchild tag", - actionButtonName: /more actions for tag "the grandchild tag"/i, - }); + const { dialog, input, cancelButton, deleteButton } = await openDeleteDialogForTag({ + tagName: 'the grandchild tag', + actionButtonName: /more actions for tag "the grandchild tag"/i, + }); expect(dialog).toHaveTextContent( - "Warning! You are about to delete 1 tag(s).", + 'Warning! You are about to delete 1 tag(s).', ); expect(dialog).toHaveTextContent( - "Any tags applied to course content will be removed across all assigned organizations.", + 'Any tags applied to course content will be removed across all assigned organizations.', ); - expect(dialog).toHaveTextContent("Type DELETE to confirm"); + expect(dialog).toHaveTextContent('Type DELETE to confirm'); expect(cancelButton).toBeInTheDocument(); - expect(deleteButton).toHaveTextContent("Delete Tag"); + expect(deleteButton).toHaveTextContent('Delete Tag'); expect(deleteButton).toBeDisabled(); - fireEvent.change(input, { target: { value: "DELETE ALL 2 TAGS" } }); + fireEvent.change(input, { target: { value: 'DELETE ALL 2 TAGS' } }); expect(deleteButton).toBeDisabled(); - fireEvent.change(input, { target: { value: "DELETE" } }); + fireEvent.change(input, { target: { value: 'DELETE' } }); expect(deleteButton).toBeEnabled(); }); - it("opens the stronger delete confirmation dialog for a parent tag and requires the exact descendant-aware confirmation phrase before enabling delete", async () => { - fireEvent.click(screen.getAllByText("Expand All")[0]); - await screen.findByText("the child tag"); + it('opens the stronger delete confirmation dialog for a parent tag and requires the exact descendant-aware confirmation phrase before enabling delete', async () => { + fireEvent.click(screen.getAllByText('Expand All')[0]); + await screen.findByText('the child tag'); const { dialog, input, deleteButton } = await openDeleteDialogForTag({ - tagName: "the child tag", + tagName: 'the child tag', actionButtonName: /more actions for tag "the child tag"/i, }); expect(dialog).toHaveTextContent( - "Warning! You are about to delete a tag containing sub-tags. If you proceed, 2 tags will be deleted.", + 'Warning! You are about to delete a tag containing sub-tags. If you proceed, 2 tags will be deleted.', ); - expect(dialog).toHaveTextContent("Type DELETE ALL 2 TAGS to confirm"); - expect(deleteButton).toHaveTextContent("Delete Tags"); + expect(dialog).toHaveTextContent('Type DELETE ALL 2 TAGS to confirm'); + expect(deleteButton).toHaveTextContent('Delete Tags'); expect(deleteButton).toBeDisabled(); - fireEvent.change(input, { target: { value: "DELETE" } }); + fireEvent.change(input, { target: { value: 'DELETE' } }); expect(deleteButton).toBeDisabled(); - fireEvent.change(input, { target: { value: "DELETE ALL 2 TAGS" } }); + fireEvent.change(input, { target: { value: 'DELETE ALL 2 TAGS' } }); expect(deleteButton).toBeEnabled(); }); - it("shows the descendant deletion warning and the total recursive tag count when deleting a tag with nested subtags", async () => { + it('shows the descendant deletion warning and the total recursive tag count when deleting a tag with nested subtags', async () => { const { dialog, deleteButton } = await openDeleteDialogForTag({ - tagName: "root tag 1", + tagName: 'root tag 1', }); expect(dialog).toHaveTextContent( - "Warning! You are about to delete a tag containing sub-tags. If you proceed, 3 tags will be deleted.", + 'Warning! You are about to delete a tag containing sub-tags. If you proceed, 3 tags will be deleted.', ); - expect(dialog).toHaveTextContent("Type DELETE ALL 3 TAGS to confirm"); - expect(deleteButton).toHaveTextContent("Delete Tags"); + expect(dialog).toHaveTextContent('Type DELETE ALL 3 TAGS to confirm'); + expect(deleteButton).toHaveTextContent('Delete Tags'); }); - it("deletes a leaf tag after typed confirmation and shows the success toast describing that tagged content will be updated", async () => { + it('deletes a leaf tag after typed confirmation and shows the success toast describing that tagged content will be updated', async () => { axiosMock.reset(); axiosMock.onDelete(deleteTagUrl).reply(204); axiosMock @@ -1095,35 +1085,35 @@ describe('', () => { 200, buildTagsResponse( mockTagsResponse.results.filter( - (tag) => tag.value !== "root tag 2", + (tag) => tag.value !== 'root tag 2', ), ), ); - expect(screen.queryByText("root tag 2")).toBeInTheDocument(); + expect(screen.queryByText('root tag 2')).toBeInTheDocument(); const { input, deleteButton } = await openDeleteDialogForTag({ - tagName: "root tag 2", + tagName: 'root tag 2', }); - fireEvent.change(input, { target: { value: "DELETE" } }); + fireEvent.change(input, { target: { value: 'DELETE' } }); fireEvent.click(deleteButton); - await expectDeleteRequest({ tagName: "root tag 2", withSubtags: false }); + await expectDeleteRequest({ tagName: 'root tag 2', withSubtags: false }); expect( await screen.findByText( - "1 tag(s) deleted. This change will be applied across all tagged content.", + '1 tag(s) deleted. This change will be applied across all tagged content.', ), ).toBeInTheDocument(); await waitFor(() => { - expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); - expect(screen.queryByText("root tag 2")).not.toBeInTheDocument(); + expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); + expect(screen.queryByText('root tag 2')).not.toBeInTheDocument(); }); }); - it("deletes a parent tag together with all rendered descendants after typed confirmation and shows the success toast with the total deleted count", async () => { - fireEvent.click(screen.getAllByText("Expand All")[0]); - await screen.findByText("the child tag"); - await screen.findByText("the grandchild tag"); + it('deletes a parent tag together with all rendered descendants after typed confirmation and shows the success toast with the total deleted count', async () => { + fireEvent.click(screen.getAllByText('Expand All')[0]); + await screen.findByText('the child tag'); + await screen.findByText('the grandchild tag'); axiosMock.reset(); axiosMock.onDelete(deleteTagUrl).reply(204); @@ -1134,7 +1124,7 @@ describe('', () => { buildTagsResponse( mockTagsResponse.results.filter( (tag) => - !["root tag 1", "the child tag", "the grandchild tag"].includes( + !['root tag 1', 'the child tag', 'the grandchild tag'].includes( tag.value, ), ), @@ -1142,60 +1132,60 @@ describe('', () => { ); const { input, deleteButton } = await openDeleteDialogForTag({ - tagName: "root tag 1", + tagName: 'root tag 1', }); - fireEvent.change(input, { target: { value: "DELETE ALL 3 TAGS" } }); + fireEvent.change(input, { target: { value: 'DELETE ALL 3 TAGS' } }); fireEvent.click(deleteButton); - await expectDeleteRequest({ tagName: "root tag 1", withSubtags: true }); + await expectDeleteRequest({ tagName: 'root tag 1', withSubtags: true }); expect( await screen.findByText( - "3 tag(s) deleted. This change will be applied across all tagged content.", + '3 tag(s) deleted. This change will be applied across all tagged content.', ), ).toBeInTheDocument(); await waitFor(() => { - expect(screen.queryByText("root tag 1")).not.toBeInTheDocument(); - expect(screen.queryByText("the child tag")).not.toBeInTheDocument(); + expect(screen.queryByText('root tag 1')).not.toBeInTheDocument(); + expect(screen.queryByText('the child tag')).not.toBeInTheDocument(); expect( - screen.queryByText("the grandchild tag"), + screen.queryByText('the grandchild tag'), ).not.toBeInTheDocument(); - expect(screen.getByText("root tag 2")).toBeInTheDocument(); + expect(screen.getByText('root tag 2')).toBeInTheDocument(); }); }); - it("does not issue a delete request when the dialog is canceled and leaves the table unchanged", async () => { + it('does not issue a delete request when the dialog is canceled and leaves the table unchanged', async () => { const { cancelButton } = await openDeleteDialogForTag({ - tagName: "root tag 1", + tagName: 'root tag 1', }); fireEvent.click(cancelButton); await waitFor(() => { - expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); + expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); }); expect(axiosMock.history.delete.length).toBe(0); - expect(screen.getByText("root tag 1")).toBeInTheDocument(); + expect(screen.getByText('root tag 1')).toBeInTheDocument(); }); - it("does not keep a previous typed confirmation when the delete dialog is closed and reopened for the same tag", async () => { - const firstOpen = await openDeleteDialogForTag({ tagName: "root tag 1" }); + it('does not keep a previous typed confirmation when the delete dialog is closed and reopened for the same tag', async () => { + const firstOpen = await openDeleteDialogForTag({ tagName: 'root tag 1' }); fireEvent.change(firstOpen.input, { - target: { value: "DELETE ALL 3 TAGS" }, + target: { value: 'DELETE ALL 3 TAGS' }, }); expect(firstOpen.deleteButton).toBeEnabled(); fireEvent.click(firstOpen.cancelButton); await waitFor(() => { - expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); + expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); }); const secondOpen = await openDeleteDialogForTag({ - tagName: "root tag 1", + tagName: 'root tag 1', }); - expect(secondOpen.input.value).toEqual(""); + expect(secondOpen.input.value).toEqual(''); expect(secondOpen.deleteButton).toBeDisabled(); }); - it("completes the delete workflow with keyboard only by opening the menu, selecting Delete, typing the confirmation phrase, and pressing Enter", async () => { + it('completes the delete workflow with keyboard only by opening the menu, selecting Delete, typing the confirmation phrase, and pressing Enter', async () => { const user = userEvent.setup(); axiosMock.reset(); axiosMock.onDelete(deleteTagUrl).reply(204); @@ -1205,101 +1195,101 @@ describe('', () => { 200, buildTagsResponse( mockTagsResponse.results.filter( - (tag) => tag.value !== "root tag 2", + (tag) => tag.value !== 'root tag 2', ), ), ); - const row = screen.getByText("root tag 2").closest("tr"); - const actionsButton = within(row).getByRole("button", { + const row = screen.getByText('root tag 2').closest('tr'); + const actionsButton = within(row).getByRole('button', { name: /more actions for tag "root tag 2"/i, }); actionsButton.focus(); expect(actionsButton).toHaveFocus(); - await user.keyboard("{Enter}"); + await user.keyboard('{Enter}'); - const deleteMenuItem = await screen.findByRole("button", { + const deleteMenuItem = await screen.findByRole('button', { name: /^Delete$/i, }); deleteMenuItem.focus(); expect(deleteMenuItem).toHaveFocus(); - await user.keyboard("{Enter}"); + await user.keyboard('{Enter}'); - const dialog = await screen.findByRole("dialog"); - const input = within(dialog).getByRole("textbox"); - await user.type(input, "DELETE"); - await user.keyboard("{Enter}"); + const dialog = await screen.findByRole('dialog'); + const input = within(dialog).getByRole('textbox'); + await user.type(input, 'DELETE'); + await user.keyboard('{Enter}'); - await expectDeleteRequest({ tagName: "root tag 2", withSubtags: false }); + await expectDeleteRequest({ tagName: 'root tag 2', withSubtags: false }); expect( await screen.findByText( - "1 tag(s) deleted. This change will be applied across all tagged content.", + '1 tag(s) deleted. This change will be applied across all tagged content.', ), ).toBeInTheDocument(); await waitFor(() => { - expect(screen.queryByText("root tag 2")).not.toBeInTheDocument(); + expect(screen.queryByText('root tag 2')).not.toBeInTheDocument(); }); }); - it("cancels the delete workflow with keyboard only by pressing Escape and does not send a delete request", async () => { + it('cancels the delete workflow with keyboard only by pressing Escape and does not send a delete request', async () => { const user = userEvent.setup(); - const row = screen.getByText("root tag 1").closest("tr"); - const actionsButton = within(row).getByRole("button", { + const row = screen.getByText('root tag 1').closest('tr'); + const actionsButton = within(row).getByRole('button', { name: /more actions for tag "root tag 1"/i, }); actionsButton.focus(); expect(actionsButton).toHaveFocus(); - await user.keyboard("{Enter}"); + await user.keyboard('{Enter}'); - const deleteMenuItem = await screen.findByRole("button", { + const deleteMenuItem = await screen.findByRole('button', { name: /^Delete$/i, }); deleteMenuItem.focus(); expect(deleteMenuItem).toHaveFocus(); - await user.keyboard("{Enter}"); + await user.keyboard('{Enter}'); - const dialog = await screen.findByRole("dialog"); + const dialog = await screen.findByRole('dialog'); expect( within(dialog).getByText('Delete "root tag 1"'), ).toBeInTheDocument(); - await user.keyboard("{Escape}"); + await user.keyboard('{Escape}'); await waitFor(() => { - expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); + expect(screen.queryByRole('dialog')).not.toBeInTheDocument(); }); expect(axiosMock.history.delete.length).toBe(0); - expect(screen.getByText("root tag 1")).toBeInTheDocument(); + expect(screen.getByText('root tag 1')).toBeInTheDocument(); }); - it("surfaces a failed delete through both the persistent error alert and the delete-error toast while leaving the row visible", async () => { + it('surfaces a failed delete through both the persistent error alert and the delete-error toast while leaving the row visible', async () => { axiosMock.reset(); axiosMock.onDelete(deleteTagUrl).reply(() => { - const error = new AxiosError("Request failed with status code 500"); + const error = new AxiosError('Request failed with status code 500'); error.response = { data: { - value: ["Delete failed"], + value: ['Delete failed'], }, }; return Promise.reject(error); }); const { input, deleteButton } = await openDeleteDialogForTag({ - tagName: "root tag 1", + tagName: 'root tag 1', }); - fireEvent.change(input, { target: { value: "DELETE ALL 3 TAGS" } }); + fireEvent.change(input, { target: { value: 'DELETE ALL 3 TAGS' } }); fireEvent.click(deleteButton); - await expectDeleteRequest({ tagName: "root tag 1", withSubtags: true }); + await expectDeleteRequest({ tagName: 'root tag 1', withSubtags: true }); expect( - await screen.findByText("Error saving changes"), + await screen.findByText('Error saving changes'), ).toBeInTheDocument(); expect( - await screen.findByText("Error deleting tag: Delete failed"), + await screen.findByText('Error deleting tag: Delete failed'), ).toBeInTheDocument(); - expect(screen.getByText("root tag 1")).toBeInTheDocument(); + expect(screen.getByText('root tag 1')).toBeInTheDocument(); }); }); }); diff --git a/src/taxonomy/tag-list/TagListTable.tsx b/src/taxonomy/tag-list/TagListTable.tsx index 51bfd84d7c..11f0b98280 100644 --- a/src/taxonomy/tag-list/TagListTable.tsx +++ b/src/taxonomy/tag-list/TagListTable.tsx @@ -16,7 +16,6 @@ import { TABLE_MODES, } from './constants'; import { useTableModes, useEditActions } from './hooks'; -import TypeXToConfirmModal from '@src/generic/TypeXToConfirmModal'; import DeleteModal from './DeleteModal'; interface TagListTableProps { @@ -48,7 +47,7 @@ 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 [, setActiveActionMenuRowId] = useState(null); const [draftError, setDraftError] = useState(''); const treeData = (tagTree?.getAllAsDeepCopy() || []) as unknown as TreeRowData[]; const hasOpenDraft = isCreatingTopTag || creatingParentId !== null || editingRowId !== null; diff --git a/src/taxonomy/tag-list/hooks.ts b/src/taxonomy/tag-list/hooks.ts index 7f109974dd..9ff5540e16 100644 --- a/src/taxonomy/tag-list/hooks.ts +++ b/src/taxonomy/tag-list/hooks.ts @@ -15,7 +15,6 @@ import { } from './constants'; import messages from './messages'; -import { start } from '@src/library-authoring/__mocks__/contentLibrariesListV2'; import { getTagListRowData, getTagWithDescendantsCount } from './utils'; import { Row } from '@tanstack/react-table'; diff --git a/src/taxonomy/tree-table/TableView.tsx b/src/taxonomy/tree-table/TableView.tsx index 0d3d2bda01..7e8b1a919e 100644 --- a/src/taxonomy/tree-table/TableView.tsx +++ b/src/taxonomy/tree-table/TableView.tsx @@ -80,7 +80,12 @@ const TableView = ({ return ( <> - +
From 89d0f9f424f81fd330e855d89214565df8bc8a3c Mon Sep 17 00:00:00 2001 From: Jesper Hodge Date: Thu, 23 Apr 2026 18:40:45 -0400 Subject: [PATCH 13/16] fix: test --- src/taxonomy/tag-list/hooks.test.tsx | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/taxonomy/tag-list/hooks.test.tsx b/src/taxonomy/tag-list/hooks.test.tsx index 89fb70c0c6..d3ea744ed4 100644 --- a/src/taxonomy/tag-list/hooks.test.tsx +++ b/src/taxonomy/tag-list/hooks.test.tsx @@ -122,7 +122,7 @@ describe('useEditActions', () => { } = buildActions(); await act(async () => { - await actions.handleUpdateTag(' same value ', 'same value'); + await actions.handleUpdateRow(' same value ', 'same value'); }); expect(enterPreviewMode).not.toHaveBeenCalled(); @@ -139,7 +139,7 @@ describe('useEditActions', () => { } = buildActions(); await act(async () => { - await actions.handleUpdateTag('updated', 'original'); + await actions.handleUpdateRow('updated', 'original'); }); await waitFor(() => { @@ -152,17 +152,12 @@ describe('useEditActions', () => { expect(setEditingRowId).toHaveBeenCalledWith(null); }); - it('keeps draft open and shows failure toast when createTag request fails', async () => { - const { - actions, - createTagMutation, - setDraftError, - setToast, - } = buildActions(); + it('keeps draft open and shows failure toast when createRow request fails', async () => { + const { actions, createTagMutation, setDraftError, setToast } = buildActions(); createTagMutation.mutateAsync.mockRejectedValue(new Error('server failed')); await act(async () => { - await actions.handleCreateTag('new tag'); + await actions.handleCreateRow('new tag'); }); expect(setDraftError).toHaveBeenCalledWith('server failed'); From fa044c023d3cc055a110b2045e56ccc9910e5eb0 Mon Sep 17 00:00:00 2001 From: Jesper Hodge Date: Thu, 23 Apr 2026 18:43:30 -0400 Subject: [PATCH 14/16] fix: lint --- tsconfig.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tsconfig.json b/tsconfig.json index b924613492..e529f8012f 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -14,6 +14,6 @@ ".eslintrc.js", "src/**/*", "plugins/**/*" -, "src/generic/TypeXToConfirmModal.tsx" ], + ], "exclude": ["dist", "node_modules"] } From 79941db3aec6e5dfd534d0b058a6d82e4f74d3dc Mon Sep 17 00:00:00 2001 From: Jesper Hodge Date: Thu, 23 Apr 2026 19:21:29 -0400 Subject: [PATCH 15/16] refactor: make use of context --- src/taxonomy/tag-list/DeleteModal.test.tsx | 1 + src/taxonomy/tag-list/TagListTable.tsx | 1 + src/taxonomy/tree-table/CreateRow.test.tsx | 81 +++++++++----- src/taxonomy/tree-table/CreateRow.tsx | 35 +++--- src/taxonomy/tree-table/DraftRow.tsx | 9 +- src/taxonomy/tree-table/EditRow.tsx | 20 ++-- src/taxonomy/tree-table/NestedRows.test.tsx | 112 ++++++++++++------- src/taxonomy/tree-table/NestedRows.tsx | 81 ++------------ src/taxonomy/tree-table/TableBody.tsx | 95 ++++------------ src/taxonomy/tree-table/TableView.test.tsx | 1 + src/taxonomy/tree-table/TableView.tsx | 54 +++------ src/taxonomy/tree-table/TreeTableContext.tsx | 3 + 12 files changed, 214 insertions(+), 279 deletions(-) diff --git a/src/taxonomy/tag-list/DeleteModal.test.tsx b/src/taxonomy/tag-list/DeleteModal.test.tsx index c810435553..e1293216eb 100644 --- a/src/taxonomy/tag-list/DeleteModal.test.tsx +++ b/src/taxonomy/tag-list/DeleteModal.test.tsx @@ -75,6 +75,7 @@ const baseContextValue = (overrides = {}) => ({ handleDeleteRow: jest.fn(), startEditRow: jest.fn(), startDeleteRow: jest.fn(), + table: null, ...overrides, }); diff --git a/src/taxonomy/tag-list/TagListTable.tsx b/src/taxonomy/tag-list/TagListTable.tsx index 11f0b98280..e388448d56 100644 --- a/src/taxonomy/tag-list/TagListTable.tsx +++ b/src/taxonomy/tag-list/TagListTable.tsx @@ -159,6 +159,7 @@ const TagListTable = ({ taxonomyId, maxDepth }: TagListTableProps) => { const contextValue = { ...contextValueArgs, columns: getColumns(contextValueArgs), + table: null, }; return ( diff --git a/src/taxonomy/tree-table/CreateRow.test.tsx b/src/taxonomy/tree-table/CreateRow.test.tsx index 57a7840441..33144603c1 100644 --- a/src/taxonomy/tree-table/CreateRow.test.tsx +++ b/src/taxonomy/tree-table/CreateRow.test.tsx @@ -3,30 +3,57 @@ import { IntlProvider } from '@edx/frontend-platform/i18n'; import { fireEvent, render, screen } from '@testing-library/react'; import CreateRow from './CreateRow'; +import { TreeTableContext } from './TreeTableContext'; const wrapper = ({ children }: { children: React.ReactNode; }) => ( {children} ); -const baseProps = () => ({ +const baseContextValue = () => ({ + treeData: [], + columns: [], + pageCount: -1, + pagination: { pageIndex: 0, pageSize: 10 }, + handlePaginationChange: jest.fn(), + isLoading: false, + isCreatingTopRow: false, draftError: '', setDraftError: jest.fn(), handleCreateRow: jest.fn(), setIsCreatingTopRow: jest.fn(), exitDraftWithoutSave: jest.fn(), createRowMutation: { isPending: false }, + updateRowMutation: {}, + deleteRowMutation: {}, + toast: { show: false, message: '', variant: 'success' }, + setToast: jest.fn(), + creatingParentId: null, + setCreatingParentId: jest.fn(), validate: jest.fn((value: string) => value.trim().length > 0), + handleUpdateRow: jest.fn(), + editingRowId: null, + setEditingRowId: jest.fn(), + confirmDeleteDialogOpen: false, + setConfirmDeleteDialogOpen: jest.fn(), + confirmDeleteDialogContext: null, + setConfirmDeleteDialogContext: jest.fn(), + handleDeleteRow: jest.fn(), + startEditRow: jest.fn(), + startDeleteRow: jest.fn(), + table: null, }); describe('CreateRow', () => { it('saves on Enter when value is valid', () => { - const props = baseProps(); + const contextValue = baseContextValue(); render( - - - - -
, + + + + + +
+ , { wrapper }, ); @@ -34,19 +61,21 @@ describe('CreateRow', () => { fireEvent.change(input, { target: { value: ' new tag ' } }); fireEvent.keyDown(input, { key: 'Enter' }); - expect(props.handleCreateRow).toHaveBeenCalledWith('new tag'); + expect(contextValue.handleCreateRow).toHaveBeenCalledWith('new tag'); }); it('does not save on Enter when mutation is pending', () => { - const props = baseProps(); - props.createRowMutation = { isPending: true }; + const contextValue = baseContextValue(); + contextValue.createRowMutation = { isPending: true }; render( - - - - -
, + + + + + +
+ , { wrapper }, ); @@ -54,18 +83,20 @@ describe('CreateRow', () => { fireEvent.change(input, { target: { value: 'pending tag' } }); fireEvent.keyDown(input, { key: 'Enter' }); - expect(props.handleCreateRow).not.toHaveBeenCalled(); + expect(contextValue.handleCreateRow).not.toHaveBeenCalled(); }); it('cancels on Escape and resets draft state', () => { - const props = baseProps(); + const contextValue = baseContextValue(); render( - - - - -
, + + + + + +
+ , { wrapper }, ); @@ -73,8 +104,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(contextValue.setDraftError).toHaveBeenCalledWith(''); + expect(contextValue.setIsCreatingTopRow).toHaveBeenCalledWith(false); + expect(contextValue.exitDraftWithoutSave).toHaveBeenCalled(); }); }); diff --git a/src/taxonomy/tree-table/CreateRow.tsx b/src/taxonomy/tree-table/CreateRow.tsx index f130560b02..d16399f77b 100644 --- a/src/taxonomy/tree-table/CreateRow.tsx +++ b/src/taxonomy/tree-table/CreateRow.tsx @@ -1,42 +1,41 @@ -import React from 'react'; -import type { CreateRowMutationState } from './types'; +import React, { useContext } from 'react'; import DraftRow from './DraftRow'; +import { TreeTableContext } from './TreeTableContext'; interface CreateRowProps { - draftError: string; - setDraftError: (error: string) => void; - handleCreateRow: (value: string) => void; - setIsCreatingTopRow: (isCreating: boolean) => void; - exitDraftWithoutSave: () => void; - createRowMutation: CreateRowMutationState; + handleCreateRow?: (value: string) => void; + exitDraftWithoutSave?: () => void; indent?: number; - validate: (value: string, mode?: 'soft' | 'hard') => boolean; } const CreateRow: React.FC = ({ - draftError, - setDraftError, handleCreateRow, - setIsCreatingTopRow, exitDraftWithoutSave, - createRowMutation, indent = 0, - validate, }) => { + const { + setDraftError, + handleCreateRow: contextHandleCreateRow, + setIsCreatingTopRow, + exitDraftWithoutSave: contextExitDraftWithoutSave, + createRowMutation, + } = useContext(TreeTableContext); + + const onCreateRow = handleCreateRow ?? contextHandleCreateRow; + const onExitDraftWithoutSave = exitDraftWithoutSave ?? contextExitDraftWithoutSave; + const handleCancel = () => { setDraftError(''); setIsCreatingTopRow(false); - exitDraftWithoutSave(); + onExitDraftWithoutSave(); }; return ( diff --git a/src/taxonomy/tree-table/DraftRow.tsx b/src/taxonomy/tree-table/DraftRow.tsx index 1be3b01a2c..57b0b7ef1f 100644 --- a/src/taxonomy/tree-table/DraftRow.tsx +++ b/src/taxonomy/tree-table/DraftRow.tsx @@ -1,4 +1,4 @@ -import React, { useState } from 'react'; +import React, { useContext, useState } from 'react'; import { useIntl } from '@edx/frontend-platform/i18n'; import { Button, Spinner } from '@openedx/paragon'; import { Row } from '@tanstack/react-table'; @@ -7,15 +7,14 @@ import UsageCountDisplay from '@src/taxonomy/tag-list/UsageCountDisplay'; import { EditableCell } from './EditableCell'; import type { CreateRowMutationState, TreeRowData } from './types'; import messages from './messages'; +import { TreeTableContext } from './TreeTableContext'; 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; @@ -23,13 +22,11 @@ interface DraftRowProps { } const DraftRow: React.FC = ({ - draftError, initialValue = '', onSave, onCancel, mutationState, indent = 0, - validate, requireValueChangeToEnableSave = false, rowTestId, rowId, @@ -39,6 +36,8 @@ const DraftRow: React.FC = ({ const [saveDisabled, setSaveDisabled] = useState(true); const intl = useIntl(); + const { draftError, validate } = useContext(TreeTableContext); + const updateSaveDisabled = (value: string) => { const trimmedValue = value.trim(); const isValid = validate(value, 'soft'); diff --git a/src/taxonomy/tree-table/EditRow.tsx b/src/taxonomy/tree-table/EditRow.tsx index 12a83fa055..d4ef94d9c3 100644 --- a/src/taxonomy/tree-table/EditRow.tsx +++ b/src/taxonomy/tree-table/EditRow.tsx @@ -1,31 +1,29 @@ -import React from 'react'; +import React, { useContext } from 'react'; import { Row } from '@tanstack/react-table'; -import type { CreateRowMutationState, TreeRowData } from './types'; +import type { TreeRowData } from './types'; import DraftRow from './DraftRow'; +import { TreeTableContext } from './TreeTableContext'; 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 { + setDraftError, + updateRowMutation, + } = useContext(TreeTableContext); + const handleCancel = () => { setDraftError(''); cancelEditRow(); @@ -33,13 +31,11 @@ const EditRow: React.FC = ({ return ( diff --git a/src/taxonomy/tree-table/NestedRows.test.tsx b/src/taxonomy/tree-table/NestedRows.test.tsx index ceeecf0395..994a495000 100644 --- a/src/taxonomy/tree-table/NestedRows.test.tsx +++ b/src/taxonomy/tree-table/NestedRows.test.tsx @@ -3,21 +3,45 @@ import { IntlProvider } from '@edx/frontend-platform/i18n'; import { fireEvent, render, screen } from '@testing-library/react'; import NestedRows from './NestedRows'; +import { TreeTableContext } from './TreeTableContext'; const wrapper = ({ children }: { children: React.ReactNode; }) => ( {children} ); -const defaultRequiredProps = { - setIsCreatingTopRow: jest.fn(), +const baseContextValue: any = () => ({ + treeData: [], + columns: [], + pageCount: -1, + pagination: { pageIndex: 0, pageSize: 10 }, + handlePaginationChange: jest.fn(), + isLoading: false, + isCreatingTopRow: false, + draftError: '', createRowMutation: {}, updateRowMutation: {}, + deleteRowMutation: {}, + 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(), - exitDraftWithoutSave: jest.fn(), - validate: () => true, -}; + confirmDeleteDialogOpen: false, + setConfirmDeleteDialogOpen: jest.fn(), + confirmDeleteDialogContext: null, + setConfirmDeleteDialogContext: jest.fn(), + handleDeleteRow: jest.fn(), + startEditRow: jest.fn(), + startDeleteRow: jest.fn(), + table: null, +}); const makeCell = (id: string, content: string) => ({ id, @@ -46,16 +70,18 @@ const makeRow = ({ describe('NestedRows', () => { it('renders nothing when parent row is collapsed', () => { const parent = makeRow({ id: 1, value: 'parent', expanded: false }); + const contextValue = baseContextValue(); const { container } = render( - - - - -
, + + + + + +
+ , { wrapper }, ); @@ -70,30 +96,31 @@ describe('NestedRows', () => { expanded: true, subRows: [nestedChild], }); - const setCreatingParentId = jest.fn(); + const contextValue = baseContextValue(); + contextValue.setCreatingParentId = jest.fn(); + contextValue.creatingParentId = 2; + contextValue.createRowMutation = { isPending: false }; const onCancelCreation = jest.fn(); render( - - - - -
, + + + + + +
+ , { wrapper }, ); fireEvent.click(screen.getByText('Cancel')); - expect(setCreatingParentId).toHaveBeenCalledWith(null); + expect(contextValue.setCreatingParentId).toHaveBeenCalledWith(null); expect(onCancelCreation).toHaveBeenCalled(); }); @@ -106,18 +133,21 @@ describe('NestedRows', () => { subRows: [nestedChild], }); + const contextValue = baseContextValue(); + contextValue.editingRowId = '2:child'; + render( - - - - -
, + + + + + +
+ , { wrapper }, ); diff --git a/src/taxonomy/tree-table/NestedRows.tsx b/src/taxonomy/tree-table/NestedRows.tsx index c2647f9f98..25f3669d02 100644 --- a/src/taxonomy/tree-table/NestedRows.tsx +++ b/src/taxonomy/tree-table/NestedRows.tsx @@ -1,14 +1,10 @@ -import React from 'react'; +import React, { useContext } from 'react'; import { flexRender } from '@tanstack/react-table'; -import type { - RowId, - TreeRow, - TreeColumnDef, - CreateRowMutationState, -} from './types'; +import type { TreeRow } from './types'; import CreateRow from './CreateRow'; import EditRow from './EditRow'; +import { TreeTableContext } from './TreeTableContext'; interface NestedRowsProps { /** The parent row object from TanStack React Table */ @@ -25,33 +21,6 @@ interface NestedRowsProps { 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; } /** @@ -75,20 +44,16 @@ const NestedRows = ({ onCancelCreation = () => {}, childRowsData = [], depth = 1, - draftError = '', - setDraftError = () => {}, - creatingParentId = null, - setCreatingParentId = () => {}, - setIsCreatingTopRow, - createRowMutation, - updateRowMutation, - handleUpdateRow, - editingRowId, - setEditingRowId, - exitDraftWithoutSave, - columns = [], - validate, }: NestedRowsProps) => { + const { + creatingParentId, + setCreatingParentId, + handleUpdateRow, + editingRowId, + setEditingRowId, + exitDraftWithoutSave, + } = useContext(TreeTableContext); + if (!parentRow.getIsExpanded()) { return null; } @@ -98,14 +63,9 @@ const NestedRows = ({ <> {isCreating && ( onSaveNewChildRow(value, parentRowValue)} - setIsCreatingTopRow={setIsCreatingTopRow} exitDraftWithoutSave={onCancelCreation} - createRowMutation={createRowMutation} indent={indent} - validate={validate} /> )} {childRowsData?.map(row => { @@ -115,17 +75,13 @@ const NestedRows = ({ {editingRowId === `${row.original.id}:${String(row.original.value)}` ? ( handleUpdateRow(value, String(row.original.value))} cancelEditRow={() => { setEditingRowId(null); exitDraftWithoutSave(); }} - updateRowMutation={updateRowMutation} indent={indent} - validate={validate} row={row} /> ) : @@ -159,20 +115,7 @@ const NestedRows = ({ 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/TableBody.tsx b/src/taxonomy/tree-table/TableBody.tsx index 0962c4c6c7..c10469e6b3 100644 --- a/src/taxonomy/tree-table/TableBody.tsx +++ b/src/taxonomy/tree-table/TableBody.tsx @@ -1,4 +1,4 @@ -import React from 'react'; +import React, { useContext } from 'react'; import { useIntl } from '@edx/frontend-platform/i18n'; import { flexRender } from '@tanstack/react-table'; @@ -6,56 +6,31 @@ import { LoadingSpinner } from '@src/generic/Loading'; import NestedRows from './NestedRows'; import messages from './messages'; +import { TreeTableContext } from './TreeTableContext'; -import type { - CreateRowMutationState, - RowId, - TreeColumnDef, - TreeTable, -} from './types'; import CreateRow from './CreateRow'; import EditRow from './EditRow'; -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 TableBody = () => { const intl = useIntl(); + const { + columns, + isCreatingTopRow, + handleCreateRow, + exitDraftWithoutSave, + creatingParentId, + setCreatingParentId, + setDraftError, + table, + isLoading, + handleUpdateRow, + editingRowId, + setEditingRowId, + } = useContext(TreeTableContext); + + if (!table) { + return null; + } if (isLoading) { return ( @@ -80,15 +55,7 @@ const TableBody = ({ )} {isCreatingTopRow && ( - + )} {table.getRowModel().rows.filter(row => row.depth === 0).map(row => ( @@ -96,16 +63,12 @@ const TableBody = ({ {editingRowId === `${row.original.id}:${String(row.original.value)}` ? ( handleUpdateRow(value, String(row.original.value))} cancelEditRow={() => { setEditingRowId(null); exitDraftWithoutSave(); }} - updateRowMutation={updateRowMutation} - validate={validate} row={row} /> ) : @@ -130,20 +93,6 @@ const TableBody = ({ 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 4682ee1ae3..1edbf5991d 100644 --- a/src/taxonomy/tree-table/TableView.test.tsx +++ b/src/taxonomy/tree-table/TableView.test.tsx @@ -51,6 +51,7 @@ const baseContextValue = () => ({ handleDeleteRow: jest.fn(), startEditRow: jest.fn(), startDeleteRow: jest.fn(), + table: null, }); const renderTableView = ( diff --git a/src/taxonomy/tree-table/TableView.tsx b/src/taxonomy/tree-table/TableView.tsx index 7e8b1a919e..ca38d666f0 100644 --- a/src/taxonomy/tree-table/TableView.tsx +++ b/src/taxonomy/tree-table/TableView.tsx @@ -1,4 +1,4 @@ -import React, { useContext } from 'react'; +import React, { useContext, useMemo } from 'react'; import { Button, Toast, @@ -23,6 +23,7 @@ import './TableView.scss'; import messages from './messages'; import SaveErrorAlert from './SaveErrorAlert'; import { TreeTableContext } from './TreeTableContext'; +import { TreeRowData, TreeTable } from './types'; interface TableViewProps { enablePagination?: boolean; @@ -32,33 +33,24 @@ const TableView = ({ enablePagination = false, }: TableViewProps) => { const intl = useIntl(); + + const contextValue = useContext(TreeTableContext); + const { treeData, columns, pageCount, pagination, handlePaginationChange, - isLoading, - isCreatingTopRow, draftError, createRowMutation, updateRowMutation, deleteRowMutation, - handleCreateRow, toast, setToast, - setIsCreatingTopRow, - exitDraftWithoutSave, - creatingParentId, - setCreatingParentId, - setDraftError, - validate, - handleUpdateRow, - editingRowId, - setEditingRowId, - } = useContext(TreeTableContext); + } = contextValue; - const table = useReactTable({ + const table: TreeTable = useReactTable({ data: treeData, columns, getCoreRowModel: getCoreRowModel(), @@ -72,6 +64,11 @@ const TableView = ({ getSubRows: (row) => row?.subRows || undefined, }); + const nestedContextValue = useMemo(() => ({ + ...contextValue, + table, + }), [contextValue, table]); + const currentPageIndex = table.getState().pagination.pageIndex + 1; const { isError } = createRowMutation; @@ -79,7 +76,10 @@ const TableView = ({ const { isError: isDeleteError } = deleteRowMutation; return ( - <> + // This is a nested context provider. It is a valid pattern in React even if it looks odd, + // and the purpose here is to add the react-table instance to the overall context. + // Note that there is an outer context provider higher up in `TagListTable` as well. + ))} - + @@ -180,7 +162,7 @@ const TableView = ({ {toast.message} - + ); }; diff --git a/src/taxonomy/tree-table/TreeTableContext.tsx b/src/taxonomy/tree-table/TreeTableContext.tsx index bcb085cdfa..e004956064 100644 --- a/src/taxonomy/tree-table/TreeTableContext.tsx +++ b/src/taxonomy/tree-table/TreeTableContext.tsx @@ -7,6 +7,7 @@ import type { RowId, ToastState, TreeColumnDef, + TreeTable, TreeRowData, } from './types'; @@ -41,6 +42,7 @@ export interface TreeTableContextValue { handleDeleteRow: (row: Row) => void; startEditRow: (row: Row) => void; startDeleteRow: (row: Row) => void; + table: TreeTable | null; } export const TreeTableContext = createContext({ @@ -74,4 +76,5 @@ export const TreeTableContext = createContext({ handleDeleteRow: () => {}, startEditRow: () => {}, startDeleteRow: () => {}, + table: null, }); From 66ab6a5e099e6bbb0babe0b18827f7ec69b184de Mon Sep 17 00:00:00 2001 From: Jesper Hodge Date: Thu, 23 Apr 2026 19:32:25 -0400 Subject: [PATCH 16/16] fix: lint --- src/taxonomy/tree-table/TableBody.tsx | 4 +--- src/taxonomy/tree-table/TableView.tsx | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/taxonomy/tree-table/TableBody.tsx b/src/taxonomy/tree-table/TableBody.tsx index c10469e6b3..c46df26d10 100644 --- a/src/taxonomy/tree-table/TableBody.tsx +++ b/src/taxonomy/tree-table/TableBody.tsx @@ -54,9 +54,7 @@ const TableBody = () => { )} - {isCreatingTopRow && ( - - )} + {isCreatingTopRow && } {table.getRowModel().rows.filter(row => row.depth === 0).map(row => ( diff --git a/src/taxonomy/tree-table/TableView.tsx b/src/taxonomy/tree-table/TableView.tsx index ca38d666f0..c442279d0a 100644 --- a/src/taxonomy/tree-table/TableView.tsx +++ b/src/taxonomy/tree-table/TableView.tsx @@ -23,7 +23,7 @@ import './TableView.scss'; import messages from './messages'; import SaveErrorAlert from './SaveErrorAlert'; import { TreeTableContext } from './TreeTableContext'; -import { TreeRowData, TreeTable } from './types'; +import { TreeTable } from './types'; interface TableViewProps { enablePagination?: boolean;