fix: group configurations - resolve discussions

fix: [AXIMST-714] icon is aligned with text (#210)
openedx · Mar 31, 2024 · 5afea18 · 5afea18
1 parent 48b0aeb
commit 5afea18
Show file tree

Hide file tree

Showing 14 changed files with 218 additions and 53 deletions.
diff --git a/src/generic/prompt-if-dirty/PromptIfDirty.jsx b/src/generic/prompt-if-dirty/PromptIfDirty.jsx
@@ -0,0 +1,24 @@
+import { useEffect } from 'react';
+import PropTypes from 'prop-types';
+
+const PromptIfDirty = ({ dirty }) => {
+  useEffect(() => {
+    // eslint-disable-next-line consistent-return
+    const handleBeforeUnload = (event) => {
+      if (dirty) {
+        event.preventDefault();
+      }
+    };
+    window.addEventListener('beforeunload', handleBeforeUnload);
+
+    return () => {
+      window.removeEventListener('beforeunload', handleBeforeUnload);
+    };
+  }, [dirty]);
+
+  return null;
+};
+PromptIfDirty.propTypes = {
+  dirty: PropTypes.bool.isRequired,
+};
+export default PromptIfDirty;
diff --git a/src/generic/prompt-if-dirty/PromptIfDirty.test.jsx b/src/generic/prompt-if-dirty/PromptIfDirty.test.jsx
@@ -0,0 +1,72 @@
+import React from 'react';
+import { render, unmountComponentAtNode } from 'react-dom';
+import { act } from 'react-dom/test-utils';
+import PromptIfDirty from './PromptIfDirty';
+
+describe('PromptIfDirty', () => {
+  let container = null;
+  let mockEvent = null;
+
+  beforeEach(() => {
+    container = document.createElement('div');
+    document.body.appendChild(container);
+    mockEvent = new Event('beforeunload');
+    jest.spyOn(window, 'addEventListener');
+    jest.spyOn(window, 'removeEventListener');
+    jest.spyOn(mockEvent, 'preventDefault');
+    Object.defineProperty(mockEvent, 'returnValue', { writable: true });
+    mockEvent.returnValue = '';
+  });
+
+  afterEach(() => {
+    window.addEventListener.mockRestore();
+    window.removeEventListener.mockRestore();
+    mockEvent.preventDefault.mockRestore();
+    mockEvent = null;
+    unmountComponentAtNode(container);
+    container.remove();
+    container = null;
+  });
+
+  it('should add event listener on mount', () => {
+    act(() => {
+      render(<PromptIfDirty dirty />, container);
+    });
+
+    expect(window.addEventListener).toHaveBeenCalledWith('beforeunload', expect.any(Function));
+  });
+
+  it('should remove event listener on unmount', () => {
+    act(() => {
+      render(<PromptIfDirty dirty />, container);
+    });
+    act(() => {
+      unmountComponentAtNode(container);
+    });
+
+    expect(window.removeEventListener).toHaveBeenCalledWith('beforeunload', expect.any(Function));
+  });
+
+  it('should call preventDefault and set returnValue when dirty is true', () => {
+    act(() => {
+      render(<PromptIfDirty dirty />, container);
+    });
+    act(() => {
+      window.dispatchEvent(mockEvent);
+    });
+
+    expect(mockEvent.preventDefault).toHaveBeenCalled();
+    expect(mockEvent.returnValue).toBe('');
+  });
+
+  it('should not call preventDefault when dirty is false', () => {
+    act(() => {
+      render(<PromptIfDirty dirty={false} />, container);
+    });
+    act(() => {
+      window.dispatchEvent(mockEvent);
+    });
+
+    expect(mockEvent.preventDefault).not.toHaveBeenCalled();
+  });
+});
diff --git a/src/group-configurations/common/UsageList.jsx b/src/group-configurations/common/UsageList.jsx
@@ -1,7 +1,10 @@
 import PropTypes from 'prop-types';
 import { useIntl } from '@edx/frontend-platform/i18n';
 import { Hyperlink, Stack, Icon } from '@openedx/paragon';
-import { Warning as WarningIcon, Error as ErrorIcon } from '@openedx/paragon/icons';
+import {
+  Warning as WarningIcon,
+  Error as ErrorIcon,
+} from '@openedx/paragon/icons';
 
 import { MESSAGE_VALIDATION_TYPES } from '../constants';
 import { formatUrlToUnitPage } from '../utils';
@@ -13,9 +16,13 @@ const UsageList = ({ className, itemList, isExperiment }) => {
     ? messages.experimentAccessTo
     : messages.accessTo;
 
-  const renderValidationMessage = ({ text }) => (
-    <span className="d-inline-flex">
-      <Icon src={MESSAGE_VALIDATION_TYPES.error ? ErrorIcon : WarningIcon} size="sm" className="mr-2" />
+  const renderValidationMessage = ({ text, type }) => (
+    <span className="d-inline-flex align-items-center">
+      <Icon
+        src={MESSAGE_VALIDATION_TYPES.error === type ? ErrorIcon : WarningIcon}
+        size="sm"
+        className="mr-2"
+      />
       <span className="small text-gray-700">{text}</span>
     </span>
   );

diff --git a/src/group-configurations/content-groups-section/ContentGroupForm.jsx b/src/group-configurations/content-groups-section/ContentGroupForm.jsx
@@ -13,7 +13,7 @@ import {
 
 import { WarningFilled as WarningFilledIcon } from '@openedx/paragon/icons';
 
-import PromptIfDirty from '../../generic/PromptIfDirty';
+import PromptIfDirty from '../../generic/prompt-if-dirty/PromptIfDirty';
 import { isAlreadyExistsGroup } from './utils';
 import messages from './messages';
 

diff --git a/src/group-configurations/empty-placeholder/index.jsx b/src/group-configurations/empty-placeholder/index.jsx
@@ -1,7 +1,7 @@
 import PropTypes from 'prop-types';
 import { useIntl } from '@edx/frontend-platform/i18n';
-import { Add as IconAdd } from '@edx/paragon/icons';
-import { Button } from '@edx/paragon';
+import { Add as IconAdd } from '@openedx/paragon/icons';
+import { Button } from '@openedx/paragon';
 
 import messages from './messages';
 

diff --git a/src/group-configurations/experiment-configurations-section/ExperimentCard.jsx b/src/group-configurations/experiment-configurations-section/ExperimentCard.jsx
@@ -1,4 +1,4 @@
-import { useState } from 'react';
+import { useState, useEffect } from 'react';
 import PropTypes from 'prop-types';
 import { useParams } from 'react-router-dom';
 import { getConfig } from '@edx/frontend-platform';
@@ -26,6 +26,7 @@ import { initialExperimentConfiguration } from './constants';
 const ExperimentCard = ({
   configuration,
   experimentConfigurationActions,
+  isExpandedByDefault,
   onCreate,
 }) => {
   const { formatMessage } = useIntl();
@@ -34,6 +35,10 @@ const ExperimentCard = ({
   const [isEditMode, switchOnEditMode, switchOffEditMode] = useToggle(false);
   const [isOpenDeleteModal, openDeleteModal, closeDeleteModal] = useToggle(false);
 
+  useEffect(() => {
+    setIsExpanded(isExpandedByDefault);
+  }, [isExpandedByDefault]);
+
   const {
     id, groups: groupsControl, description, usage,
   } = configuration;
@@ -59,8 +64,14 @@ const ExperimentCard = ({
     </span>
   );
 
+  // We need to store actual idx as an additional field for getNextGroupName utility.
+  const configurationGroupsWithIndexField = {
+    ...configuration,
+    groups: configuration.groups.map((group, idx) => ({ ...group, idx })),
+  };
+
   const formValues = isEditMode
-    ? configuration
+    ? configurationGroupsWithIndexField
     : initialExperimentConfiguration;
 
   const handleDeleteConfiguration = () => {
@@ -85,7 +96,11 @@ const ExperimentCard = ({
           onEditClick={handleEditConfiguration}
         />
       ) : (
-        <div className="configuration-card" data-testid="configuration-card">
+        <div
+          className="configuration-card"
+          data-testid="configuration-card"
+          id={id}
+        >
           <div className="configuration-card-header">
             <TitleButton
               group={configuration}
@@ -106,7 +121,7 @@ const ExperimentCard = ({
                 className="configuration-card-header__delete-tooltip"
                 tooltipContent={formatMessage(
                   isUsedInLocation
-                    ? messages.deleteRestriction
+                    ? messages.experimentConfigurationDeleteRestriction
                     : messages.actionDelete,
                 )}
                 alt={formatMessage(messages.actionDelete)}
@@ -155,6 +170,7 @@ ExperimentCard.defaultProps = {
     usage: [],
     version: undefined,
   },
+  isExpandedByDefault: false,
   onCreate: null,
   experimentConfigurationActions: {},
 };
@@ -194,6 +210,7 @@ ExperimentCard.propTypes = {
     }),
     scheme: PropTypes.string,
   }),
+  isExpandedByDefault: PropTypes.bool,
   onCreate: PropTypes.func,
   experimentConfigurationActions: PropTypes.shape({
     handleCreate: PropTypes.func,

diff --git a/src/group-configurations/experiment-configurations-section/ExperimentForm.jsx b/src/group-configurations/experiment-configurations-section/ExperimentForm.jsx
@@ -11,7 +11,7 @@ import {
 } from '@openedx/paragon';
 import { WarningFilled as WarningFilledIcon } from '@openedx/paragon/icons';
 
-import PromptIfDirty from '../../generic/PromptIfDirty';
+import PromptIfDirty from '../../generic/prompt-if-dirty/PromptIfDirty';
 import ExperimentFormGroups from './ExperimentFormGroups';
 import messages from './messages';
 import { experimentFormValidationSchema } from './validation';
@@ -87,6 +87,7 @@ const ExperimentForm = ({
                 placeholder={formatMessage(
                   messages.experimentConfigurationDescriptionPlaceholder,
                 )}
+                as="textarea"
               />
               <Form.Control.Feedback hasIcon={false} type="default">
                 {formatMessage(

diff --git a/src/group-configurations/experiment-configurations-section/constants.js b/src/group-configurations/experiment-configurations-section/constants.js
@@ -3,8 +3,12 @@ export const initialExperimentConfiguration = {
   name: '',
   description: '',
   groups: [
-    { name: 'Group A', version: 1, usage: [] },
-    { name: 'Group B', version: 1, usage: [] },
+    {
+      name: 'Group A', version: 1, usage: [], idx: 0,
+    },
+    {
+      name: 'Group B', version: 1, usage: [], idx: 1,
+    },
   ],
   scheme: 'random',
   parameters: {},

diff --git a/src/group-configurations/experiment-configurations-section/index.jsx b/src/group-configurations/experiment-configurations-section/index.jsx
@@ -3,6 +3,7 @@ import { Button, useToggle } from '@openedx/paragon';
 import { useIntl } from '@edx/frontend-platform/i18n';
 import { Add as AddIcon } from '@openedx/paragon/icons';
 
+import { useScrollToHashElement } from '../../hooks';
 import EmptyPlaceholder from '../empty-placeholder';
 import ExperimentForm from './ExperimentForm';
 import ExperimentCard from './ExperimentCard';
@@ -24,6 +25,8 @@ const ExperimentConfigurationsSection = ({
     experimentConfigurationActions.handleCreate(configuration, hideNewConfiguration);
   };
 
+  const { elementWithHash } = useScrollToHashElement({ isLoading: true });
+
   return (
     <div className="mt-2.5">
       <h2 className="lead text-black mb-3 configuration-section-name">
@@ -36,6 +39,7 @@ const ExperimentConfigurationsSection = ({
               key={configuration.id}
               configuration={configuration}
               experimentConfigurationActions={experimentConfigurationActions}
+              isExpandedByDefault={configuration.id === +elementWithHash}
               onCreate={handleCreateConfiguration}
             />
           ))}

diff --git a/src/group-configurations/experiment-configurations-section/messages.js b/src/group-configurations/experiment-configurations-section/messages.js
@@ -108,7 +108,7 @@ const messages = defineMessages({
   },
   subtitleModalDelete: {
     id: 'course-authoring.group-configurations.experiment-card.delete-modal.subtitle',
-    defaultMessage: 'content group',
+    defaultMessage: 'group configurations',
   },
   deleteRestriction: {
     id: 'course-authoring.group-configurations.experiment-card.delete-restriction',

diff --git a/src/group-configurations/experiment-configurations-section/utils.js b/src/group-configurations/experiment-configurations-section/utils.js
@@ -12,27 +12,28 @@ const getNextGroupName = (groups, groupFieldName = 'name') => {
   const existingGroupNames = groups.map((group) => group.name);
   const lettersCount = ALPHABET_LETTERS.length;
 
-  let nextIndex = existingGroupNames.length + 1;
+  // Calculate the maximum index of existing groups
+  const maxIdx = groups.reduce((max, group) => Math.max(max, group.idx), -1);
 
-  let groupName = '';
-  while (nextIndex > 0) {
-    groupName = ALPHABET_LETTERS[(nextIndex - 1) % lettersCount] + groupName;
-    nextIndex = Math.floor((nextIndex - 1) / lettersCount);
-  }
+  // Calculate the next index for the new group
+  const nextIndex = maxIdx + 1;
 
+  let groupName = '';
   let counter = 0;
-  let newName = groupName;
-  while (existingGroupNames.includes(`Group ${newName}`)) {
-    counter++;
-    let newIndex = existingGroupNames.length + 1 + counter;
+
+  do {
+    let tempIndex = nextIndex + counter;
     groupName = '';
-    while (newIndex > 0) {
-      groupName = ALPHABET_LETTERS[(newIndex - 1) % lettersCount] + groupName;
-      newIndex = Math.floor((newIndex - 1) / lettersCount);
+    while (tempIndex >= 0) {
+      groupName = ALPHABET_LETTERS[tempIndex % lettersCount] + groupName;
+      tempIndex = Math.floor(tempIndex / lettersCount) - 1;
     }
-    newName = groupName;
-  }
-  return { [groupFieldName]: `Group ${newName}`, version: 1, usage: [] };
+    counter++;
+  } while (existingGroupNames.includes(`Group ${groupName}`));
+
+  return {
+    [groupFieldName]: `Group ${groupName}`, version: 1, usage: [], idx: nextIndex,
+  };
 };
 
 /**