From 4609ba9c7e094452a16e76555fb96515921e6ab1 Mon Sep 17 00:00:00 2001 From: Maxim Stykow Date: Wed, 13 Mar 2024 15:56:48 +0100 Subject: [PATCH] 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 --- .../AttributionForm/Comment/Comment.tsx | 2 +- .../CopyrightSubPanel/CopyrightSubPanel.tsx | 2 +- .../LicenseSubPanel/LicenseSubPanel.tsx | 183 +++++------------- .../__tests__/AttributionForm.test.tsx | 83 ++++---- src/Frontend/Components/TextBox/TextBox.tsx | 7 +- .../comparing-attribution-with-origin.test.ts | 3 - .../__tests__/updating-attributions.test.ts | 7 - src/e2e-tests/page-objects/AttributionForm.ts | 14 +- src/shared/text.ts | 3 +- 9 files changed, 103 insertions(+), 201 deletions(-) diff --git a/src/Frontend/Components/AttributionForm/Comment/Comment.tsx b/src/Frontend/Components/AttributionForm/Comment/Comment.tsx index d84e4afab..d18f3ab93 100644 --- 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 index 6cf0b0d93..424967f06 100644 --- 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 index 0277693c9..ed08de0dc 100644 --- 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 // // 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 : ( - - + + - setExpanded((prev) => !prev)} - > - - - ) - } - > - - {text.attributionColumn.licenseTextModified} - - ) : undefined) - } - defaults={defaultLicenses} - color={config?.licenseName?.color} - focused={config?.licenseName?.focused} - /> - - - - onEdit?.(() => - dispatch( - setTemporaryDisplayPackageInfo({ - ...packageInfo, - licenseText: value, - wasPreferred: undefined, - }), - ), - ) - } - endIcon={config?.licenseText?.endIcon} - /> - - + 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} + /> ); } diff --git a/src/Frontend/Components/AttributionForm/__tests__/AttributionForm.test.tsx b/src/Frontend/Components/AttributionForm/__tests__/AttributionForm.test.tsx index d30a9032d..ba7a84a46 100644 --- 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( , + { + 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(, { - 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( , { - 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 index 1573c0856..8f194c064 100644 --- 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; } @@ -79,6 +80,7 @@ export function TextBox(props: TextBoxProps) { => { await expect(this.licenseText).toHaveValue(licenseText); }, - licenseTextIsVisible: async (): Promise => { - await expect(this.licenseText).toBeVisible(); - }, - licenseTextIsHidden: async (): Promise => { - await expect(this.licenseText).toBeHidden(); - }, commentIs: async (comment: string): Promise => { await expect(this.comment).toHaveValue(comment); }, @@ -299,10 +291,6 @@ export class AttributionForm { await this.window.keyboard.press('Escape'); } - async toggleLicenseTextVisibility(): Promise { - await this.licenseTextToggleButton.click(); - } - async selectLicense(license: RawFrequentLicense): Promise { await this.window.getByText(license.fullName).click(); } diff --git a/src/shared/text.ts b/src/shared/text.ts index cb13edf37..87b4ee907 100644 --- a/src/shared/text.ts +++ b/src/shared/text.ts @@ -25,7 +25,8 @@ export const text = { invalidPurl: 'INVALID PURL', legalInformation: 'Legal Information', licenseName: 'License Name', - licenseTextModified: '(License text modified)', + licenseText: 'License Text', + licenseTextDefault: 'License Text (inferred from license name)', link: 'Link as attribution on selected resource', noLinkToOpen: 'No link to open. Please enter a URL.', occurrence: 'occurrence',