Skip to content

feat(topology): Allow tooltip usage on pipeline task node badges #8208

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Oct 21, 2022
Merged

feat(topology): Allow tooltip usage on pipeline task node badges #8208

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
25 changes: 22 additions & 3 deletions packages/react-integration/demo-app-ts/src/components/demos/TopologyDemo/PipelineTasks.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React from 'react';
import { Flex, FlexItem, Radio, ToolbarItem } from '@patternfly/react-core';
import { Checkbox, Flex, FlexItem, Radio, ToolbarItem } from '@patternfly/react-core';
import {
TopologyView,
Visualization,
Expand All @@ -21,11 +21,22 @@ export const PipelineTasks: React.FC = () => {
const [showBadges, setShowBadges] = React.useState<boolean>(false);
const [showIcons, setShowIcons] = React.useState<boolean>(false);
const [selectedIds, setSelectedIds] = React.useState<string[]>();
const [badgeTooltips, setBadgeTooltips] = React.useState<boolean>(false);

const controller = useVisualizationController();

React.useEffect(() => {
const tasks = createStatusTasks('task', 4, undefined, false, false, showContext, showBadges, showIcons);
const tasks = createStatusTasks(
'task',
4,
undefined,
false,
false,
showContext,
showBadges,
showIcons,
badgeTooltips
);
setWhenStatus(tasks);
const finallyNodes = createFinallyTasks('finally', 2, tasks);
const finallyGroup = {
Expand All @@ -45,7 +56,7 @@ export const PipelineTasks: React.FC = () => {
nodes: [...tasks, ...finallyNodes, finallyGroup]
};
controller.fromModel(model, false);
}, [controller, showBadges, showContext, showIcons]);
}, [badgeTooltips, controller, showBadges, showContext, showIcons]);

useEventListener<SelectionEventListener>(SELECTION_EVENT, ids => {
setSelectedIds(ids);
Expand Down Expand Up @@ -126,6 +137,14 @@ export const PipelineTasks: React.FC = () => {
onChange={checked => checked && setShowContext(false)}
/>
</FlexItem>
<FlexItem className="pf-u-ml-2xl">
<Checkbox
id="badge-tips-checkbox"
label="Use tooltips for badges"
isChecked={badgeTooltips}
onChange={setBadgeTooltips}
/>
</FlexItem>
</Flex>
</ToolbarItem>
</>
Expand Down
13 changes: 9 additions & 4 deletions ...act-integration/demo-app-ts/src/components/demos/TopologyDemo/components/DemoTaskNode.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ type DemoTaskNodeProps = {
} & WithContextMenuProps &
WithSelectionProps;

const DEMO_TIP_TEXT =
'Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nullam id feugiat augue, nec fringilla turpis.';

const DemoTaskNode: React.FunctionComponent<DemoTaskNodeProps> = ({
element,
onContextMenu,
Expand Down Expand Up @@ -49,13 +52,14 @@ const DemoTaskNode: React.FunctionComponent<DemoTaskNodeProps> = ({
leftOffset={hasTaskIcon ? DEFAULT_WHEN_OFFSET + (element.getBounds().height - 4) * 0.75 : DEFAULT_WHEN_OFFSET}
/>
) : null;

// Set the badgePopoverParams, but if the node has badgeTooltips, this will be ignored
const badgePopoverParams: PopoverProps = {
headerContent: <div>Popover header</div>,
bodyContent: (
<div>Lorem ipsum dolor sit amet, consectetur adipiscing elit. Nullam id feugiat augue, nec fringilla turpis.</div>
),
headerContent: 'Popover header',
bodyContent: DEMO_TIP_TEXT,
footerContent: 'Popover footer'
};

return (
<Layer id={detailsLevel !== ScaleDetailsLevel.high && hover ? TOP_LAYER : DEFAULT_LAYER}>
<TaskNode
Expand All @@ -68,6 +72,7 @@ const DemoTaskNode: React.FunctionComponent<DemoTaskNodeProps> = ({
{...passedData}
{...rest}
badgePopoverParams={badgePopoverParams}
Copy link
Contributor

@nicolethoen nicolethoen Oct 20, 2022

Choose a reason for hiding this comment

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

I think it's worth communicating somehow, that when data.badgeTooltips is true, then the badgePopoverParams are not going to be used anymore. Right now, they are constant and appear to always be used, but that is not the case.

badgeTooltip={data.badgeTooltips && DEMO_TIP_TEXT}
>
{whenDecorator}
</TaskNode>
Expand Down
5 changes: 4 additions & 1 deletion ...es/react-integration/demo-app-ts/src/components/demos/TopologyDemo/utils/pipelineUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ export const createTask = (options: {
showContextMenu?: boolean;
showBadge?: boolean;
showIcon?: boolean;
badgeTooltips?: boolean;
}): PipelineNodeModel => {
const width = options.width || DEFAULT_TASK_WIDTH;
const height = options.height || DEFAULT_TASK_HEIGHT;
Expand Down Expand Up @@ -86,7 +87,8 @@ export const createStatusTasks = (
noLocation?: boolean,
showContextMenu?: boolean,
showBadge?: boolean,
showIcon?: boolean
showIcon?: boolean,
badgeTooltips?: boolean
): PipelineNodeModel[] =>
TASK_STATUSES.map((status, index) =>
createTask({
Expand All @@ -100,6 +102,7 @@ export const createStatusTasks = (
showContextMenu,
showBadge,
showIcon,
badgeTooltips,
width: DEFAULT_TASK_WIDTH + (showContextMenu ? 10 : 0) + (showBadge ? 40 : 0)
})
);
Expand Down
37 changes: 26 additions & 11 deletions packages/react-topology/src/pipelines/components/nodes/TaskNode.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,10 @@ export type TaskNodeProps = {
badgeTextColor?: string;
badgeBorderColor?: string;
badgeClassName?: string;
/** @deprecated Use badgePopoverParams instead */
badgePopoverProps?: string;
Copy link
Contributor

@nicolethoen nicolethoen Oct 20, 2022

Choose a reason for hiding this comment

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

Removing props is considered a breaking change. Rather than removing it, maybe just log a warning that this prop is not used for anything.

Copy link
Member Author

@jeff-phillips-18 jeff-phillips-18 Oct 20, 2022

Choose a reason for hiding this comment

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

This prop was never used. Anyone using it (only one adopter thus far and they are not) would not have gotten any use from doing so.

I would rather remove it to avoid confusion that would be cause by auto-complete showing this parameter.

Copy link
Contributor

@nicolethoen nicolethoen Oct 20, 2022

Choose a reason for hiding this comment

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

WDYT @tlabaj

Copy link
Contributor

@tlabaj tlabaj Oct 21, 2022

Choose a reason for hiding this comment

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

@jeff-phillips-18 can you at least bump the major version if remove the prop?

Copy link
Member Author

@jeff-phillips-18 jeff-phillips-18 Oct 21, 2022

Choose a reason for hiding this comment

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

I'll just put it back and deprecate it

badgePopoverParams?: PopoverProps;
badgeTooltip?: React.ReactNode; // Set to use a tooltip on the badge, takes precedence over the badgePopoverParams
badgePopoverParams?: PopoverProps; // Set to use a popover on the badge, ignored if the badgeTooltip parameter is set
taskIconClass?: string; // Icon to show for the task
taskIcon?: React.ReactNode;
taskIconTooltip?: React.ReactNode;
Expand Down Expand Up @@ -81,6 +83,8 @@ const TaskNode: React.FC<TaskNodeProps & { innerRef: React.Ref<SVGGElement> }> =
badgeTextColor,
badgeBorderColor,
badgeClassName = styles.topologyPipelinesPillBadge,
badgeTooltip,
badgePopoverProps,
badgePopoverParams,
taskIconClass,
taskIcon,
Expand Down Expand Up @@ -114,6 +118,11 @@ const TaskNode: React.FC<TaskNodeProps & { innerRef: React.Ref<SVGGElement> }> =
const [contextSize, contextRef] = useSize([onContextMenu, paddingX]);
const detailsLevel = useDetailsLevel();

if (badgePopoverProps) {
// eslint-disable-next-line no-console
console.warn('badgePopoverProps is deprecated. Use badgePopoverParams instead.');
}

const textWidth = textSize?.width ?? 0;
const textHeight = textSize?.height ?? 0;
useAnchor(
Expand Down Expand Up @@ -261,7 +270,7 @@ const TaskNode: React.FC<TaskNodeProps & { innerRef: React.Ref<SVGGElement> }> =
/>
);

const badgeComponent = badge && (
const badgeLabel = badge ? (
<LabelBadge
ref={badgeRef}
x={badgeStartX}
Expand All @@ -272,7 +281,20 @@ const TaskNode: React.FC<TaskNodeProps & { innerRef: React.Ref<SVGGElement> }> =
badgeTextColor={badgeTextColor}
badgeBorderColor={badgeBorderColor}
/>
);
) : null;

let badgeComponent: React.ReactNode;
if (badgeLabel && badgeTooltip) {
badgeComponent = <Tooltip content={badgeTooltip}>{badgeLabel}</Tooltip>;
} else if (badgeLabel && badgePopoverParams) {
badgeComponent = (
<g onClick={e => e.stopPropagation()}>
<Popover {...badgePopoverParams}>{badgeLabel}</Popover>
</g>
);
} else {
badgeComponent = badgeLabel;
}

const renderTask = () => {
if (showStatusState && !scaleNode && hideDetailsAtMedium && detailsLevel !== ScaleDetailsLevel.high) {
Expand Down Expand Up @@ -352,14 +374,7 @@ const TaskNode: React.FC<TaskNodeProps & { innerRef: React.Ref<SVGGElement> }> =
)}
{taskIconComponent &&
(taskIconTooltip ? <Tooltip content={taskIconTooltip}>{taskIconComponent}</Tooltip> : taskIconComponent)}
{badgeComponent &&
(badgePopoverParams ? (
<g onClick={e => e.stopPropagation()}>
<Popover {...badgePopoverParams}>{badgeComponent}</Popover>
</g>
) : (
badgeComponent
))}
{badgeComponent}
{actionIcon && (
<>
<line
Expand Down