Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: collapse license text by default #2636

Merged
merged 1 commit into from
May 17, 2024
Merged

feat: collapse license text by default #2636

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions src/Frontend/Components/App/App.style.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,20 @@ export const theme = createTheme({
},
},
components: {
MuiInputBase: {
styleOverrides: {
root: {
minHeight: '36px !important',
},
},
},
MuiToggleButton: {
styleOverrides: {
root: {
padding: '5px',
},
},
},
MuiSwitch: {
styleOverrides: {
switchBase: {
Expand Down
109 changes: 67 additions & 42 deletions src/Frontend/Components/AttributionForm/LicenseSubPanel/LicenseSubPanel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@
// SPDX-FileCopyrightText: TNG Technology Consulting GmbH <https://www.tngtech.com>
//
// 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';
Expand Down Expand Up @@ -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);
Expand All @@ -71,47 +74,69 @@ export function LicenseSubPanel({

return hidden ? null : (
<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}
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 display={'flex'} alignItems={'center'} gap={'8px'}>
<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}
/>
{!expanded && (
<ToggleButton
value={'license-text'}
selected={showLicenseText}
onChange={() => setShowLicenseText((prev) => !prev)}
size={'small'}
aria-label="license-text-toggle-button"
>
<Badge
anchorOrigin={{ vertical: 'bottom', horizontal: 'right' }}
color={'info'}
variant={'dot'}
invisible={!packageInfo.licenseText}
>
<NotesIcon />
Comment on lines +98 to +104
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Subjective: I think it could also be nice to just give the color to the Icon, instead of adding a badge. But go with what you think is best.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i tried color first but that's not how the MUI toggle button is designed: it will only display the color once it's active

</Badge>
</ToggleButton>
)}
</MuiBox>
{(showLicenseText || expanded) && (
<TextBox
readOnly={!onEdit}
placeholder={defaultLicenseText}
sx={classes.licenseText}
maxRows={8}
minRows={3}
color={config?.licenseText?.color}
focused={config?.licenseText?.focused}
multiline
expanded={expanded}
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>
);
}
8 changes: 6 additions & 2 deletions src/Frontend/Components/AttributionForm/__tests__/AttributionForm.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -215,6 +215,8 @@ describe('AttributionForm', () => {
},
);

await userEvent.click(screen.getByLabelText('license-text-toggle-button'));

expect(
screen.getByRole('textbox', {
name: text.attributionColumn.licenseTextDefault,
Expand All @@ -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(),
Expand All @@ -245,6 +247,8 @@ describe('AttributionForm', () => {
},
);

await userEvent.click(screen.getByLabelText('license-text-toggle-button'));

expect(
screen.getByRole('textbox', {
name: text.attributionColumn.licenseText,
Expand Down
3 changes: 1 addition & 2 deletions src/Frontend/Components/GoToLinkButton/__tests__/GoToLinkButton.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@
// SPDX-FileCopyrightText: Nico Carl <nicocarl@protonmail.com>
//
// 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';
Expand Down
2 changes: 1 addition & 1 deletion src/Frontend/Components/IconButton/IconButton.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export function IconButton(props: IconButtonProps) {
disabled={props.disabled}
data-testid={props['data-testid']}
sx={{
'&:focus': {
'&.Mui-focusVisible': {
background: OpossumColors.middleBlue,
},
}}
Expand Down
3 changes: 3 additions & 0 deletions src/e2e-tests/__tests__/comparing-attribution-with-origin.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);
Expand Down Expand Up @@ -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,
);
Expand Down Expand Up @@ -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,
);
Expand Down
1 change: 1 addition & 0 deletions src/e2e-tests/__tests__/updating-attributions.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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!,
);
Expand Down
4 changes: 4 additions & 0 deletions src/e2e-tests/page-objects/AttributionForm.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down Expand Up @@ -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 },
Expand Down
22 changes: 11 additions & 11 deletions src/shared/text.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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',
Expand Down Expand Up @@ -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',
Expand Down
Loading