feat: uxc tables improvements (#3118)

* feat: popover instead of tooltip for protected resources icon * feat: moved protected resource directly after name instead of separate column * fix: removed additional space after last table item * feat: make entire row navigateable in ClusterList * test: adjust tests to removed Ui5Links in list * feat: text in a table row can be selected without executing the onClick * fix: small mistake
kyma-project · Aug 14, 2024 · c1050c0 · c1050c0
1 parent b6afa48
commit c1050c0
Show file tree

Hide file tree

Showing 12 changed files with 131 additions and 77 deletions.
diff --git a/src/components/Clusters/views/ClusterList.js b/src/components/Clusters/views/ClusterList.js
@@ -2,8 +2,7 @@ import { useState } from 'react';
 import jsyaml from 'js-yaml';
 import { saveAs } from 'file-saver';
 import { useTranslation } from 'react-i18next';
-import { Button } from '@ui5/webcomponents-react';
-import { Link } from 'shared/components/Link/Link';
+import { Button, Text } from '@ui5/webcomponents-react';
 
 import { useClustersInfo } from 'state/utils/getClustersInfo';
 
@@ -52,6 +51,10 @@ function ClusterList() {
 
   const { clusters, currentCluster } = clustersInfo;
 
+  const handleClickResource = (_, selectedEntry) => {
+    navigate(`/cluster/${selectedEntry.contextName}`);
+  };
+
   const isClusterActive = entry => {
     return (
       entry?.kubeconfig?.['current-context'] === currentCluster?.contextName
@@ -97,13 +100,14 @@ function ClusterList() {
   ];
 
   const rowRenderer = entry => [
-    <Link
-      design={isClusterActive(entry) ? 'Emphasized' : 'Default'}
-      url={`/cluster/${entry.contextName}`}
-      resetLayout={false}
+    <Text
+      style={{
+        fontWeight: isClusterActive(entry) ? 'bold' : 'normal',
+        color: 'var(--sapLinkColor)',
+      }}
     >
       {entry.name}
-    </Link>,
+    </Text>,
     entry.currentContext.cluster.cluster.server,
     <ClusterStorageType clusterConfig={entry.config} />,
     entry.config?.description || EMPTY_TEXT_PLACEHOLDER,
@@ -235,6 +239,8 @@ function ClusterList() {
                 showSearchSuggestion: false,
                 noSearchResultMessage: t('clusters.list.no-clusters-found'),
               }}
+              customRowClick={handleClickResource}
+              hasDetailsView
             />
             {createPortal(
               <DeleteMessageBox

diff --git a/src/components/Extensibility/helpers/index.js b/src/components/Extensibility/helpers/index.js
@@ -180,7 +180,6 @@ export const getResourceDescAndUrl = descID => {
 
   if (typeof trans === 'string') {
     const links = extractLinks(trans);
-    console.log(links, trans);
 
     if (links?.length >= 1) {
       const matchedLink = links[0];

diff --git a/src/shared/components/GenericList/GenericList.js b/src/shared/components/GenericList/GenericList.js
@@ -386,8 +386,12 @@ export const GenericList = ({
         className={`ui5-generic-list ${
           hasDetailsView && filteredEntries.length ? 'cursor-pointer' : ''
         }`}
+        onMouseDown={() => {
+          window.getSelection().removeAllRanges();
+        }}
         onRowClick={e => {
-          if (!hasDetailsView) return;
+          const selection = window.getSelection().toString();
+          if (!hasDetailsView || selection.length > 0) return;
           handleActionIfFormOpen(
             isResourceEdited,
             setIsResourceEdited,

diff --git a/src/shared/components/GenericList/GenericList.scss b/src/shared/components/GenericList/GenericList.scss
@@ -1,4 +1,6 @@
 .ui5-generic-list {
+  display: block;
+
   .body-fallback {
     font-size: 18px;
     padding: 20px;

diff --git a/src/shared/components/GenericList/Pagination/Pagination.scss b/src/shared/components/GenericList/Pagination/Pagination.scss
@@ -1,4 +1,5 @@
 .pagination {
+  border-top: 1px solid var(--sapList_BorderColor);
   display: flex;
   justify-content: space-between;
   align-items: center;
@@ -30,7 +31,7 @@
       font-size: var(--sapFontSize);
 
       i::before {
-        color: var(--sapBrandColor);
+        color: var(--sapLinkColor);
         font-size: 80%;
       }
 
@@ -39,7 +40,7 @@
       }
 
       &.interactable {
-        color: var(--sapBrandColor);
+        color: var(--sapLinkColor);
         cursor: pointer;
       }
     }

diff --git a/src/shared/components/ResourceDetails/ResourceDetails.js b/src/shared/components/ResourceDetails/ResourceDetails.js
@@ -191,7 +191,11 @@ function Resource({
 
   const pluralizedResourceKind = pluralize(prettifiedResourceKind);
   useWindowTitle(windowTitle || pluralizedResourceKind);
-  const { isProtected, protectedResourceWarning } = useProtectedResources();
+  const {
+    isProtected,
+    protectedResourceWarning,
+    protectedResourcePopover,
+  } = useProtectedResources();
 
   const [DeleteMessageBox, handleResourceDelete] = useDeleteResource({
     resourceTitle,
@@ -398,6 +402,7 @@ function Resource({
 
   return (
     <ResourceDetailContext.Provider value={true}>
+      {protectedResourcePopover()}
       <DynamicPageComponent
         className={className}
         headerContent={headerContent}

diff --git a/src/shared/components/ResourcesList/ResourcesList.js b/src/shared/components/ResourcesList/ResourcesList.js
@@ -221,7 +221,11 @@ export function ResourceListRenderer({
     resourceType,
   });
   const { t } = useTranslation();
-  const { isProtected, protectedResourceWarning } = useProtectedResources();
+  const {
+    isProtected,
+    protectedResourceWarning,
+    protectedResourcePopover,
+  } = useProtectedResources();
   const [layoutState, setLayoutColumn] = useRecoilState(columnLayoutState);
   const setIsFormOpen = useSetRecoilState(isFormOpenState);
 
@@ -387,16 +391,22 @@ export function ResourceListRenderer({
   const nameColIndex = customColumns.findIndex(col => col.id === 'name');
 
   const headerRenderer = () => {
-    const rowColumns = customColumns?.map(col => col?.header || null);
-    rowColumns.splice(nameColIndex + 1, 0, '');
-    return rowColumns;
+    return customColumns?.map(col => col?.header || null);
   };
 
   const rowRenderer = entry => {
-    const rowColumns = customColumns?.map(col =>
-      col?.value ? col.value(entry) : null,
-    );
-    rowColumns.splice(nameColIndex + 1, 0, protectedResourceWarning(entry));
+    const rowColumns = customColumns?.map((col, index) => {
+      if (col?.value && index === nameColIndex) {
+        return (
+          <div style={{ display: 'flex', alignItems: 'center', gap: '4px' }}>
+            {col.value(entry)}
+            {protectedResourceWarning(entry)}
+          </div>
+        );
+      }
+      return col?.value ? col.value(entry) : null;
+    });
+
     return rowColumns;
   };
 
@@ -491,46 +501,49 @@ export function ResourceListRenderer({
         document.body,
       )}
       {!(error && error.status === 'Definition not found') && (
-        <GenericList
-          displayArrow={displayArrow ?? true}
-          disableHiding={disableHiding ?? false}
-          hasDetailsView={hasDetailsView}
-          customUrl={customUrl}
-          resourceType={resourceType}
-          customColumnLayout={customColumnLayout}
-          columnLayout={columnLayout}
-          enableColumnLayout={enableColumnLayout}
-          disableMargin={disableMargin}
-          title={showTitle ? title || prettifiedResourceName : null}
-          actions={actions}
-          entries={resources || []}
-          headerRenderer={headerRenderer}
-          rowRenderer={rowRenderer}
-          serverDataError={error}
-          serverDataLoading={loading}
-          pagination={{ autoHide: true, ...pagination }}
-          extraHeaderContent={extraHeaderContent}
-          testid={testid}
-          sortBy={sortBy}
-          searchSettings={{
-            ...searchSettings,
-            textSearchProperties: textSearchProperties(),
-          }}
-          emptyListProps={
-            simpleEmptyListMessage
-              ? null
-              : {
-                  titleText: `${t('common.labels.no')} ${processTitle(
-                    prettifyNamePlural(resourceTitle, resourceType),
-                  )}`,
-                  onClick: handleShowCreate,
-                  showButton: !disableCreate && namespace !== '-all-',
-                  ...emptyListProps,
-                }
-          }
-          handleRedirect={handleRedirect}
-          nameColIndex={nameColIndex}
-        />
+        <>
+          {protectedResourcePopover()}
+          <GenericList
+            displayArrow={displayArrow ?? true}
+            disableHiding={disableHiding ?? false}
+            hasDetailsView={hasDetailsView}
+            customUrl={customUrl}
+            resourceType={resourceType}
+            customColumnLayout={customColumnLayout}
+            columnLayout={columnLayout}
+            enableColumnLayout={enableColumnLayout}
+            disableMargin={disableMargin}
+            title={showTitle ? title || prettifiedResourceName : null}
+            actions={actions}
+            entries={resources || []}
+            headerRenderer={headerRenderer}
+            rowRenderer={rowRenderer}
+            serverDataError={error}
+            serverDataLoading={loading}
+            pagination={{ autoHide: true, ...pagination }}
+            extraHeaderContent={extraHeaderContent}
+            testid={testid}
+            sortBy={sortBy}
+            searchSettings={{
+              ...searchSettings,
+              textSearchProperties: textSearchProperties(),
+            }}
+            emptyListProps={
+              simpleEmptyListMessage
+                ? null
+                : {
+                    titleText: `${t('common.labels.no')} ${processTitle(
+                      prettifyNamePlural(resourceTitle, resourceType),
+                    )}`,
+                    onClick: handleShowCreate,
+                    showButton: !disableCreate && namespace !== '-all-',
+                    ...emptyListProps,
+                  }
+            }
+            handleRedirect={handleRedirect}
+            nameColIndex={nameColIndex}
+          />
+        </>
       )}
       {!isCompact && createPortal(<YamlUploadDialog />, document.body)}
     </>

diff --git a/src/shared/helpers/linkExtractor.tsx b/src/shared/helpers/linkExtractor.tsx
@@ -84,7 +84,6 @@ const getI18nVarLink = (text: string) => {
   if (matches?.length) {
     links = matches.map(link => {
       const { links: mdLinks } = getMarkdownLinks(link);
-
       if (mdLinks?.length) {
         const { url, urlText } = mdLinks[0];
         text = text.replace(link, coveredLinkSign);

diff --git a/src/shared/hooks/useProtectedResources.js b/src/shared/hooks/useProtectedResources.js
@@ -1,14 +1,24 @@
-import { Icon, ObjectStatus } from '@ui5/webcomponents-react';
+import {
+  Button,
+  Icon,
+  ObjectStatus,
+  Popover,
+  Text,
+} from '@ui5/webcomponents-react';
 import { useFeature } from 'hooks/useFeature';
 import * as jp from 'jsonpath';
+import { useRef, useState } from 'react';
+import { createPortal } from 'react-dom';
 import { useTranslation } from 'react-i18next';
 import { useRecoilValue } from 'recoil';
 
-import { Tooltip } from 'shared/components/Tooltip/Tooltip';
 import { disableResourceProtectionState } from 'state/preferences/disableResourceProtectionAtom';
 
 export function useProtectedResources() {
   const { t } = useTranslation();
+  const popoverRef = useRef(null);
+  const [popoverMessage, setPopoverMessage] = useState('');
+
   const protectedResourcesFeature = useFeature('PROTECTED_RESOURCES');
   const disableResourceProtection = useRecoilValue(
     disableResourceProtectionState,
@@ -52,13 +62,14 @@ export function useProtectedResources() {
       .join('\n');
 
     return (
-      <Tooltip
-        className="protected-resource-warning"
-        content={message}
-        delay={0}
+      <Button
+        design="Transparent"
+        onClick={e => {
+          setPopoverMessage(message);
+          popoverRef?.current?.showAt(e?.target);
+        }}
       >
-        {!withText && <Icon design="Critical" name="locked" />}
-        {withText && (
+        {withText ? (
           <ObjectStatus
             icon={<Icon name="locked" />}
             showDefaultIcon
@@ -67,8 +78,26 @@ export function useProtectedResources() {
           >
             {t('common.protected-resource')}
           </ObjectStatus>
+        ) : (
+          <Icon
+            design="Critical"
+            name="locked"
+            style={{ marginTop: '0.125rem' }}
+          />
         )}
-      </Tooltip>
+      </Button>
+    );
+  };
+
+  const protectedResourcePopover = () => {
+    if (disableResourceProtection) {
+      return <></>;
+    }
+    return createPortal(
+      <Popover placementType="Right" ref={popoverRef}>
+        <Text className="description">{popoverMessage}</Text>
+      </Popover>,
+      document.body,
     );
   };
 
@@ -77,5 +106,6 @@ export function useProtectedResources() {
     getEntryProtection,
     isProtected,
     protectedResourceWarning,
+    protectedResourcePopover,
   };
 }
diff --git a/src/styles/index.scss b/src/styles/index.scss
@@ -8,6 +8,7 @@
   --card-border: var(--_ui5-v1-24-0_card_border);
   --card-border-radius: var(--_ui5-v1-24-0_card_border-radius);
   --card-border: var(--_ui5-v1-24-0_card_border);
+  --ui5-v1-24-0_table_bottom_border: none !important;
 }
 
 html {

diff --git a/tests/integration/support/commands.js b/tests/integration/support/commands.js
@@ -165,7 +165,6 @@ Cypress.Commands.add(
       confirmationEnabled = true,
       deletedVisible = true,
       clearSearch = true,
-      isUI5Link = true,
       checkIfResourceIsRemoved = true,
       selectSearchResult = false,
     } = options;
@@ -183,11 +182,7 @@ Cypress.Commands.add(
         .click();
     }
 
-    if (isUI5Link) {
-      cy.checkItemOnGenericListLink(resourceName);
-    } else {
-      cy.contains('ui5-link', resourceName).should('be.visible');
-    }
+    cy.checkItemOnGenericListLink(resourceName);
 
     cy.get('ui5-button[data-testid="delete"]').click();
 

diff --git a/tests/integration/tests/namespace/test-reduced-permissions.spec.js b/tests/integration/tests/namespace/test-reduced-permissions.spec.js
@@ -246,7 +246,6 @@ context('Test reduced permissions', () => {
       confirmationEnabled: true,
       deletedVisible: false,
       clearSearch: false,
-      isUI5Link: false,
     });
 
     cy.contains(/No clusters found/).should('exist');