change: table headers sorting icons

greenbone · Jan 6, 2025 · 850c1ad · 850c1ad
1 parent 3299b8f
commit 850c1ad
Show file tree

Hide file tree

Showing 18 changed files with 219 additions and 153 deletions.
diff --git a/src/web/components/icon/ArrowDown.jsx b/src/web/components/icon/ArrowDown.jsx
@@ -0,0 +1,19 @@
+/* SPDX-FileCopyrightText: 2024 Greenbone AG
+ *
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+import {ArrowDown as Icon} from 'lucide-react';
+import IconWithStrokeWidth from 'web/components/icon/IconWithStrokeWidth';
+
+import withSvgIcon from './withSvgIcon';
+
+const ArrowDown = withSvgIcon()(props => (
+  <IconWithStrokeWidth
+    IconComponent={Icon}
+    {...props}
+    data-testid="arrowDown-icon"
+  />
+));
+
+export default ArrowDown;
diff --git a/src/web/components/icon/ArrowUp.jsx b/src/web/components/icon/ArrowUp.jsx
@@ -0,0 +1,19 @@
+/* SPDX-FileCopyrightText: 2024 Greenbone AG
+ *
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+import {ArrowUp as Icon} from 'lucide-react';
+import IconWithStrokeWidth from 'web/components/icon/IconWithStrokeWidth';
+
+import withSvgIcon from './withSvgIcon';
+
+const ArrowUp = withSvgIcon()(props => (
+  <IconWithStrokeWidth
+    IconComponent={Icon}
+    {...props}
+    data-testid="arrowUp-icon"
+  />
+));
+
+export default ArrowUp;
diff --git a/src/web/components/icon/ArrowUpDown.jsx b/src/web/components/icon/ArrowUpDown.jsx
@@ -0,0 +1,19 @@
+/* SPDX-FileCopyrightText: 2024 Greenbone AG
+ *
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+import {ArrowUpDown as Icon} from 'lucide-react';
+import IconWithStrokeWidth from 'web/components/icon/IconWithStrokeWidth';
+
+import withSvgIcon from './withSvgIcon';
+
+const ArrowUpDown = withSvgIcon()(props => (
+  <IconWithStrokeWidth
+    IconComponent={Icon}
+    {...props}
+    data-testid="arrowUpDown-icon"
+  />
+));
+
+export default ArrowUpDown;
diff --git a/src/web/components/label/__tests__/severityclass.jsx b/src/web/components/label/__tests__/severityclass.jsx
@@ -5,6 +5,7 @@
 
 import {describe, test, expect} from '@gsa/testing';
 import {render} from 'web/utils/testing';
+import Theme from 'web/utils/theme';
 
 import SeverityClassLabel from '../severityclass';
 
@@ -18,40 +19,48 @@ describe('SeverityClassLabel tests', () => {
   test('should render HighLabel', () => {
     const {element} = render(<SeverityClassLabel.High />);
 
-    expect(element).toHaveStyleRule('background-color', '#C83814');
-    expect(element).toHaveStyleRule('border-color', '#C83814');
+    expect(element).toHaveStyleRule('background-color', Theme.errorRed);
+    expect(element).toHaveStyleRule('border-color', Theme.errorRed);
+    expect(element).toHaveStyleRule('color', Theme.white);
     expect(element).toHaveTextContent('High');
   });
 
   test('should render MediumLabel', () => {
     const {element} = render(<SeverityClassLabel.Medium />);
 
-    expect(element).toHaveStyleRule('background-color', '#F0A519');
-    expect(element).toHaveStyleRule('border-color', '#F0A519');
+    expect(element).toHaveStyleRule(
+      'background-color',
+      Theme.severityWarnYellow,
+    );
+    expect(element).toHaveStyleRule('border-color', Theme.severityWarnYellow);
+    expect(element).toHaveStyleRule('color', Theme.black);
     expect(element).toHaveTextContent('Medium');
   });
 
   test('should render LowLabel', () => {
     const {element} = render(<SeverityClassLabel.Low />);
 
-    expect(element).toHaveStyleRule('background-color', '#4F91C7');
-    expect(element).toHaveStyleRule('border-color', '#4F91C7');
+    expect(element).toHaveStyleRule('background-color', Theme.severityLowBlue);
+    expect(element).toHaveStyleRule('border-color', Theme.severityLowBlue);
+    expect(element).toHaveStyleRule('color', Theme.white);
     expect(element).toHaveTextContent('Low');
   });
 
   test('should render LogLabel', () => {
     const {element} = render(<SeverityClassLabel.Log />);
 
-    expect(element).toHaveStyleRule('background-color', '#191919');
-    expect(element).toHaveStyleRule('border-color', '#191919');
+    expect(element).toHaveStyleRule('background-color', Theme.mediumGray);
+    expect(element).toHaveStyleRule('border-color', Theme.mediumGray);
+    expect(element).toHaveStyleRule('color', Theme.white);
     expect(element).toHaveTextContent('Log');
   });
 
   test('should render FalsePositiveLabel', () => {
     const {element} = render(<SeverityClassLabel.FalsePositive />);
 
-    expect(element).toHaveStyleRule('background-color', '#191919');
-    expect(element).toHaveStyleRule('border-color', '#191919');
+    expect(element).toHaveStyleRule('background-color', Theme.mediumGray);
+    expect(element).toHaveStyleRule('border-color', Theme.mediumGray);
+    expect(element).toHaveStyleRule('color', Theme.white);
     expect(element).toHaveTextContent('False Pos.');
   });
 });
diff --git a/src/web/components/sortby/sortby.jsx b/src/web/components/sortby/sortby.jsx
@@ -7,39 +7,33 @@ import React from 'react';
 import styled from 'styled-components';
 import PropTypes from 'web/utils/proptypes';
 
-const Anchor = styled.a`
+const SortButton = styled.button`
   cursor: pointer;
+  background: none;
+  border: none;
+  padding: 0;
+  font: inherit;
 `;
 
-class SortBy extends React.Component {
-  static ASC = 'asc';
-  static DESC = 'desc';
-
-  constructor(...args) {
-    super(...args);
-
-    this.handleClick = this.handleClick.bind(this);
-  }
-
-  handleClick() {
-    const {by, onClick} = this.props;
-
+const SortBy = ({by, children, className, onClick}) => {
+  const handleClick = () => {
     if (onClick) {
       onClick(by);
     }
-  }
+  };
+
+  return (
+    <SortButton className={className} onClick={handleClick}>
+      {children}
+    </SortButton>
+  );
+};
 
-  render() {
-    const {children, className} = this.props;
-    return (
-      <Anchor className={className} onClick={this.handleClick}>
-        {children}
-      </Anchor>
-    );
-  }
-}
+SortBy.ASC = 'asc';
+SortBy.DESC = 'desc';
 
 SortBy.propTypes = {
+  children: PropTypes.node,
   by: PropTypes.string,
   className: PropTypes.string,
   onClick: PropTypes.func,

diff --git a/src/web/components/table/head.jsx b/src/web/components/table/head.jsx
@@ -7,11 +7,19 @@ import _ from 'gmp/locale';
 import {isDefined} from 'gmp/utils/identity';
 import React from 'react';
 import styled from 'styled-components';
+import ArrowDown from 'web/components/icon/ArrowDown';
+import ArrowUp from 'web/components/icon/ArrowUp';
+import ArrowUpDown from 'web/components/icon/ArrowUpDown';
 import Layout from 'web/components/layout/layout';
 import Sort from 'web/components/sortby/sortby';
 import PropTypes from 'web/utils/proptypes';
 import Theme from 'web/utils/theme';
 
+const SortSymbol = styled.span`
+  display: inline-flex;
+  align-items: center;
+`;
+
 const TableHead = ({
   children,
   className,
@@ -26,42 +34,39 @@ const TableHead = ({
   onSortChange,
   ...other
 }) => {
-  let sortSymbol;
-  if (isDefined(sortBy) && currentSortBy === sortBy) {
-    if (currentSortDir === Sort.DESC) {
-      sortSymbol = // triangle pointing down
-        (
-          <span
-            title={_('Sorted In Descending Order By {{sortBy}}', {
-              sortBy: `${title}`,
-            })}
-          >
-            &nbsp;&#9660;
-          </span>
-        );
-    } else if (currentSortDir === Sort.ASC) {
-      sortSymbol = // triangle pointing up
-        (
-          <span
-            title={_('Sorted In Ascending Order By {{sortBy}}', {
-              sortBy: `${title}`,
-            })}
-          >
-            &nbsp;&#9650;
-          </span>
-        );
+  const getSortSymbol = () => {
+    if (!isDefined(sortBy) || currentSortBy !== sortBy) {
+      return (
+        <SortSymbol title={_('Not Sorted')}>
+          &nbsp;
+          <ArrowUpDown />
+        </SortSymbol>
+      );
     }
-  }
+    const titleText =
+      currentSortDir === Sort.DESC
+        ? _('Sorted In Descending Order By {{sortBy}}', {sortBy: `${title}`})
+        : _('Sorted In Ascending Order By {{sortBy}}', {sortBy: `${title}`});
+    const Icon = currentSortDir === Sort.DESC ? ArrowDown : ArrowUp;
+    return (
+      <SortSymbol title={titleText}>
+        &nbsp;
+        <Icon />
+      </SortSymbol>
+    );
+  };
+
   if (isDefined(title) && !isDefined(children)) {
     children = `${title}`;
   }
+
   return (
     <th className={className} colSpan={colSpan} rowSpan={rowSpan}>
       {sort && sortBy && isDefined(onSortChange) ? (
         <Sort by={sortBy} onClick={onSortChange}>
           <Layout {...other}>
             {children}
-            {sortSymbol}
+            {getSortSymbol()}
           </Layout>
         </Sort>
       ) : (
@@ -72,6 +77,7 @@ const TableHead = ({
 };
 
 TableHead.propTypes = {
+  children: PropTypes.node,
   className: PropTypes.string,
   colSpan: PropTypes.numberString,
   currentSortBy: PropTypes.string,

diff --git a/src/web/pages/alerts/__tests__/listpage.jsx b/src/web/pages/alerts/__tests__/listpage.jsx
@@ -29,7 +29,6 @@ import {rendererWith, fireEvent, screen, wait} from 'web/utils/testing';
 
 import AlertPage, {ToolBarIcons} from '../listpage';
 
-
 const caps = new Capabilities(['everything']);
 const wrongCaps = new Capabilities(['get_config']);
 
@@ -184,11 +183,11 @@ describe('Alert listpage tests', () => {
     expect(row[1]).toHaveTextContent('report results filter');
     expect(row[1]).toHaveTextContent('Yes');
 
-    expect(icons[13]).toHaveAttribute('title', 'Move Alert to trashcan');
-    expect(icons[14]).toHaveAttribute('title', 'Edit Alert');
-    expect(icons[15]).toHaveAttribute('title', 'Clone Alert');
-    expect(icons[16]).toHaveAttribute('title', 'Export Alert');
-    expect(icons[17]).toHaveAttribute('title', 'Test Alert');
+    expect(icons[19]).toHaveAttribute('title', 'Move Alert to trashcan');
+    expect(icons[20]).toHaveAttribute('title', 'Edit Alert');
+    expect(icons[21]).toHaveAttribute('title', 'Clone Alert');
+    expect(icons[22]).toHaveAttribute('title', 'Export Alert');
+    expect(icons[23]).toHaveAttribute('title', 'Test Alert');
   });
 
   test('should allow to bulk action on page contents', async () => {

diff --git a/src/web/pages/reports/__tests__/auditdeltadetailspage.jsx b/src/web/pages/reports/__tests__/auditdeltadetailspage.jsx
@@ -319,7 +319,7 @@ describe('Audit Detla Report Details Content tests', () => {
     const bars = getAllByTestId('progressbar-box');
 
     // Toolbar Icons
-    expect(icons.length).toEqual(25);
+    expect(icons.length).toEqual(33);
 
     // Powerfilter
     expect(inputs[0]).toHaveAttribute('name', 'userFilterString');

diff --git a/src/web/pages/reports/__tests__/auditreportslistpage.jsx b/src/web/pages/reports/__tests__/auditreportslistpage.jsx
@@ -256,14 +256,13 @@ describe('AuditReportsPage tests', () => {
       entitiesActions.success([entity], filter, loadedFilter, counts),
     );
 
-    const {baseElement, getAllByTestId} = render(<AuditReportsPage />);
+    const {baseElement, getByTestId} = render(<AuditReportsPage />);
 
     await waitFor(() => baseElement.querySelectorAll('table'));
 
-    const icons = getAllByTestId('svg-icon');
-
-    expect(icons[19]).toHaveAttribute('title', 'Add tag to page contents');
-    fireEvent.click(icons[19]);
+    const icon = getByTestId('tags-icon');
+    expect(icon).toHaveAttribute('title', 'Add tag to page contents');
+    fireEvent.click(icon);
     expect(getAll).toHaveBeenCalled();
 
     fireEvent.click(screen.getAllByTitle('Delete page contents')[0]);

diff --git a/src/web/pages/reports/details/__tests__/applicationstab.jsx b/src/web/pages/reports/details/__tests__/applicationstab.jsx
@@ -60,16 +60,16 @@ describe('Report Applications Tab tests', () => {
     expect(header[3]).toHaveTextContent('Severity');
 
     // Row 1
-    expect(links[4]).toHaveAttribute('href', '/cpe/cpe%3A%2Fa%3A%20123');
-    expect(links[4]).toHaveTextContent('123');
+    expect(links[0]).toHaveAttribute('href', '/cpe/cpe%3A%2Fa%3A%20123');
+    expect(links[0]).toHaveTextContent('123');
     expect(images[0]).toHaveAttribute('src', '/img/cpe/other.svg');
     expect(rows[1]).toHaveTextContent('22'); // 2 Hosts, 2 Occurrences
     expect(bars[0]).toHaveAttribute('title', 'High');
     expect(bars[0]).toHaveTextContent('10.0 (High)');
 
     // Row 2
-    expect(links[5]).toHaveAttribute('href', '/cpe/cpe%3A%2Fa%3A%20456');
-    expect(links[5]).toHaveTextContent('456');
+    expect(links[1]).toHaveAttribute('href', '/cpe/cpe%3A%2Fa%3A%20456');
+    expect(links[1]).toHaveTextContent('456');
     expect(images[1]).toHaveAttribute('src', '/img/cpe/other.svg');
     expect(rows[2]).toHaveTextContent('11'); // 1 Hosts, 1 Occurrences
     expect(bars[1]).toHaveAttribute('title', 'Medium');

diff --git a/src/web/pages/reports/details/__tests__/closedcvestab.jsx b/src/web/pages/reports/details/__tests__/closedcvestab.jsx
@@ -57,25 +57,25 @@ describe('Report Closed CVEs Tab tests', () => {
     expect(header[3]).toHaveTextContent('Severity');
 
     // Row 1
-    expect(links[4]).toHaveAttribute('href', '/cve/CVE-2000-1234');
-    expect(links[4]).toHaveTextContent('CVE-2000-1234');
-    expect(links[5]).toHaveAttribute('href', '/host/123');
-    expect(links[5]).toHaveTextContent('123.456.78.910');
-    expect(links[6]).toHaveAttribute('href', '/nvt/201');
-    expect(links[6]).toHaveTextContent('This is a description');
+    expect(links[0]).toHaveAttribute('href', '/cve/CVE-2000-1234');
+    expect(links[0]).toHaveTextContent('CVE-2000-1234');
+    expect(links[1]).toHaveAttribute('href', '/host/123');
+    expect(links[1]).toHaveTextContent('123.456.78.910');
+    expect(links[2]).toHaveAttribute('href', '/nvt/201');
+    expect(links[2]).toHaveTextContent('This is a description');
     expect(bars[0]).toHaveAttribute('title', 'High');
     expect(bars[0]).toHaveTextContent('10.0 (High)');
 
     // Row 2
-    expect(links[7]).toHaveAttribute('href', '/cve/CVE-2000-5678');
-    expect(links[7]).toHaveTextContent('CVE-2000-5678');
-    expect(links[8]).toHaveAttribute(
+    expect(links[3]).toHaveAttribute('href', '/cve/CVE-2000-5678');
+    expect(links[3]).toHaveTextContent('CVE-2000-5678');
+    expect(links[4]).toHaveAttribute(
       'href',
       '/hosts?filter=name%3D109.876.54.321',
     ); // because the host has no asset id
-    expect(links[8]).toHaveTextContent('109.876.54.321');
-    expect(links[9]).toHaveAttribute('href', '/nvt/202');
-    expect(links[9]).toHaveTextContent('This is another description');
+    expect(links[4]).toHaveTextContent('109.876.54.321');
+    expect(links[5]).toHaveAttribute('href', '/nvt/202');
+    expect(links[5]).toHaveTextContent('This is another description');
     expect(bars[1]).toHaveAttribute('title', 'Medium');
     expect(bars[1]).toHaveTextContent('5.0 (Medium)');