From 2e6292ce21c0a753fa0f51b48c75431fb92da876 Mon Sep 17 00:00:00 2001 From: chriskari Date: Fri, 9 Aug 2024 10:04:15 +0200 Subject: [PATCH 1/7] feat: popover instead of tooltip for protected resources icon --- .../ResourceDetails/ResourceDetails.js | 7 +- .../components/ResourcesList/ResourcesList.js | 89 ++++++++++--------- src/shared/hooks/useProtectedResources.js | 48 ++++++++-- 3 files changed, 93 insertions(+), 51 deletions(-) diff --git a/src/shared/components/ResourceDetails/ResourceDetails.js b/src/shared/components/ResourceDetails/ResourceDetails.js index 7960b61f21..17ad20dbd1 100644 --- a/src/shared/components/ResourceDetails/ResourceDetails.js +++ b/src/shared/components/ResourceDetails/ResourceDetails.js @@ -185,7 +185,11 @@ function Resource({ const pluralizedResourceKind = pluralize(prettifiedResourceKind); useWindowTitle(windowTitle || pluralizedResourceKind); - const { isProtected, protectedResourceWarning } = useProtectedResources(); + const { + isProtected, + protectedResourceWarning, + protectedResourcePopover, + } = useProtectedResources(); const [DeleteMessageBox, handleResourceDelete] = useDeleteResource({ resourceTitle, @@ -392,6 +396,7 @@ function Resource({ return ( + {protectedResourcePopover()} + <> + {protectedResourcePopover()} + + )} {!isCompact && createPortal(, document.body)} diff --git a/src/shared/hooks/useProtectedResources.js b/src/shared/hooks/useProtectedResources.js index 02437602b4..54d18d292a 100644 --- 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 ( - { + setPopoverMessage(message); + popoverRef?.current?.showAt(e?.target); + }} > - {!withText && } - {withText && ( + {withText ? ( } showDefaultIcon @@ -67,8 +78,26 @@ export function useProtectedResources() { > {t('common.protected-resource')} + ) : ( + )} - + + ); + }; + + const protectedResourcePopover = () => { + if (disableResourceProtection) { + return <>; + } + return createPortal( + + {popoverMessage} + , + document.body, ); }; @@ -77,5 +106,6 @@ export function useProtectedResources() { getEntryProtection, isProtected, protectedResourceWarning, + protectedResourcePopover, }; } From b923d4d118544d542d68dfe5b4a3fc65680a4bad Mon Sep 17 00:00:00 2001 From: chriskari Date: Fri, 9 Aug 2024 10:59:44 +0200 Subject: [PATCH 2/7] feat: moved protected resource directly after name instead of separate column --- .../components/ResourcesList/ResourcesList.js | 20 ++++++++++++------- src/shared/helpers/linkExtractor.tsx | 1 - 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/shared/components/ResourcesList/ResourcesList.js b/src/shared/components/ResourcesList/ResourcesList.js index 672a5f6f23..655e17d19e 100644 --- a/src/shared/components/ResourcesList/ResourcesList.js +++ b/src/shared/components/ResourcesList/ResourcesList.js @@ -391,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 ( +
+ {col.value(entry)} + {protectedResourceWarning(entry)} +
+ ); + } + return col?.value ? col.value(entry) : null; + }); + return rowColumns; }; diff --git a/src/shared/helpers/linkExtractor.tsx b/src/shared/helpers/linkExtractor.tsx index 10ccc9103c..26bff93465 100644 --- 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); - console.log(mdLinks); if (mdLinks?.length) { const { url, urlText } = mdLinks[0]; text = text.replace(link, coveredLinkSign); From b8b41f4de1140eb5ce727dd2fb88e4211b45f456 Mon Sep 17 00:00:00 2001 From: chriskari Date: Fri, 9 Aug 2024 12:47:50 +0200 Subject: [PATCH 3/7] fix: removed additional space after last table item --- src/shared/components/GenericList/GenericList.scss | 2 ++ src/shared/components/GenericList/Pagination/Pagination.scss | 5 +++-- src/styles/index.scss | 1 + 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/shared/components/GenericList/GenericList.scss b/src/shared/components/GenericList/GenericList.scss index ed5f944f6d..6b16d79b26 100644 --- 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 index ef616923a6..9295bdf2f7 100644 --- 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/styles/index.scss b/src/styles/index.scss index a13b1763c3..a102e4a684 100644 --- 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 { From 2b792d1fc72a78a538de25778dd058d6ea5449a1 Mon Sep 17 00:00:00 2001 From: chriskari Date: Mon, 12 Aug 2024 10:06:30 +0200 Subject: [PATCH 4/7] feat: make entire row navigateable in ClusterList --- src/components/Clusters/views/ClusterList.js | 22 +++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/src/components/Clusters/views/ClusterList.js b/src/components/Clusters/views/ClusterList.js index 080dd58e71..5c9d097d90 100644 --- 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 => [ - - {entry.name} - , + {entry.contextName} + , entry.currentContext.cluster.cluster.server, , 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( Date: Mon, 12 Aug 2024 10:48:11 +0200 Subject: [PATCH 5/7] test: adjust tests to removed Ui5Links in list --- tests/integration/support/commands.js | 7 +------ .../tests/namespace/test-reduced-permissions.spec.js | 1 - 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/tests/integration/support/commands.js b/tests/integration/support/commands.js index 9148442a90..d670fdc838 100644 --- 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 index c0a613604a..c8f66062f5 100644 --- 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'); From 003548f188c82e7d24474cb0f369af98568da61d Mon Sep 17 00:00:00 2001 From: chriskari Date: Tue, 13 Aug 2024 10:49:34 +0200 Subject: [PATCH 6/7] feat: text in a table row can be selected without executing the onClick --- src/components/Extensibility/helpers/index.js | 1 - src/shared/components/GenericList/GenericList.js | 6 +++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/components/Extensibility/helpers/index.js b/src/components/Extensibility/helpers/index.js index 8f9370eb13..0daf789906 100644 --- 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 index 862b2f9e5d..d93bcc5c20 100644 --- 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, From e9d8e52857c7d32062723bc33c497c99e3d07382 Mon Sep 17 00:00:00 2001 From: chriskari Date: Tue, 13 Aug 2024 16:01:42 +0200 Subject: [PATCH 7/7] fix: small mistake --- src/components/Clusters/views/ClusterList.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/Clusters/views/ClusterList.js b/src/components/Clusters/views/ClusterList.js index 5c9d097d90..a721fc8a72 100644 --- a/src/components/Clusters/views/ClusterList.js +++ b/src/components/Clusters/views/ClusterList.js @@ -106,7 +106,7 @@ function ClusterList() { color: 'var(--sapLinkColor)', }} > - {entry.contextName} + {entry.name} , entry.currentContext.cluster.cluster.server, ,