Skip to content
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.

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

Feature/slicing pipeline add notification #2003

Merged
merged 10 commits into from
Aug 16, 2024
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
// sliced-pipeline-action-bar.test.js
import React from 'react';
import { render, screen } from '@testing-library/react';
import { render } from '@testing-library/react';
import { SlicedPipelineActionBar } from './sliced-pipeline-action-bar';

describe('SlicedPipelineActionBar', () => {
Expand All @@ -10,7 +9,7 @@ describe('SlicedPipelineActionBar', () => {
firstChild: { getBoundingClientRect: () => ({ width: 100 }) },
},
};
render(
const { container } = render(
<SlicedPipelineActionBar
chartSize={{ outerWidth: 800 }}
slicedPipeline={[]}
Expand All @@ -20,10 +19,15 @@ describe('SlicedPipelineActionBar', () => {
ref={ref}
/>
);
const notification = screen.getByText(
/Hold Shift \+ Click on another node to slice pipeline/i

// Check for the presence of the notification class
const notificationElement = container.querySelector(
'.sliced-pipeline-action-bar--info'
);
expect(notificationElement).toBeInTheDocument();
expect(notificationElement.textContent).toContain(
'Hold Shift + Click on another node to slice pipeline'
);
expect(notification).toBeInTheDocument();
});

it('displays "Slice" button when slicedPipeline is not empty', () => {
Expand All @@ -32,7 +36,7 @@ describe('SlicedPipelineActionBar', () => {
firstChild: { getBoundingClientRect: () => ({ width: 100 }) },
},
};
render(
const { container } = render(
<SlicedPipelineActionBar
chartSize={{ outerWidth: 800 }}
slicedPipeline={[1, 2, 3]}
Expand All @@ -42,7 +46,14 @@ describe('SlicedPipelineActionBar', () => {
ref={ref}
/>
);
const sliceButton = screen.getByRole('button', { name: /slice/i });

const ctaElement = container.querySelector(
'.sliced-pipeline-action-bar--cta'
);
expect(ctaElement).toBeInTheDocument();

const sliceButton = ctaElement.querySelector('button');
expect(sliceButton).toBeInTheDocument();
expect(sliceButton.textContent).toBe('Slice');
});
});
74 changes: 33 additions & 41 deletions src/components/flowchart/flowchart.js
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,7 @@ export class FlowChart extends Component {
if (!this.props.clickedNode && prevProps.clickedNode) {
if (!this.props.slicedPipeline.length) {
// Ensure this only runs when not in a slicing operation
this.updateSlicedPipelineState(null, null, []);
this.setState({ showSlicingNotification: false }); // Hide notification when clicking away
this.setState({ showSlicingNotification: false });
}
}
}
Expand Down Expand Up @@ -537,8 +536,8 @@ export class FlowChart extends Component {
// if both "from" and "to" are defined
// then on a single node click, it should reset the sliced pipeline state
if (from !== null && to !== null) {
this.updateSlicedPipelineState(null, null, []);
this.setState({ showSlicingNotification: false }); // Hide notification
this.updateSlicedPipelineState(id, null, []);
this.setState({ showSlicingNotification: true }); // Show notification
} else {
// Else, set the first node as the 'from' node based on current state
// we need this so that if user hold shift and click on a second node,
Expand All @@ -552,66 +551,59 @@ export class FlowChart extends Component {
};

/**
* Determines the correct order of nodes based on their positions,
* but keeps the user's original selection as the visual 'from' node.
* @param {string} userSelectedFromNodeId - User's initially selected 'from' node ID.
* @param {string} toNodeId - The current 'to' node ID.
* @returns {Object} - Object containing userVisualFromNodeId and adjustedFromNodeId, newToNodeId
* Determines the correct order of nodes based on their positions.
* @param {string} fromNodeId - 'From' node ID.
* @param {string} toNodeId - 'To' node ID.
* @returns {Object} - Object containing updatedFromNodeId and updatedToNodeId.
*/
determineNodesOrder = (userSelectedFromNodeId, toNodeId) => {
determineNodesOrder = (fromNodeId, toNodeId) => {
// Get bounding client rects of nodes
const fromNodeElement = document.querySelector(
`[data-id="${userSelectedFromNodeId}"]`
);
const fromNodeElement = document.querySelector(`[data-id="${fromNodeId}"]`);
SajidAlamQB marked this conversation as resolved.
Show resolved Hide resolved
const toNodeElement = document.querySelector(`[data-id="${toNodeId}"]`);

if (!fromNodeElement || !toNodeElement) {
return {
userVisualFromNodeId: null,
adjustedFromNodeId: null,
newToNodeId: null,
}; // If any element is missing, return nulls
updatedFromNodeId: null,
updatedToNodeId: null,
};
}

const fromNodeRect = fromNodeElement.getBoundingClientRect();
const toNodeRect = toNodeElement.getBoundingClientRect();

// Reorder the nodes based on their Y-coordinate
const [adjustedFromNodeId, newToNodeId] =
fromNodeRect.y < toNodeRect.y
? [userSelectedFromNodeId, toNodeId] // Keep order
: [toNodeId, userSelectedFromNodeId]; // Swap if needed

return {
userVisualFromNodeId: userSelectedFromNodeId, // Keep user's selection visually as 'from'
adjustedFromNodeId,
newToNodeId,
};
// Reorder based on their Y-coordinate
return fromNodeRect.y < toNodeRect.y
? { updatedFromNodeId: fromNodeId, updatedToNodeId: toNodeId }
: { updatedFromNodeId: toNodeId, updatedToNodeId: fromNodeId };
};

handleShiftClick = (node) => {
// Close meta data panel
this.props.onLoadNodeData(null);
const { from: userSelectedFromNodeId, range } =
this.state.slicedPipelineState;
const { from: fromNodeIdState, range } = this.state.slicedPipelineState;

const fromNodeId = userSelectedFromNodeId || node.id; // Keep existing 'from' node or set the current one if not set
// Track user's selection directly in the state
const fromNodeId = fromNodeIdState || node.id;
const toNodeId = node.id;

const { userVisualFromNodeId, adjustedFromNodeId, newToNodeId } =
this.determineNodesOrder(fromNodeId, toNodeId);
// Update the state to reflect the user's selections
this.updateSlicedPipelineState(fromNodeId, toNodeId, range);

if (!adjustedFromNodeId || !newToNodeId) {
return; // Exit if node order couldn't be determined
}
// Reorder nodes internally for slicing
const { updatedFromNodeId, updatedToNodeId } = this.determineNodesOrder(
fromNodeId,
toNodeId
);

// Visually keep the 'from' node as the user's selection, but adjust internally based on Y-coordinate
this.updateSlicedPipelineState(userVisualFromNodeId, newToNodeId, range);
// Slice the pipeline based on the determined order, or fallback to original order if undefined
if (updatedFromNodeId && updatedToNodeId) {
this.props.onSlicePipeline(updatedFromNodeId, updatedToNodeId);
} else {
this.props.onSlicePipeline(fromNodeId, toNodeId);
}

this.props.onSlicePipeline(adjustedFromNodeId, newToNodeId);
this.props.onApplySlice(false);

this.setState({ showSlicingNotification: false }); // Hide notification after selecting the second node
this.setState({ showSlicingNotification: false }); // Hide notification
};

/**
Expand Down
Loading