Skip to content

refactor(frontend): useEffectListener-ify <CanvasView> listeners - #739

Open
jaskfla wants to merge 9 commits into
mainfrom
use-event-listener
Open

refactor(frontend): useEffectListener-ify <CanvasView> listeners#739
jaskfla wants to merge 9 commits into
mainfrom
use-event-listener

Conversation

@jaskfla

@jaskfla jaskfla commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

@jaskfla
jaskfla force-pushed the use-event-listener branch 4 times, most recently from 1c1943c to 0dbdb4c Compare June 10, 2026 01:21
@jaskfla
jaskfla force-pushed the use-event-listener branch from a13a3f8 to aaaf8df Compare June 10, 2026 02:37
Comment on lines 1296 to -1332
@@ -1308,7 +1285,6 @@ export default function CanvasView({
zoom={zoom}
/>
)}

<CanvasImageWrapper
aria-busy={isLaunching || isLoading}
ref={canvasImageWrapperRef}
@@ -1327,9 +1303,7 @@ export default function CanvasView({
style={{ minWidth: canvas.width, minHeight: canvas.height }}
/>
</CanvasImageWrapper>

<CanvasGrid zoom={zoom} hidden={!isGridVisible} />

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

readability :(

@jaskfla jaskfla Jun 10, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes, but the fix is to break it into smaller, composable parts!

@Rocked03 Rocked03 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

no particular thoughts

# Conflicts:
#	packages/frontend/src/components/canvas/CanvasView.tsx
@jaskfla
jaskfla enabled auto-merge June 10, 2026 10:10
# Conflicts:
#	packages/frontend/src/components/canvas/CanvasView.tsx
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants