Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
39 changes: 36 additions & 3 deletions src/components/Graphs/Graph.module.css
Original file line number Diff line number Diff line change
@@ -1,14 +1,43 @@
.graphContainer {
display: flex;
height: 600px;
overflow: hidden;
padding: 2px;
font-family: var(--sapFontFamily);
position: relative;
}

.graphColumn {
flex: 1;
display: flex;
flex-direction: column;
background-color: var(--sapTile_Background, #fff);
}

.topLegendContainer {
position: absolute;
top: 1rem;
right: 1rem;
display: flex;
align-items: center;

z-index: 2;
}

.filterIcon {
Copy link
Contributor

Choose a reason for hiding this comment

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

That's only for the container? What do you thing about giving it more precise name?

Suggested change
.filterIcon {
.filterIconContainer {

display: flex;
align-items: center;
cursor: pointer;
Copy link
Contributor

Choose a reason for hiding this comment

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

needed? It's not interactive

Suggested change
cursor: pointer;

padding: 4px;
border-radius: 8px;
transition: background-color 0.2s ease;
Copy link
Contributor

Choose a reason for hiding this comment

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

Needed? It's not interactive

Suggested change
transition: background-color 0.2s ease;

background-color: var(--sapTile_Background, #fff);
border: 1px solid var(--sapList_BorderColor, #ccc);
box-shadow: 0 2px 4px rgba(0, 0, 0, 0.1);
margin-left: 0.5rem;
}

.filterIcon:hover {
background-color: var(--sapButton_Hover_Background, #f0f0f0);
Comment on lines +39 to +40
Copy link
Contributor

Choose a reason for hiding this comment

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

Needed? It's not interactive

Suggested change
.filterIcon:hover {
background-color: var(--sapButton_Hover_Background, #f0f0f0);

}

.graphHeader {
Expand Down Expand Up @@ -45,15 +74,19 @@
color: var(--sapTextColor, #222);
}

/* Remove the default fieldset frame when used for grouping only */
.fieldsetReset {
border: 0;
margin: 0;
padding: 0;
min-inline-size: 0;
}

/* React Flow Controls dark mode */
.popoverButtonContainer {
display: flex;
flex-direction: column;
padding: 0.5rem;
}

:global([data-theme='dark'] .react-flow__controls) {
background-color: rgba(28, 28, 28, 0.9);
border: 1px solid rgba(255, 255, 255, 0.15);
Expand Down
86 changes: 53 additions & 33 deletions src/components/Graphs/Graph.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React, { useState, useCallback, useMemo } from 'react';
import { ReactFlow, Background, Controls, MarkerType, Node, Panel } from '@xyflow/react';
import { ReactFlow, Background, Controls, MarkerType, Node } from '@xyflow/react';
import { Button, Popover } from '@ui5/webcomponents-react';
import type { NodeProps } from '@xyflow/react';
import { RadioButton, FlexBox, FlexBoxAlignItems } from '@ui5/webcomponents-react';
import styles from './Graph.module.css';
import '@xyflow/react/dist/style.css';
import { NodeData, ColorBy } from './types';
Expand Down Expand Up @@ -32,7 +32,8 @@ const Graph: React.FC = () => {
const { t } = useTranslation();
const { openInAside } = useSplitter();
const { isDarkTheme } = useTheme();
const [colorBy, setColorBy] = useState<ColorBy>('provider');
const [colorBy, setColorBy] = useState<ColorBy>('source');
const [filterPopoverOpen, setFilterPopoverOpen] = useState(false);

const handleYamlClick = useCallback(
(item: ManagedResourceItem) => {
Expand Down Expand Up @@ -92,37 +93,56 @@ const Graph: React.FC = () => {
>
<Controls showInteractive={false} />
<Background />
<Panel position="top-left">
<FlexBox alignItems={FlexBoxAlignItems.Center} role="radiogroup">
<fieldset className={styles.fieldsetReset}>
<div className={styles.graphHeader}>
<span className={styles.colorizedTitle}>{t('Graphs.colorizedTitle')}</span>
<RadioButton
name="colorBy"
text={t('Graphs.colorsProviderConfig')}
checked={colorBy === 'provider'}
onChange={() => setColorBy('provider')}
/>
<RadioButton
name="colorBy"
text={t('Graphs.colorsProvider')}
checked={colorBy === 'source'}
onChange={() => setColorBy('source')}
/>
<RadioButton
name="colorBy"
text={t('Graphs.colorsFlux')}
checked={colorBy === 'flux'}
onChange={() => setColorBy('flux')}
/>
</div>
</fieldset>
</FlexBox>
</Panel>
<Panel position="top-right">
<Legend legendItems={legendItems} />
</Panel>
</ReactFlow>

<div className={styles.topLegendContainer}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason for replacing the Panel of ReactFlow with a custom div? The Panel already gives us many of the things for free, like positioning and floating on top.

Can't we just position the panel to top-right and put everything there?

<Legend legendItems={legendItems} />
<Popover
opener="filter-button"
open={filterPopoverOpen}
placement="Bottom"
onClose={() => setFilterPopoverOpen(false)}
>
<div className={styles.popoverButtonContainer}>
<Button
design={colorBy === 'source' ? 'Emphasized' : 'Default'}
onClick={() => {
setColorBy('source');
setFilterPopoverOpen(false);
}}
>
{t('Graphs.colorsProvider')}
</Button>
<Button
design={colorBy === 'provider' ? 'Emphasized' : 'Default'}
onClick={() => {
setColorBy('provider');
setFilterPopoverOpen(false);
}}
>
{t('Graphs.colorsProviderConfig')}
</Button>
<Button
design={colorBy === 'flux' ? 'Emphasized' : 'Default'}
onClick={() => {
setColorBy('flux');
setFilterPopoverOpen(false);
}}
>
{t('Graphs.colorsFlux')}
</Button>
</div>
</Popover>
<div className={styles.filterIcon}>
<Button
id="filter-button"
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 use useId() to generate an ID? IDs have to be unique on the entire HTML page. Adding the component twice would not work with a fixed ID. useId() will generate a unique ID.

design="Transparent"
icon="filter"
tooltip={t('Graphs.colorizedTitle')}
Copy link
Contributor

Choose a reason for hiding this comment

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

There’s a colon in the tooltip which we should remove.

Could also think of moving the text as a header of the popover. Another idea would be to call it something like "color by" because we're not really filtering or grouping (in a sense of rearranging content), are we? There are also other icons like sap-icon://legend or sap-icon://palette – just as an idea. Leaving it up to you ✌️

onClick={() => setFilterPopoverOpen(!filterPopoverOpen)}
/>
</div>
</div>
</div>
</div>
);
Expand Down
14 changes: 9 additions & 5 deletions src/components/Graphs/useGraph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { useMemo, useEffect, useState } from 'react';
import { useApiResource, useProvidersConfigResource } from '../../lib/api/useApiResource';
import { ManagedResourcesRequest } from '../../lib/api/types/crossplane/listManagedResources';
import { resourcesInterval } from '../../lib/shared/constants';
import { Node, Edge, Position, MarkerType } from '@xyflow/react';
import { Node, Edge, Position } from '@xyflow/react';
import dagre from 'dagre';
import { NodeData, ColorBy } from './types';
import { buildTreeData, generateColorMap } from './graphUtils';
Expand All @@ -24,14 +24,18 @@ function buildGraph(
treeData.forEach((n) => {
const colorKey: string =
colorBy === 'source' ? n.providerType : colorBy === 'flux' ? (n.fluxName ?? 'default') : n.providerConfigName;
const borderColor = colorMap[colorKey] || '#ccc';
// 8% opacity for background
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// 8% opacity for background
// some opacity for background

It's 8/255, so about 3% opacity.

const backgroundColor = `${borderColor}08`;

const node: Node<NodeData> = {
id: n.id,
type: 'custom',
data: { ...n },
style: {
border: `2px solid ${colorMap[colorKey] || '#ccc'}`,
border: `2px solid ${borderColor}`,
borderRadius: 8,
backgroundColor: 'var(--sapTile_Background, #fff)',
backgroundColor,
width: nodeWidth,
height: nodeHeight,
},
Expand All @@ -53,7 +57,7 @@ function buildGraph(
id: `e-${n.parentId}-${n.id}`,
source: n.parentId,
target: n.id,
markerEnd: { type: MarkerType.ArrowClosed },
style: { strokeWidth: 2, stroke: '#888' },
});
}
n.extraRefs?.forEach((refId) => {
Expand All @@ -63,7 +67,7 @@ function buildGraph(
id: `e-${refId}-${n.id}`,
source: refId,
target: n.id,
markerEnd: { type: MarkerType.ArrowClosed },
style: { strokeWidth: 2, stroke: '#888' },
});
}
});
Expand Down
Loading