Skip to content

#3845 mouse placement bug #3966

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

Open
Santiordon wants to merge 6 commits into
base: feature/gantt-chart-improvements
Choose a base branch
from
Open

#3845 mouse placement bug #3966

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7ad3307
Fix width
Santiordon Feb 5, 2026
6a2fa96
Merge Calendar
Santiordon Feb 6, 2026
6d3a65d
#3928 Mouse Placement Fix Implemented
Santiordon Feb 10, 2026
dac9f3c
#3845 Yarn revert
Santiordon Feb 10, 2026
ffbbc81
Prettier
Santiordon Feb 10, 2026
f2f4938
Delete unused Box
Santiordon Feb 10, 2026
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
54 changes: 31 additions & 23 deletions ...src/pages/GanttPage/GanttChart/GanttChartComponents/GanttTaskBar/GanttTaskBarEditView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Box, Chip, Typography } from '@mui/material';
import { useTheme } from '@mui/system';
import { CSSProperties, DragEvent, MouseEvent, useEffect, useState } from 'react';
import { CSSProperties, DragEvent, MouseEvent, useCallback, useEffect, useRef, useState } from 'react';
import useMeasure from 'react-use-measure';
import { addDaysToDate } from 'shared';
import { GanttChange, GanttTask, GANTT_CHART_CELL_SIZE } from '../../../../../utils/gantt.utils';
Expand Down Expand Up @@ -33,18 +33,20 @@ export const GanttTaskBarEditView = <T,>({
onAddTaskPressed
}: GanttTaskBarEditProps<T>) => {
const theme = useTheme();
const [startX, setStartX] = useState<number | null>(null);
const [showDropPoints, setShowDropPoints] = useState(false);
const [isResizing, setIsResizing] = useState(false);
const [width, setWidth] = useState(0); // current width of component, will change on resize
const [width, setWidth] = useState(0);
const [correctWidth, setCorrectWidth] = useState(0);
const [measureRef, bounds] = useMeasure();
const hasMeasuredRef = useRef(false);
const boxRef = useRef<HTMLDivElement | null>(null);
const widthPerDay = 7.2; //width per day to use for resizing calculations, kind of arbitrary,

const taskBarDisplayStyles: CSSProperties = {
gridColumnStart: getStartCol(task.start),
gridColumnEnd: getEndCol(task.end),
height: '2rem',
width: task.root ? 'unset' : width === 0 ? `unset` : `${width}px`,
width: task.root ? 'unset' : correctWidth > 0 ? `${correctWidth}px` : 'auto',
border: `1px solid ${isResizing ? theme.palette.text.primary : theme.palette.divider}`,
borderRadius: '0.25rem',
backgroundColor: task.styles ? task.styles.backgroundColor : theme.palette.background.paper,
Expand All @@ -67,48 +69,54 @@ export const GanttTaskBarEditView = <T,>({
right: '-10'
};

const getCorrectWidth = useCallback((rawWidth: number) => {
const newEventLengthInDays = roundToMultipleOf7(rawWidth / widthPerDay);
const displayWeeks = newEventLengthInDays / 7 + 1;
return displayWeeks * 40 + (displayWeeks - 1) * 10;
Copy link

Copilot AI Feb 21, 2026

Choose a reason for hiding this comment

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

The magic number 40 appears to be changed from 38, but this is inconsistent with GANTT_CHART_CELL_SIZE which is defined as '2.375rem' (approximately 38px at 16px base font size). This hardcoded value should either use the constant or have a clear explanation for the 2px discrepancy. Similarly, the gap value of 10 doesn't match GANTT_CHART_GAP_SIZE ('0.75rem' = 12px). Consider using the constants or deriving pixel values from them to maintain consistency and make the calculation more maintainable.

Copilot uses AI. Check for mistakes.
}, []);
Comment on lines +72 to +76
Copy link

Copilot AI Feb 21, 2026

Choose a reason for hiding this comment

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

The useCallback hook is missing widthPerDay in its dependency array. Since getCorrectWidth references widthPerDay (line 73), it should be included in the dependencies. However, since widthPerDay is a constant defined in the component, consider moving it outside the component as a module-level constant to avoid this issue and improve performance.

Copilot uses AI. Check for mistakes.

useEffect(() => {
if (bounds.width !== 0 && width === 0) {
if (!hasMeasuredRef.current && bounds.width > 0) {
setWidth(bounds.width);
setCorrectWidth(getCorrectWidth(bounds.width));
hasMeasuredRef.current = true;
}
}, [bounds, width]);
}, [bounds.width, getCorrectWidth]);

// used to make sure that any changes to the start and end dates are made in multiples of 7
const roundToMultipleOf7 = (num: number) => {
return Math.round(num / 7) * 7;
return Math.ceil(num / 7) * 7;
Copy link

Copilot AI Feb 21, 2026

Choose a reason for hiding this comment

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

Changing from Math.round to Math.ceil is a significant behavioral change. With Math.round, a value of 10.4 days would round down to 7 days, but with Math.ceil it will round up to 14 days. This means users will always get a larger-than-expected duration when resizing tasks. While this may fix the "one off bug", it could lead to unexpected behavior where tasks become longer than the user intended. Consider whether this is the desired behavior or if a more sophisticated rounding strategy is needed.

Suggested change
return Math.ceil(num / 7) * 7;
const weeks = Math.max(1, Math.round(num / 7));
return weeks * 7;

Copilot uses AI. Check for mistakes.
};
Comment on lines 87 to +89
Copy link

Copilot AI Feb 21, 2026

Choose a reason for hiding this comment

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

The roundToMultipleOf7 function is defined within the component body and referenced by getCorrectWidth. This causes the function to be redefined on every render. Consider moving roundToMultipleOf7 outside the component as a module-level utility function to improve performance and avoid unnecessary re-creations.

Copilot uses AI. Check for mistakes.

const getDistanceFromLeft = (clientX: number) => {
const rect = boxRef.current!.getBoundingClientRect();
return clientX - rect.left;
};
Comment on lines +91 to 94
Copy link

Copilot AI Feb 21, 2026

Choose a reason for hiding this comment

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

This function uses a non-null assertion operator on boxRef.current without checking if it's null. If handleMouseMove is called before handleMouseDown sets boxRef.current, or if the closest() call returns null, this will throw a runtime error. Consider adding a null check to prevent potential crashes.

Copilot uses AI. Check for mistakes.

const handleMouseDown = (e: MouseEvent<HTMLElement>) => {
setIsResizing(true);
setStartX(e.clientX);
boxRef.current = (e.currentTarget as HTMLElement).closest('[data-gantt-bar]') as HTMLDivElement;
Copy link

Copilot AI Feb 21, 2026

Choose a reason for hiding this comment

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

The closest() method can return null if no matching ancestor is found. This code uses type assertion to assume it's always an HTMLDivElement, which could lead to runtime errors. Consider adding a null check or using optional chaining to handle cases where the data-gantt-bar attribute might not be found in the element's ancestors.

Copilot uses AI. Check for mistakes.
};

const handleMouseMove = (e: MouseEvent<HTMLElement>) => {
if (isResizing) {
const currentX = e.clientX;
const deltaX = currentX - startX!;
setWidth(Math.max(100, width + deltaX));
setStartX(currentX);
}
if (!isResizing) return;

const newWidth = Math.max(100, getDistanceFromLeft(e.clientX));

setWidth(newWidth); // sync render
setCorrectWidth(getCorrectWidth(newWidth));
};

const handleMouseUp = () => {
if (isResizing) {
setIsResizing(false);
// Use change in width to calculate new length
const newEventLengthInDays = roundToMultipleOf7(width / widthPerDay);
// The gantt chart tasks are inclusive (their width includes the full width of their start and end date)
const displayWeeks = newEventLengthInDays / 7 + 1;
// We need these magic pixel numbers to dynamically calculate the correct width of the task to keep it in sync with the stored end date
const correctWidth = displayWeeks * 38 + (displayWeeks - 1) * 10;
const newEndDate = addDaysToDate(task.start, newEventLengthInDays);
setWidth(correctWidth);
createChange({
id: uuidv4(),
element: task.element,
type: 'change-end-date',
originalEnd: task.end,
newEnd: newEndDate
newEnd: addDaysToDate(task.start, newEventLengthInDays)
});
}
};
Expand Down Expand Up @@ -156,7 +164,7 @@ export const GanttTaskBarEditView = <T,>({
};
})}
>
<div ref={measureRef} style={taskBarDisplayStyles}>
<div data-gantt-bar ref={measureRef} style={taskBarDisplayStyles}>
<Box sx={webKitBoxContainerStyles()}>
<Box draggable={!task.root} onDrag={onDragStart} onDragEnd={onDragEnd} sx={webKitBoxStyles()}>
<Box sx={{ display: 'flex', flexDirection: 'row' }}>
Expand Down