From b37a4b3c4098ec3e760a79703272a50bd446c8f1 Mon Sep 17 00:00:00 2001 From: Maxim Stykow Date: Fri, 17 May 2024 17:08:31 +0200 Subject: [PATCH] feat: collapse license text by default - prevent license text from taking up vertical space by introducing toggle button to show/hide it - rename "package" to "component" for OSS naming consistency - make sure icon button keyboard-focus style is separate from click-focus style Signed-off-by: Maxim Stykow --- src/Frontend/Components/App/App.style.ts | 14 +++ .../LicenseSubPanel/LicenseSubPanel.tsx | 109 +++++++++++------- .../__tests__/AttributionForm.test.tsx | 8 +- .../__tests__/GoToLinkButton.test.tsx | 3 +- .../Components/IconButton/IconButton.tsx | 2 +- .../comparing-attribution-with-origin.test.ts | 3 + .../__tests__/updating-attributions.test.ts | 1 + src/e2e-tests/page-objects/AttributionForm.ts | 4 + src/shared/text.ts | 22 ++-- 9 files changed, 108 insertions(+), 58 deletions(-) diff --git a/src/Frontend/Components/App/App.style.ts b/src/Frontend/Components/App/App.style.ts index 08aa00938..30c57ff23 100644 --- a/src/Frontend/Components/App/App.style.ts +++ b/src/Frontend/Components/App/App.style.ts @@ -70,6 +70,20 @@ export const theme = createTheme({ }, }, components: { + MuiInputBase: { + styleOverrides: { + root: { + minHeight: '36px !important', + }, + }, + }, + MuiToggleButton: { + styleOverrides: { + root: { + padding: '5px', + }, + }, + }, MuiSwitch: { styleOverrides: { switchBase: { diff --git a/src/Frontend/Components/AttributionForm/LicenseSubPanel/LicenseSubPanel.tsx b/src/Frontend/Components/AttributionForm/LicenseSubPanel/LicenseSubPanel.tsx index ed08de0dc..0696c33b0 100644 --- a/src/Frontend/Components/AttributionForm/LicenseSubPanel/LicenseSubPanel.tsx +++ b/src/Frontend/Components/AttributionForm/LicenseSubPanel/LicenseSubPanel.tsx @@ -2,9 +2,11 @@ // SPDX-FileCopyrightText: TNG Technology Consulting GmbH // // SPDX-License-Identifier: Apache-2.0 +import NotesIcon from '@mui/icons-material/Notes'; +import { Badge, ToggleButton } from '@mui/material'; import MuiBox from '@mui/material/Box'; import { sortBy } from 'lodash'; -import { useMemo } from 'react'; +import { useMemo, useState } from 'react'; import { PackageInfo } from '../../../../shared/shared-types'; import { text } from '../../../../shared/text'; @@ -46,6 +48,7 @@ export function LicenseSubPanel({ hidden, config, }: LicenseSubPanelProps) { + const [showLicenseText, setShowLicenseText] = useState(false); const dispatch = useAppDispatch(); const frequentLicenseTexts = useAppSelector(getFrequentLicensesTexts); const frequentLicensesNames = useAppSelector(getFrequentLicensesNameOrder); @@ -71,47 +74,69 @@ export function LicenseSubPanel({ return hidden ? null : ( - - - onEdit?.(() => - dispatch( - setTemporaryDisplayPackageInfo({ - ...packageInfo, - licenseText: value, - wasPreferred: undefined, - }), - ), - ) - } - endIcon={config?.licenseText?.endIcon} - /> + + + {!expanded && ( + setShowLicenseText((prev) => !prev)} + size={'small'} + aria-label="license-text-toggle-button" + > + + + + + )} + + {(showLicenseText || expanded) && ( + + 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 ba7a84a46..4fd5aca3e 100644 --- a/src/Frontend/Components/AttributionForm/__tests__/AttributionForm.test.tsx +++ b/src/Frontend/Components/AttributionForm/__tests__/AttributionForm.test.tsx @@ -200,7 +200,7 @@ describe('AttributionForm', () => { expect(screen.queryByLabelText('Url icon')).not.toBeInTheDocument(); }); - it('shows default license text placeholder when frequent license name selected and no custom license text entered', () => { + it('shows default license text placeholder when frequent license name selected and no custom license text entered', async () => { const defaultLicenseText = faker.lorem.paragraphs(); const packageInfo = faker.opossum.packageInfo(); renderComponent( @@ -215,6 +215,8 @@ describe('AttributionForm', () => { }, ); + await userEvent.click(screen.getByLabelText('license-text-toggle-button')); + expect( screen.getByRole('textbox', { name: text.attributionColumn.licenseTextDefault, @@ -228,7 +230,7 @@ describe('AttributionForm', () => { ).not.toBeInTheDocument(); }); - it('does not show default license text placeholder when custom license text entered', () => { + it('does not show default license text placeholder when custom license text entered', async () => { const defaultLicenseText = faker.lorem.paragraphs(); const packageInfo = faker.opossum.packageInfo({ licenseText: faker.lorem.paragraphs(), @@ -245,6 +247,8 @@ describe('AttributionForm', () => { }, ); + await userEvent.click(screen.getByLabelText('license-text-toggle-button')); + expect( screen.getByRole('textbox', { name: text.attributionColumn.licenseText, diff --git a/src/Frontend/Components/GoToLinkButton/__tests__/GoToLinkButton.test.tsx b/src/Frontend/Components/GoToLinkButton/__tests__/GoToLinkButton.test.tsx index 5d290c4be..126b24cf8 100644 --- a/src/Frontend/Components/GoToLinkButton/__tests__/GoToLinkButton.test.tsx +++ b/src/Frontend/Components/GoToLinkButton/__tests__/GoToLinkButton.test.tsx @@ -3,8 +3,7 @@ // SPDX-FileCopyrightText: Nico Carl // // SPDX-License-Identifier: Apache-2.0 -import { fireEvent, screen } from '@testing-library/react'; -import { act } from 'react-dom/test-utils'; +import { act, fireEvent, screen } from '@testing-library/react'; import { BaseUrlsForSources } from '../../../../shared/shared-types'; import { setBaseUrlsForSources } from '../../../state/actions/resource-actions/all-views-simple-actions'; diff --git a/src/Frontend/Components/IconButton/IconButton.tsx b/src/Frontend/Components/IconButton/IconButton.tsx index 576cbea17..d05c4851e 100644 --- a/src/Frontend/Components/IconButton/IconButton.tsx +++ b/src/Frontend/Components/IconButton/IconButton.tsx @@ -41,7 +41,7 @@ export function IconButton(props: IconButtonProps) { disabled={props.disabled} data-testid={props['data-testid']} sx={{ - '&:focus': { + '&.Mui-focusVisible': { background: OpossumColors.middleBlue, }, }} diff --git a/src/e2e-tests/__tests__/comparing-attribution-with-origin.test.ts b/src/e2e-tests/__tests__/comparing-attribution-with-origin.test.ts index 81c1c8c3b..52d1c1643 100644 --- a/src/e2e-tests/__tests__/comparing-attribution-with-origin.test.ts +++ b/src/e2e-tests/__tests__/comparing-attribution-with-origin.test.ts @@ -93,6 +93,7 @@ test('reverts all changes and applies reverted state to temporary package info', await diffPopup.revertAllButton.click(); await diffPopup.applyButton.click(); await diffPopup.assert.isHidden(); + await attributionDetails.attributionForm.licenseTextToggleButton.click(); await attributionDetails.attributionForm.assert.matchesPackageInfo( manualPackageInfo2, ); @@ -132,6 +133,7 @@ test('reverts single fields correctly', async ({ await diffPopup.revertAllButton.click(); await diffPopup.applyButton.click(); await diffPopup.assert.isHidden(); + await attributionDetails.attributionForm.licenseTextToggleButton.click(); await attributionDetails.attributionForm.assert.matchesPackageInfo( manualPackageInfo2, ); @@ -170,6 +172,7 @@ test('handles pending license and copyright changes in temporary package info co await attributionDetails.attributionForm.selectAttributionType( AttributionType.ThirdParty, ); + await attributionDetails.attributionForm.licenseTextToggleButton.click(); await attributionDetails.attributionForm.assert.matchesPackageInfo( manualPackageInfo2, ); diff --git a/src/e2e-tests/__tests__/updating-attributions.test.ts b/src/e2e-tests/__tests__/updating-attributions.test.ts index 93bc92228..245176ca0 100644 --- a/src/e2e-tests/__tests__/updating-attributions.test.ts +++ b/src/e2e-tests/__tests__/updating-attributions.test.ts @@ -133,6 +133,7 @@ test('allows user to update an attribution on the selected resource only', async await attributionDetails.assert.saveButtonIsDisabled(); await attributionDetails.assert.revertButtonIsDisabled(); + await attributionDetails.attributionForm.licenseTextToggleButton.click(); await attributionDetails.attributionForm.licenseText.fill( newPackageInfo.licenseText!, ); diff --git a/src/e2e-tests/page-objects/AttributionForm.ts b/src/e2e-tests/page-objects/AttributionForm.ts index 610b9c13d..4fa552cb4 100644 --- a/src/e2e-tests/page-objects/AttributionForm.ts +++ b/src/e2e-tests/page-objects/AttributionForm.ts @@ -21,6 +21,7 @@ export class AttributionForm { readonly copyright: Locator; readonly licenseName: Locator; readonly licenseText: Locator; + readonly licenseTextToggleButton: Locator; readonly attributionType: Locator; readonly addAuditingOptionButton: Locator; readonly auditingOptionsMenu: { @@ -81,6 +82,9 @@ export class AttributionForm { exact: true, }, ); + this.licenseTextToggleButton = this.node.getByLabel( + 'license-text-toggle-button', + ); this.licenseText = this.node.getByLabel( text.attributionColumn.licenseText, { exact: true }, diff --git a/src/shared/text.ts b/src/shared/text.ts index 02151661f..f52b8bdfa 100644 --- a/src/shared/text.ts +++ b/src/shared/text.ts @@ -12,10 +12,10 @@ export const text = { copyToClipboard: 'Copy to clipboard', copyToClipboardSuccess: 'Copied to clipboard', currentLicenseInformation: 'Current License Information', - currentPackageCoordinates: 'Current Package Coordinates', + currentPackageCoordinates: 'Current Component Coordinates', delete: 'Delete', description: 'Description', - enrichFailure: 'No package found with these coordinates', + enrichFailure: 'No component found with these coordinates', enrichNoop: 'No more information found', enrichSuccess: 'Added information where possible', fromAttributions: 'From Attributions', @@ -30,23 +30,23 @@ export const text = { link: 'Link as attribution on selected resource', noLinkToOpen: 'No link to open. Please enter a URL.', occurrence: 'occurrence', - openLinkInBrowser: 'Open repository URL in browser', + openLinkInBrowser: 'Open component URL in browser', openSourceInsights: 'Open Source Insights', origin: 'Origin', originalLicenseInformation: 'Original License Information', - originalPackageCoordinates: 'Original Package Coordinates', + originalPackageCoordinates: 'Original Component Coordinates', originallyFrom: 'Originally from ', - packageCoordinates: 'Package Coordinates', - packageName: 'Package Name', - packageNamespace: 'Package Namespace', - packageType: 'Package Type', - packageVersion: 'Package Version', + packageCoordinates: 'Component Coordinates', + packageName: 'Component Name', + packageNamespace: 'Component Namespace', + packageType: 'Component Type', + packageVersion: 'Component Version', pasteFromClipboard: 'Paste from clipboard', pasteFromClipboardFailed: 'Clipboard does not contain a valid PURL', pasteFromClipboardSuccess: 'Pasted from clipboard', purl: 'PURL', replace: 'Use as replacement', - repositoryUrl: 'Repository URL', + repositoryUrl: 'Component URL', restore: 'Restore', revert: 'Revert', save: 'Save', @@ -197,7 +197,7 @@ export const text = { excludedFromNotice: 'Excluded from Notice', firstParty: 'First Party', highConfidence: 'High Confidence', - incompleteCoordinates: 'Incomplete Package Coordinates', + incompleteCoordinates: 'Incomplete Component Coordinates', incompleteLegal: 'Incomplete Legal Information', lowConfidence: 'Low Confidence', modifiedPreferred: 'Modified Previously Preferred',