Skip to content

Make sketching canvas responsive - #30

Merged
nritschel merged 10 commits into
masterfrom
responsive-canvas
Sep 24, 2026
Merged

nritschel merged 10 commits into
masterfrom
responsive-canvas

Conversation

@nritschel

Copy link
Copy Markdown
Collaborator

This addresses the scaling issue brought up in PrairieLearn/PrairieLearn#14904 for both editable and read-only canvases. The former need some extra helpers to adjust pixel coordinates, but the change is not as major as I initially expected.

@SybelBlue @nwalters512 any thoughts on this? I've updated the debugging page (if you run npm start inside the sketch_tool folder) to mimic the PL question/submission panels, so it should be easy to try this out without loading it into PL.

@SybelBlue SybelBlue left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Codex Code Review

Summary

The responsive canvas coordinate changes behave correctly, but the toolbar shrink algorithm introduces one substantial accessibility barrier on narrow layouts. The toolbar should retain usable control and text sizes before this is merged.

Priority rankings

  • P0: 0
  • P1: 0
  • P2: 0
  • P3: 0
  • A0: 0
  • A1: 0
  • A2: 1
  • A3: 0

Validation

  • Reviewed the complete merge diff from 691099d168cc1a9cda759f6e010e1b9f71341b1a through f9407f2d53773f782989e5346ea3f0bd30738f8c, the PR description, linked issue discussion, current PrairieLearn integration, and existing review activity.
  • All five GitHub CI checks are passing.
  • Local ESLint, Prettier, and production Webpack build passed.
  • Headless Chrome 153 confirmed responsive point placement and dragging map back to the expected SVG coordinates.
  • At a 320px viewport, the provided allPlugins toolbar rendered its first button at approximately 18.2 × 25.8 CSS pixels.
  • There is no automated JavaScript/browser interaction suite; manual browser validation was limited to Chrome.

Review configuration

  • Model: not exposed by current runtime
  • Thinking level (reasoning effort): not exposed by current runtime

Comment thread sketch_tool/lib/main.js Outdated
@SybelBlue

Copy link
Copy Markdown
Collaborator

Messing around with this further, it looks good to me! I'm excited to get this deployed...

@nritschel

Copy link
Copy Markdown
Collaborator Author

I changed the menu bar so it doesn't scale the buttons but only consumes the whitespace and then adds a "More" item that has the overflowing items. Tested manually and reviewed by Claude.

@nritschel
nritschel merged commit 601cd56 into master Sep 24, 2026
5 checks passed
@nritschel
nritschel deleted the responsive-canvas branch September 24, 2026 01:13
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