fix: address comments on PR openedx#3024

Co-authored-by: Copilot <[email protected]>

Author: mgwozdz-unicon

src/generic/TypeXToConfirmModal.test.tsx

@@ -1,7 +1,7 @@
 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 { render, screen } from '@testing-library/react';
 
 import TypeXToConfirmModal from './TypeXToConfirmModal';
 
@@ -11,11 +11,11 @@ const defaultProps = () => ({
   confirmLabel: 'Delete',
   cancelLabel: 'Cancel',
   X: 'DELETE',
-  context: { id: 7 },
+  confirmPayload: { id: 7 },
   isOpen: true,
   onConfirm: jest.fn(),
   onCancel: jest.fn(),
-  setContext: jest.fn(),
+  setConfirmPayload: jest.fn(),
 });
 
 const renderModal = (props = defaultProps()) =>
@@ -26,45 +26,58 @@ const renderModal = (props = defaultProps()) =>
   );
 
 describe('TypeXToConfirmModal', () => {
-  it('keeps the destructive confirm button disabled until the typed value exactly matches the required confirmation phrase', () => {
+  it('renders the required confirmation phrase with strong emphasis', () => {
+    renderModal();
+
+    expect(screen.getByText('DELETE', { selector: 'strong' })).toBeInTheDocument();
+  });
+
+  it('keeps the destructive confirm button disabled until the typed value exactly matches the required confirmation phrase', async () => {
+    const user = userEvent.setup();
     renderModal();
 
     const input = screen.getByRole('textbox');
     const confirmButton = screen.getByRole('button', { name: 'Delete' });
 
     expect(confirmButton).toBeDisabled();
-    fireEvent.change(input, { target: { value: 'DEL' } });
+    await user.type(input, 'DEL');
     expect(confirmButton).toBeDisabled();
-    fireEvent.change(input, { target: { value: 'DELETE' } });
+    await user.type(input, 'ETE');
     expect(confirmButton).toBeEnabled();
   });
 
-  it('does not enable confirmation for partial, differently cased, or whitespace-padded confirmation text', () => {
+  it('does not enable confirmation for partial, differently cased, or whitespace-padded confirmation text', async () => {
+    const user = userEvent.setup();
     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 } });
+    for (const value of ['DEL', 'delete', ' DELETE', 'DELETE ', ' Delete ']) {
+      await user.clear(input);
+      await user.type(input, value);
       expect(confirmButton).toBeDisabled();
-    });
+    }
   });
 
-  it('submits on Enter only after the exact confirmation phrase has been entered', async () => {
+  it('requires explicit activation of the enabled destructive confirm button', async () => {
     const user = userEvent.setup();
     const props = defaultProps();
     renderModal(props);
 
     const input = screen.getByRole('textbox');
+    const confirmButton = screen.getByRole('button', { name: 'Delete' });
 
     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);
+    expect(props.onConfirm).not.toHaveBeenCalled();
+
+    await user.click(confirmButton);
+    expect(props.onConfirm).toHaveBeenCalledWith(props.confirmPayload);
   });
 
   it('resets confirmation state when the modal closes so a reopened dialog starts disabled again', async () => {
@@ -90,7 +103,7 @@ describe('TypeXToConfirmModal', () => {
     expect(screen.getByRole('button', { name: 'Delete' })).toBeDisabled();
   });
 
-  it('clears the provided context when the modal is closed without confirming', () => {
+  it('clears the provided confirm payload when the modal is closed without confirming', () => {
     const props = defaultProps();
     const { rerender } = renderModal(props);
 
@@ -100,7 +113,7 @@ describe('TypeXToConfirmModal', () => {
       </IntlProvider>,
     );
 
-    expect(props.setContext).toHaveBeenCalledWith(null);
+    expect(props.setConfirmPayload).toHaveBeenCalledWith(null);
     expect(props.onConfirm).not.toHaveBeenCalled();
   });
 });

src/generic/TypeXToConfirmModal.tsx

@@ -1,5 +1,12 @@
 import React, { useEffect } from 'react';
-import { Button, Card, Form, Icon, ModalDialog } from '@openedx/paragon';
+import {
+  ActionRow,
+  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';
@@ -10,12 +17,11 @@ interface TypeXToConfirmModalProps {
   confirmLabel: string;
   cancelLabel: string;
   X: string;
-  // any additional context that the caller wants to pass to the onConfirm callback; not a React context.
-  context?: Record<string, any> | null;
+  confirmPayload?: Record<string, any> | null;
   isOpen: boolean;
-  onConfirm: (context?: Record<string, any> | null) => void;
+  onConfirm: (confirmPayload?: Record<string, any> | null) => void;
   onCancel: () => void;
-  setContext?: (context: Record<string, any> | null) => void;
+  setConfirmPayload?: (confirmPayload: Record<string, any> | null) => void;
 }
 
 const TypeXToConfirmModal: React.FC<TypeXToConfirmModalProps> = ({
@@ -25,25 +31,18 @@ const TypeXToConfirmModal: React.FC<TypeXToConfirmModalProps> = ({
   confirmLabel,
   cancelLabel,
   isOpen,
-  context,
+  confirmPayload,
   onConfirm,
   onCancel,
-  setContext,
+  setConfirmPayload,
 }) => {
   const [confirmedByTyping, setConfirmedByTyping] = React.useState(false);
   const intl = useIntl();
 
-  const handleKeyDown = (e: React.KeyboardEvent<HTMLInputElement>) => {
-    if (!confirmedByTyping) { return; }
-    if (e.key === 'Enter') {
-      onConfirm(context);
-    }
-  };
-
   const handleConfirm = () => {
     if (!confirmedByTyping) { return; }
     setConfirmedByTyping(false);
-    onConfirm(context);
+    onConfirm(confirmPayload);
   };
 
   const handleCancel = () => {
@@ -63,12 +62,11 @@ const TypeXToConfirmModal: React.FC<TypeXToConfirmModalProps> = ({
   useEffect(() => {
     if (!isOpen) {
       setConfirmedByTyping(false);
-      if (setContext) {
-        // reset onConfirm callback context when modal is closed
-        setContext(null);
+      if (setConfirmPayload) {
+        setConfirmPayload(null);
       }
     }
-  }, [X, isOpen, context, setContext]);
+  }, [X, isOpen, confirmPayload, setConfirmPayload]);
 
   return (
     <ModalDialog
@@ -91,33 +89,27 @@ const TypeXToConfirmModal: React.FC<TypeXToConfirmModalProps> = ({
         </Card>
         <div className="mt-3">
           <div>
-            {(() => {
-              const messageText = intl.formatMessage(messages.typeToConfirmInstruction, { X });
-              const parts = messageText.split(X);
-              return (
-                <>
-                  {parts[0]}
-                  <strong>{X}</strong>
-                  {parts[1]}
-                </>
-              );
-            })()}
+            {intl.formatMessage(messages.typeToConfirmInstruction, {
+              X,
+              strong: (chunks: React.ReactNode) => <strong>{chunks}</strong>,
+            })}
           </div>
           <Form.Control
-            onKeyDown={handleKeyDown}
             onChange={handleChange}
             className="mt-4"
           />
         </div>
-        <ModalDialog.Footer>
+      </ModalDialog.Body>
+      <ModalDialog.Footer>
+        <ActionRow>
           <Button variant="tertiary" onClick={handleCancel}>
             {cancelLabel}
           </Button>
           <Button onClick={handleConfirm} disabled={!confirmedByTyping} variant="danger">
             {confirmLabel}
           </Button>
-        </ModalDialog.Footer>
-      </ModalDialog.Body>
+        </ActionRow>
+      </ModalDialog.Footer>
     </ModalDialog>
   );
 };

src/generic/messages.ts

@@ -3,7 +3,7 @@ import { defineMessages } from '@edx/frontend-platform/i18n';
 const messages = defineMessages({
   typeToConfirmInstruction: {
     id: 'course-authoring.generic.type-to-confirm-instruction',
-    defaultMessage: 'Type {X} to confirm',
+    defaultMessage: 'Type <strong>{X}</strong> to confirm',
   },
 });
 

src/taxonomy/tag-list/DeleteModal.tsx

@@ -72,8 +72,8 @@ const DeleteModal = ({
       confirmLabel={intl.formatMessage(count > 1 ? messages.deleteLabelPlural : messages.deleteLabelSingular)}
       cancelLabel={intl.formatMessage(messages.cancelLabel)}
       isOpen={isOpen}
-      context={row}
-      setContext={setRow}
+      confirmPayload={row}
+      setConfirmPayload={setRow}
       onConfirm={handleConfirm}
       onCancel={handleCancel}
     />

src/taxonomy/tag-list/utils.test.ts

@@ -28,6 +28,8 @@ describe('getTagsWithDescendantCount', () => {
       ],
     } as any;
 
+    // The total includes the root tag itself:
+    // root + child 1 + 2 grandchildren + child 2 + grandchild 3 + great grandchild 1 = 7.
     expect(getTagWithDescendantsCount(rowData)).toBe(7);
   });
 });

src/taxonomy/tag-list/utils.ts

@@ -11,6 +11,12 @@ export const getTagListRowData = (row: Row<TreeRowData>): TagListRowData => (
   row.original as unknown as TagListRowData
 );
 
+/**
+ * Counts this tag and every nested descendant below it.
+ *
+ * A leaf tag counts as 1. For parent tags, start with 1 for the tag itself,
+ * then recursively add the same count for each child in `subRows`.
+ */
 export const getTagWithDescendantsCount = (rowData: TreeRowData): number => {
   if (!rowData.subRows || rowData.subRows.length === 0) {
     return 1;