website: Fix sidebar in examples #1874
Conversation
zbigg
force-pushed
the
zbiggg/website-fix-examples-sidebar
branch
from
773a9e2 to
8067068
Compare
|
|
||
| } else { | ||
| animationLoop = makeAnimationLoop(props.template as unknown as typeof AnimationLoopTemplate, { |
There was a problem hiding this comment.
Nit: put such a nasty cast on its own line?
| animationLoop = makeAnimationLoop(props.template as unknown as typeof AnimationLoopTemplate, { | |
| const animationLoopTemplate = props.template as unknown as typeof AnimationLoopTemplate; | |
| animationLoop = makeAnimationLoop(animationLoopTemplate, { |
| const animationLoopRef = useRef<AnimationLoop | null>(null); | ||
| const [canvas, setCanvas] = useState<HTMLCanvasElement | null>(null); | ||
| const usedCanvases = useRef(new WeakMap()) | ||
| const currentTask = useRef<Promise<any> | null>(null); |
There was a problem hiding this comment.
Is it possible to type this Promise stronger, instead of any? This part is probably the most confusing in the new code, so comments and types will help.
| console.log(`unmounting ${deviceType}`); | ||
| if (animationLoopRef.current) { | ||
| animationLoopRef.current.stop(); | ||
| animationLoopRef.current.destroy(); |
There was a problem hiding this comment.
Do we need to call both stop and destroy?
There was a problem hiding this comment.
indeed, only destroy is needed
| } | ||
|
|
||
| if (device) { | ||
| (device as any)?.destroy(); |
There was a problem hiding this comment.
Why is this cast needed? Seems like something we should fix.
There was a problem hiding this comment.
added destroy to Device
|
If you browse through the examples by clicking in the sidebar you will pretty quick crash the browser page. this is because WebGL contexts do not have a destroy method, and the limit on creating new contexts is fairly low (~10). Therefore it would be very advantageous to be able to reuse the canvas (reparenting it) when we switch example, as the webgl context is linked to the canvas. This would be a follow-up PR. |
Co-authored-by: Ib Green <ib@foursquare.com>
zbigg
force-pushed
the
zbiggg/website-fix-examples-sidebar
branch
from
7784baa to
db09110
Compare
Didn't experienced it in my browser, but i know that certain environments have this limitation. Good idea. |
For #1501
Background
Change List