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.

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: respect settings for showing also in overview table #2861

Open
wants to merge 4 commits into
base: chore/user-config-handling-and-performance-fixes
Choose a base branch
from

Conversation

Hellgartner
Copy link
Contributor

Summary of changes

Also adapt the statistics table based on the configuration for showing classifications and criticality

Context and reason for change

See #2849

How can the changes be tested

Open the statistics popup and toggle the classifications/criticality on and off

Note: Please review the guidelines for contributing to this repository.

@Hellgartner Hellgartner linked an issue Mar 19, 2025 that may be closed by this pull request
@Hellgartner Hellgartner marked this pull request as ready for review March 20, 2025 07:37
Signed-off-by: Dominikus Hellgartner <dominikus.hellgartner@tngtech.com>
Signed-off-by: Dominikus Hellgartner <dominikus.hellgartner@tngtech.com>
only allow one channel to get the full user-settings information from the backend

Signed-off-by: Dominikus Hellgartner <dominikus.hellgartner@tngtech.com>
@Hellgartner Hellgartner force-pushed the feat/toggle-also-statistics-table branch from 63855e9 to fa3605e Compare March 20, 2025 15:30
Fixed bug:
* User selects one of the switchable columns for ordering
* User switches that column off
* User clicks on Name
Without the fix, the sorting direction for names would not change as expected

Signed-off-by: Dominikus Hellgartner <dominikus.hellgartner@tngtech.com>
@Hellgartner Hellgartner requested a review from PhilippMa March 20, 2025 15:48
@@ -9,13 +9,19 @@ import upath from 'upath';

import { getIconPath } from './iconHelpers';

const openDevTools = (mainWindow: BrowserWindow) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you commit this on purpose?

orderDirection:
effectiveOrdering.orderDirection === 'asc' ? 'desc' : 'asc',
});
} else if (ordering.orderedColumn === columnId) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we not just go into the first case regardless of whether effectiveOrdering equals ordering?

Comment on lines +59 to +71
describe('Attribution count per source per license table', () => {
it('shows by default criticality and classification columns', () => {
renderComponent(<AttributionCountPerSourcePerLicenseTable {...props} />);

expectHeaderTextsToEqual(screen, [
'Namesorted ascending', //correct, the sorted, ascending is for a11y
'Criticality',
'Classification',
'SourceA',
'SourceB',
'Total',
]);
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a way to get rid of the linter warnings that these tests have no assertions? Maybe it's better to move the expect statement out of the function back into the test cases themselves?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make display of classification and criticality optional
2 participants