fix: license text modified overlaps license name

- always show license text input - display default license text as placeholder when a default license name is selected closes #2529 Signed-off-by: Maxim Stykow <[email protected]>
opossum-tool · Mar 21, 2024 · 4609ba9 · 4609ba9
1 parent 12a723d
commit 4609ba9
Show file tree

Hide file tree

Showing 9 changed files with 103 additions and 201 deletions.
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,58 +2,24 @@
 // 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 { sortBy } from 'lodash';
-import { useMemo, useState } from 'react';
+import { useMemo } from 'react';
 
 import { PackageInfo } from '../../../../shared/shared-types';
 import { text } from '../../../../shared/text';
 import { setTemporaryDisplayPackageInfo } from '../../../state/actions/resource-actions/all-views-simple-actions';
 import { useAppDispatch, useAppSelector } from '../../../state/hooks';
-import { getFrequentLicensesNameOrder } from '../../../state/selectors/resource-selectors';
+import {
+  getFrequentLicensesNameOrder,
+  getFrequentLicensesTexts,
+} from '../../../state/selectors/resource-selectors';
 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 +42,12 @@ export function LicenseSubPanel({
   packageInfo,
   showHighlight,
   onEdit,
-  expanded: expandedOverride,
+  expanded,
   hidden,
   config,
 }: LicenseSubPanelProps) {
   const dispatch = useAppDispatch();
-  const [expanded, setExpanded] = useState(expandedOverride);
+  const frequentLicenseTexts = useAppSelector(getFrequentLicensesTexts);
   const frequentLicensesNames = useAppSelector(getFrequentLicensesNameOrder);
   const defaultLicenses = useMemo(
     () =>
@@ -98,99 +64,54 @@ 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 defaultLicenseText = packageInfo.licenseText
+    ? undefined
+    : frequentLicenseTexts[packageInfo.licenseName || ''];
 
   return hidden ? null : (
-    <MuiBox sx={classes.panel}>
-      <MuiAccordion
-        sx={classes.expansionPanel}
-        elevation={0}
-        key={'License'}
-        disableGutters
+    <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}
+      />
+      <TextBox
+        readOnly={!onEdit}
+        placeholder={defaultLicenseText}
+        sx={classes.licenseText}
+        maxRows={8}
+        minRows={3}
+        color={config?.licenseText?.color}
+        focused={config?.licenseText?.focused}
+        multiline
         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>
-            )
-          }
-        >
-          <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>
+        title={
+          defaultLicenseText
+            ? text.attributionColumn.licenseTextDefault
+            : text.attributionColumn.licenseText
+        }
+        text={packageInfo.licenseText}
+        handleChange={({ target: { value } }) =>
+          onEdit?.(() =>
+            dispatch(
+              setTemporaryDisplayPackageInfo({
+                ...packageInfo,
+                licenseText: value,
+                wasPreferred: undefined,
+              }),
+            ),
+          )
+        }
+        endIcon={config?.licenseText?.endIcon}
+      />
     </MuiBox>
   );
 }
diff --git a/src/Frontend/Components/AttributionForm/__tests__/AttributionForm.test.tsx b/src/Frontend/Components/AttributionForm/__tests__/AttributionForm.test.tsx
@@ -6,11 +6,10 @@
 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,63 +200,61 @@ describe('AttributionForm', () => {
     expect(screen.queryByLabelText('Url icon')).not.toBeInTheDocument();
   });
 
-  it('shows standard text if editable and non frequent license', () => {
-    const packageInfo: PackageInfo = {
-      packageName: 'jQuery',
-      id: faker.string.uuid(),
-    };
+  it('shows default license text placeholder when frequent license name selected and no custom license text entered', () => {
+    const defaultLicenseText = faker.lorem.paragraphs();
+    const packageInfo = faker.opossum.packageInfo();
     renderComponent(
       <AttributionForm packageInfo={packageInfo} onEdit={jest.fn()} />,
+      {
+        actions: [
+          setFrequentLicenses({
+            nameOrder: [],
+            texts: { [packageInfo.licenseName!]: defaultLicenseText },
+          }),
+        ],
+      },
     );
 
     expect(
-      screen.getByLabelText('License Text (to appear in attribution document)'),
-    ).toBeInTheDocument();
-  });
-
-  it('shows shortened text if not editable and frequent license', () => {
-    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} />, {
-      actions: [
-        setFrequentLicenses(frequentLicenses),
-        setSelectedAttributionId(packageInfo.id),
-      ],
-    });
-
+      screen.getByRole('textbox', {
+        name: text.attributionColumn.licenseTextDefault,
+      }),
+    ).toHaveAttribute('placeholder', defaultLicenseText);
     expect(
-      screen.getByLabelText('Standard license text implied.'),
+      screen.getByLabelText(text.attributionColumn.licenseTextDefault),
     ).toBeInTheDocument();
+    expect(
+      screen.queryByLabelText(text.attributionColumn.licenseText),
+    ).not.toBeInTheDocument();
   });
 
-  it('shows long text if editable and frequent license', () => {
-    const packageInfo: PackageInfo = {
-      packageName: 'jQuery',
-      licenseName: 'mit',
-      id: faker.string.uuid(),
-    };
-    const frequentLicenses: FrequentLicenses = {
-      nameOrder: [{ shortName: 'MIT', fullName: 'MIT license' }],
-      texts: { MIT: 'text' },
-    };
+  it('does not show default license text placeholder when custom license text entered', () => {
+    const defaultLicenseText = faker.lorem.paragraphs();
+    const packageInfo = faker.opossum.packageInfo({
+      licenseText: faker.lorem.paragraphs(),
+    });
     renderComponent(
       <AttributionForm packageInfo={packageInfo} onEdit={jest.fn()} />,
       {
-        actions: [setFrequentLicenses(frequentLicenses)],
+        actions: [
+          setFrequentLicenses({
+            nameOrder: [],
+            texts: { [packageInfo.licenseName!]: defaultLicenseText },
+          }),
+        ],
       },
     );
 
     expect(
-      screen.getByLabelText(
-        'Standard license text implied. Insert notice text if necessary.',
-      ),
+      screen.getByRole('textbox', {
+        name: text.attributionColumn.licenseText,
+      }),
+    ).not.toHaveAttribute('placeholder', defaultLicenseText);
+    expect(
+      screen.queryByLabelText(text.attributionColumn.licenseTextDefault),
+    ).not.toBeInTheDocument();
+    expect(
+      screen.getByLabelText(text.attributionColumn.licenseText),
     ).toBeInTheDocument();
   });
 

diff --git a/src/Frontend/Components/TextBox/TextBox.tsx b/src/Frontend/Components/TextBox/TextBox.tsx
@@ -57,6 +57,7 @@ export const classes = {
 
 export interface TextBoxProps {
   color?: TextFieldProps['color'];
+  placeholder?: string;
   disabled?: boolean;
   expanded?: boolean;
   focused?: boolean;
@@ -70,7 +71,7 @@ export interface TextBoxProps {
   readOnly?: boolean;
   sx?: SxProps;
   text?: string;
-  title?: string;
+  title: string;
   endIcon?: React.ReactElement | Array<React.ReactElement>;
 }
 
@@ -79,13 +80,17 @@ export function TextBox(props: TextBoxProps) {
     <MuiBox sx={props.sx}>
       <MuiTextField
         disabled={props.disabled}
+        placeholder={props.placeholder}
         sx={{
           ...classes.textField,
           ...(props.error && classes.defaultHighlightedTextField),
         }}
         label={props.title}
         focused={props.focused}
         color={props.color}
+        InputLabelProps={{
+          shrink: !!props.placeholder || !!props.text,
+        }}
         InputProps={{
           readOnly: props.readOnly,
           slotProps: { root: { sx: { padding: 0 } } },