feat: Calculate width/height of canvas based on container dimensions (".excalidraw" selector) & remove props width & height

[ad1992]

1. Remove width/height from the ".excalidraw" container so it will sized automatically.
2. updated all ref calculation to ".excalidraw" instead of parent since now ".excalidraw" will get resized
3. Remove props width/height as its not needed anymore.
4. Resize handler is also not needed anymore.
5. Position absolute canvas due to feat: Calculate width/height of canvas based on container dimensions (".excalidraw" selector) & remove props width & height #3379 (comment)

Tests show an extra rerender but that's only specific to the tests due to mocking the getBoundingClientRect. The real app actually does one less render.

Try here

• Testing
• fix tests
• update docs

[vercel]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/excalidraw/excalidraw/9wMRtzSKUnLYa5K1qdMn9nXgHeCS
✅ Preview: https://excalidraw-git-aakansha-wh-excalidraw.vercel.app

[dwelle]

3. Kept the props width/height which will be only used to determine the canvas width/height but we can remove it as well as we are calculating from parent.

I'd not remove the props since host apps may wanna do something custom and not make the width/height be dependent on parent container — though I guess it could be hacked by setting the width/height on the parent container instead — dunno, we'll see.

Btw, you're still defaulting the canvas width/height to window.innerWidth/innerHeight

[ad1992]

3. Kept the props width/height which will be only used to determine the canvas width/height but we can remove it as well as we are calculating from parent.

I'd not remove the props since host apps may wanna do something custom and not make the width/height be dependent on parent container — though I guess it could be hacked by setting the width/height on the parent container instead — dunno, we'll see.

Now that width/height is actually set by browser I think we shouldn't take it from props any more as it just sets the canvas width/height which can be calculated from ".excalidraw" container and we also need not handle the additional cases of updating offsets when width/height prop updates.

Btw, you're still defaulting the canvas width/height to window.innerWidth/innerHeight

yep since i am updating it in didMount so thats actually not getting used. I will remove that now.

[ad1992]

Fixed the tests, not sure if this is the best way to fix it, I had to stub the getBoundingClientRect at few places since now width/height is calculated using this.

[dwelle]

There's a problem where if you put the excalidraw component into a container with no explicit dimensions it'll keep on expanding due to a feedback loop because it seems there's some discrepancy between the <canvas> height and <main> height (in which the canvas is rendered) — 4px discrepancy, not sure where it comes from, but it causes an infinite loop:

https://codesandbox.io/s/excalidraw-v050-forked-9uure?file=/src/App.js

In the example above, I've added a max-height: 500px on the canvas so it doesn't loop ad infinitum and you can debug.

Making <main> into a flex container fixes it.

But, it just fixes this very case. Problem is any other thing can cause this feedback loop, e.g. adding margin on canvas or whatever (check the styles.scss comments).

Point is, dropping the <Excalidraw/> component into your DOM shouldn't cause an infinite loop no matter what layout/CSS you happen to have.

[ad1992]

There's a problem where if you put the excalidraw component into a container with no explicit dimensions it'll keep on expanding due to a feedback loop because it seems there's some discrepancy between the <canvas> height and <main> height (in which the canvas is rendered) — 4px discrepancy, not sure where it comes from, but it causes an infinite loop:

https://codesandbox.io/s/excalidraw-v050-forked-9uure?file=/src/App.js

In the example above, I've added a max-height: 500px on the canvas so it doesn't loop ad infinitum and you can debug.

Making <main> into a flex container fixes it.

But, it just fixes this very case. Problem is any other thing can cause this feedback loop, e.g. adding margin on canvas or whatever (check the styles.scss comments).

Point is, dropping the <Excalidraw/> component into your DOM shouldn't cause an infinite loop no matter what layout/CSS you happen to have.

Good catch 👍 . Yep this is a known issue / behaviour when using canvas when we try to resize it according to parent (example), I had faced the same issue in excal app but that got fixed after giving 100% height to #root but didn't realise it will happen when no dimensions on parent .
There are several stack overflow issues regarding the same as well like https://stackoverflow.com/questions/8624268/html5-canvas-resize-to-parent, https://stackoverflow.com/questions/45949881/canvas-element-not-maintaining-height-of-parent-div

There are two work around for this

1. Display block in canvas - this will make sure that 4px difference is not there but still it will have some initial height. Also adding padding, marging will still lead to circular loop since it contributes to increasing height (since we multiply by window.devicePixelRatio)
2. Position absoulte the canvas - This is safe fix as now there is no circular dependency, height is also 0 and margin, padding etc wont affect.

I have pushed the fix with absolute, you can try out with version aakansha-excalidraw@0.6.0-wh3, tested in your fork as well, works fine.

[dwelle]

Good idea about the absolute positioning 👍