Skip to content

fix: responsive canvas dimensions for static and infinite scroll pages #27

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
anth0nycodes wants to merge 13 commits into
base: main
Choose a base branch
from
Open

fix: responsive canvas dimensions for static and infinite scroll pages #27

Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
7e25976
fix: canvas dimensions
anth0nycodes Mar 8, 2026
5cbf15e
wip
anth0nycodes Mar 9, 2026
e181bdf
add comments
anth0nycodes Mar 9, 2026
e2f41a0
Merge branch 'main' into anthony/canvas-dimensions-fix
anth0nycodes Mar 9, 2026
94dc27d
update variable names to be simpler and more comprehensive
anth0nycodes Mar 9, 2026
6e54ba0
revert
anth0nycodes Mar 9, 2026
1851d53
Merge branch 'main' into anthony/canvas-dimensions-fix
anth0nycodes Mar 11, 2026
10417fe
fix
anth0nycodes Mar 11, 2026
4d12f5d
add alert cap
anth0nycodes Mar 11, 2026
e11a97e
updated cap and added another condition
anth0nycodes Mar 11, 2026
5548349
remove redundant check
anth0nycodes Mar 11, 2026
f2293ba
Merge branch 'main' into anthony/canvas-dimensions-fix
anth0nycodes Mar 12, 2026
212ab56
Merge remote-tracking branch 'origin' into anthony/canvas-dimensions-fix
anth0nycodes Mar 16, 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
81 changes: 74 additions & 7 deletions src/components/canvas.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,23 +15,73 @@ import { usePencilPopover } from "@/context/pencil-popover/use-pencil-popover";
import { useTextPopover } from "@/context/text-popover/use-text-popover";
import { getCanvasCoordinates, getOS } from "@/lib/helpers";

function setupCanvas(fc: FabricCanvas) {
// Get the full document dimensions
const EXPANSION_INCREMENT_IN_PIXELS = 500;
const CANVAS_MAX_HEIGHT_IN_PIXELS = 8000; // Set a maximum height to prevent excessive canvas size
Comment thread
anth0nycodes marked this conversation as resolved.

function updateCanvasWidth(fc: FabricCanvas) {
const contentWidth = Math.max(
document.documentElement.clientWidth,
document.body.clientWidth
);

fc.setDimensions({ width: contentWidth });
}

function updateCanvasHeight(fc: FabricCanvas) {
const contentHeight = Math.max(
document.documentElement.clientHeight,
document.body.clientHeight
);

fc.setDimensions({
width: contentWidth,
height: contentHeight,
});
}

function updateDynamicCanvasHeight(fc: FabricCanvas) {
const { scrollY: scrollYAmount, innerHeight: viewportHeight } = window;
const currentCanvasHeight = fc.getHeight();
let newCanvasHeight = currentCanvasHeight;
const visibleBottomY = viewportHeight + scrollYAmount;

fc.setDimensions({ height: 0 });

const contentScrollHeight = Math.max(
document.documentElement.scrollHeight,
document.body.scrollHeight
);

fc.setDimensions({ height: currentCanvasHeight });

if (
visibleBottomY >= currentCanvasHeight &&
currentCanvasHeight > CANVAS_MAX_HEIGHT_IN_PIXELS
) {
Comment thread
anth0nycodes marked this conversation as resolved.
alert(
"This page is too tall for Tracemark to support. You can still draw on the visible canvas area, but it won't expand further."
);
fc.setDimensions({
height: CANVAS_MAX_HEIGHT_IN_PIXELS,
});
// TODO: remove alert and replace with better user-facing error handling
return;
Comment on lines +56 to +67

Copilot AI Mar 12, 2026

Copy link

Choose a reason for hiding this comment

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

Using alert(...) here is blocking and can be triggered repeatedly as the user scrolls near the bottom (especially if the height briefly exceeds the cap). Consider switching to non-blocking in-app UI and/or guarding so the message is only shown once per page/session.

Copilot uses AI. Check for mistakes.
}

while (
newCanvasHeight < visibleBottomY &&
visibleBottomY <= contentScrollHeight &&
newCanvasHeight < CANVAS_MAX_HEIGHT_IN_PIXELS
) {
newCanvasHeight += EXPANSION_INCREMENT_IN_PIXELS;
}

if (newCanvasHeight > currentCanvasHeight) {
fc.setDimensions({
height: Math.max(newCanvasHeight, viewportHeight),
});
Comment on lines +70 to +81

Copilot AI Mar 12, 2026

Copy link

Choose a reason for hiding this comment

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

newCanvasHeight can exceed CANVAS_MAX_HEIGHT_IN_PIXELS because the loop condition checks the value before adding EXPANSION_INCREMENT_IN_PIXELS. This allows setting the Fabric canvas height above the intended cap (e.g. 7800 -> 8300). Clamp the final height to the max (or change the loop to prevent overshoot) and consider adjusting the alert/guard to trigger when an expansion would cross the cap (not only when currentCanvasHeight > CANVAS_MAX_HEIGHT_IN_PIXELS).

Copilot uses AI. Check for mistakes.
}
}

interface CanvasProps {
currentTool: ToolbarStates;
setCurrentTool: (currentTool: ToolbarStates) => void;
Expand Down Expand Up @@ -61,10 +111,26 @@ export function Canvas({ currentTool, setCurrentTool }: CanvasProps) {

fcRef.current = fc;

const initCanvasDimensions = () => setupCanvas(fc);
initCanvasDimensions();
const handleResize = () => {
updateCanvasWidth(fc);
updateCanvasHeight(fc);
Comment on lines +115 to +116

Copilot AI Mar 11, 2026

Copy link

Choose a reason for hiding this comment

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

handleResize calls updateCanvasHeight, which sets the canvas height to clientHeight (viewport). Because ResizeObserver is observing documentElement/body, it can fire on content growth (e.g., infinite scroll) and shrink an already-expanded canvas back down to the viewport, potentially hiding existing drawings. Consider making resize only adjust width, or set height to Math.max(fc.getHeight(), viewportHeight/current desired height), or invoke the same dynamic-height logic instead of forcing clientHeight.

Suggested change
updateCanvasWidth(fc);
updateCanvasHeight(fc);
// On resize, only adjust the canvas width. Height is managed dynamically
// via updateDynamicCanvasHeight to avoid shrinking an already-expanded canvas.
updateCanvasWidth(fc);

Copilot uses AI. Check for mistakes.

Copilot AI Mar 12, 2026

Copy link

Choose a reason for hiding this comment

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

handleResize currently calls updateCanvasHeight, which sets the canvas height to clientHeight (viewport height). If the canvas has already been expanded via scrolling, any resize/observer trigger can shrink it back down and effectively cut off previously drawable areas. Consider keeping the existing expanded height (e.g., max(currentHeight, viewportHeight)) and only growing on resize, not shrinking.

Suggested change
updateCanvasHeight(fc);
const viewportHeight = document.documentElement.clientHeight;
const currentHeight = fc.getHeight();
const desiredHeight = Math.max(currentHeight, viewportHeight);
const newHeight = Math.min(CANVAS_MAX_HEIGHT_IN_PIXELS, desiredHeight);
if (newHeight !== currentHeight) {
fc.setHeight(newHeight);
fc.renderAll();
}

Copilot uses AI. Check for mistakes.
};
handleResize(); // Initial sizing

const handleScroll = () => {
updateDynamicCanvasHeight(fc);
};

let resizeTimeout: number;

const resizeObserver = new ResizeObserver(() => {
clearTimeout(resizeTimeout);
resizeTimeout = setTimeout(handleResize, 50); // debounce to prevent rapid calls
});

window.addEventListener("resize", initCanvasDimensions);
resizeObserver.observe(document.documentElement);
resizeObserver.observe(document.body);
window.addEventListener("scroll", handleScroll);

Copilot AI Mar 11, 2026

Copy link

Choose a reason for hiding this comment

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

updateDynamicCanvasHeight is invoked directly on every scroll event and performs multiple setDimensions calls (including a collapse to height 0). This can be expensive during scrolling. Consider throttling via requestAnimationFrame/debounce and registering the listener as { passive: true } since it doesn't call preventDefault().

Copilot uses AI. Check for mistakes.
Comment on lines +120 to +133

Copilot AI Mar 12, 2026

Copy link

Choose a reason for hiding this comment

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

The scroll handler runs on every scroll event and updateDynamicCanvasHeight does multiple setDimensions calls plus a scrollHeight read, which can force layout and be expensive during scrolling. Consider adding a requestAnimationFrame-based throttle and registering the listener as { passive: true } to reduce main-thread jank.

Copilot uses AI. Check for mistakes.

// Make all created paths erasable
fc.on("object:added", (e) => {
Expand All @@ -75,7 +141,8 @@ export function Canvas({ currentTool, setCurrentTool }: CanvasProps) {

return () => {
fc.dispose();
window.removeEventListener("resize", initCanvasDimensions);
resizeObserver.disconnect();
window.removeEventListener("scroll", handleScroll);
};
Comment on lines 142 to 146

Copilot AI Mar 11, 2026

Copy link

Choose a reason for hiding this comment

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

resizeTimeout can still fire after unmount/fc.dispose() (e.g., a ResizeObserver callback scheduled right before cleanup), which would call handleResize on a disposed Fabric canvas. Clear the pending timeout in the cleanup function to avoid post-dispose dimension updates/errors.

Copilot uses AI. Check for mistakes.
Comment on lines 124 to 146

Copilot AI Mar 12, 2026

Copy link

Choose a reason for hiding this comment

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

resizeTimeout is not cleared in the effect cleanup. If a resize event schedules handleResize and the component unmounts (disposing the Fabric canvas) before the timeout fires, the callback can run against a disposed fc. Clear the pending timeout during cleanup (and consider typing it as ReturnType<typeof setTimeout> | undefined to reflect the runtime behavior).

Copilot uses AI. Check for mistakes.
}, []);

Expand Down