fix: license text modified overlaps license name

mstykow · mstykow · commit a1fbb821df25 · 2024-03-20T08:51:31.000+01:00
- always show license text input if license text exists - show button with explanatory tooltip to specify a custom license text when no license text present - this button enables user to specify a license text when needed but hides the input in most cases closes #2529 Signed-off-by: Maxim Stykow <maxim.stykow@tngtech.com>
diff --git a/src/Frontend/Components/AttributionForm/Comment/Comment.tsx b/src/Frontend/Components/AttributionForm/Comment/Comment.tsx
@@ -29,7 +29,7 @@ export function Comment({ packageInfo, onEdit, config, expanded }: Props) {
         title={'Comment'}
         text={packageInfo.comment}
         minRows={3}
-        maxRows={10}
+        maxRows={5}
         multiline={true}
         color={config?.color}
         focused={config?.focused}
diff --git a/src/Frontend/Components/AttributionForm/CopyrightSubPanel/CopyrightSubPanel.tsx b/src/Frontend/Components/AttributionForm/CopyrightSubPanel/CopyrightSubPanel.tsx
@@ -40,7 +40,7 @@ export function CopyrightSubPanel({
         title={'Copyright'}
         text={packageInfo.copyright}
         minRows={3}
-        maxRows={7}
+        maxRows={5}
         color={config?.color}
         focused={config?.focused}
         multiline
diff --git a/src/Frontend/Components/AttributionForm/LicenseSubPanel/LicenseSubPanel.tsx b/src/Frontend/Components/AttributionForm/LicenseSubPanel/LicenseSubPanel.tsx
@@ -2,14 +2,11 @@
 // SPDX-FileCopyrightText: TNG Technology Consulting GmbH <https://www.tngtech.com>
 //
 // SPDX-License-Identifier: Apache-2.0
-import ExpandMoreIcon from '@mui/icons-material/ExpandMore';
-import MuiAccordion from '@mui/material/Accordion';
-import MuiAccordionDetails from '@mui/material/AccordionDetails';
-import MuiAccordionSummary from '@mui/material/AccordionSummary';
 import MuiBox from '@mui/material/Box';
-import MuiInputAdornment from '@mui/material/InputAdornment';
+import Link from '@mui/material/Link';
+import Tooltip from '@mui/material/Tooltip';
 import { sortBy } from 'lodash';
-import { useMemo, useState } from 'react';
+import { useEffect, useMemo, useState } from 'react';
 
 import { PackageInfo } from '../../../../shared/shared-types';
 import { text } from '../../../../shared/text';
@@ -19,41 +16,9 @@ import { getFrequentLicensesNameOrder } from '../../../state/selectors/resource-
 import { Confirm } from '../../ConfirmationDialog/ConfirmationDialog';
 import { TextBox } from '../../TextBox/TextBox';
 import { AttributionFormConfig } from '../AttributionForm';
-import { attributionColumnClasses } from '../AttributionForm.style';
 import { PackageAutocomplete } from '../PackageAutocomplete/PackageAutocomplete';
 
 const classes = {
-  ...attributionColumnClasses,
-  expansionPanel: {
-    backgroundColor: 'transparent',
-    '&.MuiAccordion-expanded': {
-      margin: '0px 0px 6px 0px !important',
-    },
-  },
-  expansionPanelSummary: {
-    minHeight: '36px',
-    padding: '0px',
-    '& div.MuiAccordionSummary-content': {
-      margin: '0px',
-    },
-    '& div.MuiAccordionSummary-expandIcon': {
-      padding: '0px',
-    },
-  },
-  expansionPanelDetails: {
-    padding: '0px',
-    width: 'calc(100% - 36px)',
-  },
-  expansionPanelDetailsDiffView: {
-    padding: '0px',
-  },
-  expandMoreIcon: {
-    display: 'flex',
-    alignItems: 'center',
-    justifyContent: 'center',
-    height: '36px',
-    width: '36px',
-  },
   licenseText: {
     marginTop: '12px',
   },
@@ -76,12 +41,12 @@ export function LicenseSubPanel({
   packageInfo,
   showHighlight,
   onEdit,
-  expanded: expandedOverride,
+  expanded,
   hidden,
   config,
 }: LicenseSubPanelProps) {
   const dispatch = useAppDispatch();
-  const [expanded, setExpanded] = useState(expandedOverride);
+  const [isLicenseTextShown, setIsLicenseTextShown] = useState(false);
   const frequentLicensesNames = useAppSelector(getFrequentLicensesNameOrder);
   const defaultLicenses = useMemo(
     () =>
@@ -98,99 +63,66 @@ export function LicenseSubPanel({
       ),
     [frequentLicensesNames],
   );
-  const label = useMemo(
-    () =>
-      packageInfo.licenseName &&
-      frequentLicensesNames
-        .map((licenseNames) => [
-          licenseNames.shortName.toLowerCase(),
-          licenseNames.fullName.toLowerCase(),
-        ])
-        .flat()
-        .includes(packageInfo.licenseName.toLowerCase())
-        ? `Standard license text implied. ${
-            !!onEdit ? 'Insert notice text if necessary.' : ''
-          }`
-        : 'License Text (to appear in attribution document)',
-    [frequentLicensesNames, onEdit, packageInfo.licenseName],
-  );
+  const showLicenseTextInput = expanded || !!packageInfo.licenseText;
+
+  useEffect(() => {
+    setIsLicenseTextShown(showLicenseTextInput);
+  }, [showLicenseTextInput]);
 
   return hidden ? null : (
-    <MuiBox sx={classes.panel}>
-      <MuiAccordion
-        sx={classes.expansionPanel}
-        elevation={0}
-        key={'License'}
-        disableGutters
-        expanded={expanded}
-        square
-      >
-        <MuiAccordionSummary
-          sx={classes.expansionPanelSummary}
-          expandIcon={
-            expandedOverride ? null : (
-              <MuiBox
-                sx={classes.expandMoreIcon}
-                onClick={() => setExpanded((prev) => !prev)}
-              >
-                <ExpandMoreIcon aria-label={'license text toggle'} />
-              </MuiBox>
+    <MuiBox>
+      <PackageAutocomplete
+        attribute={'licenseName'}
+        title={text.attributionColumn.licenseName}
+        packageInfo={packageInfo}
+        readOnly={!onEdit}
+        showHighlight={showHighlight}
+        onEdit={onEdit}
+        endAdornment={config?.licenseName?.endIcon}
+        defaults={defaultLicenses}
+        color={config?.licenseName?.color}
+        focused={config?.licenseName?.focused}
+      />
+      {isLicenseTextShown ? (
+        <TextBox
+          readOnly={!onEdit}
+          sx={classes.licenseText}
+          maxRows={10}
+          minRows={3}
+          color={config?.licenseText?.color}
+          focused={config?.licenseText?.focused}
+          multiline
+          expanded={expanded}
+          title={text.attributionColumn.licenseText}
+          text={packageInfo.licenseText}
+          handleChange={({ target: { value } }) =>
+            onEdit?.(() =>
+              dispatch(
+                setTemporaryDisplayPackageInfo({
+                  ...packageInfo,
+                  licenseText: value,
+                  wasPreferred: undefined,
+                }),
+              ),
             )
           }
-        >
-          <PackageAutocomplete
-            attribute={'licenseName'}
-            title={text.attributionColumn.licenseName}
-            packageInfo={packageInfo}
-            readOnly={!onEdit}
-            showHighlight={showHighlight}
-            onEdit={onEdit}
-            endAdornment={
-              config?.licenseName?.endIcon ||
-              (packageInfo.licenseText ? (
-                <MuiInputAdornment position="end" sx={classes.endAdornment}>
-                  {text.attributionColumn.licenseTextModified}
-                </MuiInputAdornment>
-              ) : undefined)
-            }
-            defaults={defaultLicenses}
-            color={config?.licenseName?.color}
-            focused={config?.licenseName?.focused}
-          />
-        </MuiAccordionSummary>
-        <MuiAccordionDetails
-          sx={
-            expandedOverride
-              ? classes.expansionPanelDetailsDiffView
-              : classes.expansionPanelDetails
-          }
-        >
-          <TextBox
-            readOnly={!onEdit}
-            sx={classes.licenseText}
-            maxRows={7}
-            minRows={3}
-            color={config?.licenseText?.color}
-            focused={config?.licenseText?.focused}
-            multiline
-            expanded={expandedOverride}
-            title={label}
-            text={packageInfo.licenseText}
-            handleChange={({ target: { value } }) =>
-              onEdit?.(() =>
-                dispatch(
-                  setTemporaryDisplayPackageInfo({
-                    ...packageInfo,
-                    licenseText: value,
-                    wasPreferred: undefined,
-                  }),
-                ),
-              )
-            }
-            endIcon={config?.licenseText?.endIcon}
-          />
-        </MuiAccordionDetails>
-      </MuiAccordion>
+          endIcon={config?.licenseText?.endIcon}
+        />
+      ) : (
+        onEdit && (
+          <Tooltip title={text.attributionColumn.useCustomLicenseTextHint}>
+            <Link
+              sx={{ padding: '4px' }}
+              underline={'hover'}
+              component={'button'}
+              variant={'caption'}
+              onClick={() => setIsLicenseTextShown(true)}
+            >
+              {text.attributionColumn.useCustomLicenseText}
+            </Link>
+          </Tooltip>
+        )
+      )}
     </MuiBox>
   );
 }
diff --git a/src/Frontend/Components/AttributionForm/__tests__/AttributionForm.test.tsx b/src/Frontend/Components/AttributionForm/__tests__/AttributionForm.test.tsx
@@ -6,11 +6,9 @@
 import { fireEvent, screen } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 
-import { FrequentLicenses, PackageInfo } from '../../../../shared/shared-types';
+import { PackageInfo } from '../../../../shared/shared-types';
 import { text } from '../../../../shared/text';
 import { faker } from '../../../../testing/Faker';
-import { setFrequentLicenses } from '../../../state/actions/resource-actions/all-views-simple-actions';
-import { setSelectedAttributionId } from '../../../state/actions/resource-actions/audit-view-simple-actions';
 import { getTemporaryDisplayPackageInfo } from '../../../state/selectors/resource-selectors';
 import { renderComponent } from '../../../test-helpers/render';
 import { generatePurl } from '../../../util/handle-purl';
@@ -201,7 +199,7 @@ describe('AttributionForm', () => {
     expect(screen.queryByLabelText('Url icon')).not.toBeInTheDocument();
   });
 
-  it('shows standard text if editable and non frequent license', () => {
+  it('shows button to use custom license text if no license text specified and in edit mode', () => {
     const packageInfo: PackageInfo = {
       packageName: 'jQuery',
       id: faker.string.uuid(),
@@ -211,53 +209,71 @@ describe('AttributionForm', () => {
     );
 
     expect(
-      screen.getByLabelText('License Text (to appear in attribution document)'),
+      screen.getByRole('button', {
+        name: text.attributionColumn.useCustomLicenseTextHint,
+      }),
     ).toBeInTheDocument();
+    expect(
+      screen.queryByLabelText(text.attributionColumn.licenseText),
+    ).not.toBeInTheDocument();
   });
 
-  it('shows shortened text if not editable and frequent license', () => {
+  it('shows neither button to use custom license text nor license text input field if no license text specified but not in edit mode', () => {
     const packageInfo: PackageInfo = {
       packageName: 'jQuery',
-      licenseName: 'Mit',
       id: faker.string.uuid(),
     };
-    const frequentLicenses: FrequentLicenses = {
-      nameOrder: [{ shortName: 'MIT', fullName: 'MIT license' }],
-      texts: { MIT: 'text' },
+    renderComponent(<AttributionForm packageInfo={packageInfo} />);
+
+    expect(
+      screen.queryByRole('button', {
+        name: text.attributionColumn.useCustomLicenseTextHint,
+      }),
+    ).not.toBeInTheDocument();
+    expect(
+      screen.queryByLabelText(text.attributionColumn.licenseText),
+    ).not.toBeInTheDocument();
+  });
+
+  it('shows license text instead of button if license text is specified and in edit mode', () => {
+    const licenseText = faker.lorem.paragraph();
+    const packageInfo: PackageInfo = {
+      packageName: 'jQuery',
+      id: faker.string.uuid(),
+      licenseText,
     };
-    renderComponent(<AttributionForm packageInfo={packageInfo} />, {
-      actions: [
-        setFrequentLicenses(frequentLicenses),
-        setSelectedAttributionId(packageInfo.id),
-      ],
-    });
+    renderComponent(
+      <AttributionForm packageInfo={packageInfo} onEdit={jest.fn()} />,
+    );
 
     expect(
-      screen.getByLabelText('Standard license text implied.'),
+      screen.getByLabelText(text.attributionColumn.licenseText),
     ).toBeInTheDocument();
+    expect(screen.getByDisplayValue(licenseText)).toBeInTheDocument();
+    expect(
+      screen.queryByRole('button', {
+        name: text.attributionColumn.useCustomLicenseTextHint,
+      }),
+    ).not.toBeInTheDocument();
   });
 
-  it('shows long text if editable and frequent license', () => {
+  it('shows license text input field when user clicks button to use custom license text', async () => {
     const packageInfo: PackageInfo = {
       packageName: 'jQuery',
-      licenseName: 'mit',
       id: faker.string.uuid(),
     };
-    const frequentLicenses: FrequentLicenses = {
-      nameOrder: [{ shortName: 'MIT', fullName: 'MIT license' }],
-      texts: { MIT: 'text' },
-    };
     renderComponent(
       <AttributionForm packageInfo={packageInfo} onEdit={jest.fn()} />,
-      {
-        actions: [setFrequentLicenses(frequentLicenses)],
-      },
+    );
+
+    await userEvent.click(
+      screen.getByRole('button', {
+        name: text.attributionColumn.useCustomLicenseTextHint,
+      }),
     );
 
     expect(
-      screen.getByLabelText(
-        'Standard license text implied. Insert notice text if necessary.',
-      ),
+      screen.getByLabelText(text.attributionColumn.licenseText),
     ).toBeInTheDocument();
   });
 
diff --git a/src/Frontend/Components/TextBox/TextBox.tsx b/src/Frontend/Components/TextBox/TextBox.tsx
@@ -70,7 +70,7 @@ export interface TextBoxProps {
   readOnly?: boolean;
   sx?: SxProps;
   text?: string;
-  title?: string;
+  title: string;
   endIcon?: React.ReactElement | Array<React.ReactElement>;
 }
 
diff --git a/src/e2e-tests/__tests__/comparing-attribution-with-origin.test.ts b/src/e2e-tests/__tests__/comparing-attribution-with-origin.test.ts
@@ -80,7 +80,7 @@ test('reverts all changes and applies reverted state to temporary package info',
     AttributionType.FirstParty,
   );
   await diffPopup.currentAttributionForm.assert.copyrightIs('');
-  await diffPopup.currentAttributionForm.assert.licenseTextIsVisible();
+  await diffPopup.currentAttributionForm.assert.licenseTextInputIsVisible();
   await diffPopup.currentAttributionForm.assert.licenseTextIs('');
   await diffPopup.currentAttributionForm.assert.licenseNameIs('');
   await diffPopup.currentAttributionForm.assert.nameUndoButtonIsVisible();
@@ -168,7 +168,7 @@ test('handles pending license and copyright changes in temporary package info co
 
   await diffPopup.currentAttributionForm.attributionTypeRedoButton.click();
   await diffPopup.applyButton.click();
-  await attributionDetails.attributionForm.assert.licenseTextIsHidden();
+  await attributionDetails.attributionForm.assert.licenseTextInputIsHidden();
 
   await attributionDetails.attributionForm.selectAttributionType(
     AttributionType.ThirdParty,
diff --git a/src/e2e-tests/__tests__/updating-attributions.test.ts b/src/e2e-tests/__tests__/updating-attributions.test.ts
diff --git a/src/e2e-tests/page-objects/AttributionForm.ts b/src/e2e-tests/page-objects/AttributionForm.ts
diff --git a/src/shared/text.ts b/src/shared/text.ts

Original file line number	Diff line number	Diff line change
`@@ -70,7 +70,7 @@ export interface TextBoxProps {`
`70`	`70`	`readOnly?: boolean;`
`71`	`71`	`sx?: SxProps;`
`72`	`72`	`text?: string;`
`73`		`- title?: string;`
	`73`	`+ title: string;`
`74`	`74`	`endIcon?: React.ReactElement \| Array<React.ReactElement>;`
`75`	`75`	`}`
`76`	`76`