Skip to content

Share rendering and pan/zoom helpers between widgets - #235

Open
henryhng wants to merge 3 commits into
electronmicroscopy:mainfrom
henryhng:widget-shared-helpers
Open

Share rendering and pan/zoom helpers between widgets#235
henryhng wants to merge 3 commits into
electronmicroscopy:mainfrom
henryhng:widget-shared-helpers

Conversation

@henryhng

Copy link
Copy Markdown
Contributor

What problem does this solve?

ChooseLattice previously implemented its array-to-PNG rendering, keyword-arg typo validation & canvas pan/zoom transform internally. A second widget (ShowBragg) now requires the same functionality, so it might be helpful to move these components into shared modules.

What should reviewers verify?

  1. Confirm that ChooseLattice behavior remains unchanged.
  2. Review the extracted image-view transform in js/imageView.ts.

Verification

  • PYTHONPATH=src pytest -q tests/test_choose_lattice.py — 9 passed.
  • npx vitest run js/imageView.test.ts — 2 passed.
  • npm run build — works.
  • npx tsc --noEmit — works.
  • No UI behavior changes are intended

@github-actions

Copy link
Copy Markdown

This PR is 1 commit behind electronmicroscopy/quantem.widget:main.

Fetch that remote and update local main before you branch, or rebase this branch onto it before review.
A warning is enough. The PR can stay open while main moves.

@github-actions

Copy link
Copy Markdown
  • Keep henryhng/quantem.widget:widget-shared-helpers until this PR is merged.
  • After merge, delete henryhng/quantem.widget:widget-shared-helpers if you own it and no other PR uses it.
  • Then remove its dedicated worktree and local widget-shared-helpers branch and update local main.

@bobleesj

bobleesj commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator

@henryhng henry thanks for this PR but a bit too early for abstraction.

UI stuff, better to duplicate,
Compute stuff, if used all other widgets, can be abstracted.

UI is harder to test and easier to break. Please keep building, 1000 or 2000 lines of UI code, etc it doesn't really matter in the age of agentic coding.

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