Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't update zoom to undefined on setId #550

Merged
merged 2 commits into from
Apr 14, 2024

Conversation

will-moore
Copy link
Member

Fixes a bug introduced in #527: On Edit ID the panel zoom is updated to undefined since the incoming image data has no zoom info so data.zoom is always undefined.

This results in rendering errors - e.g. Zoom slider 0 and other NaN values below:

Screenshot 2024-04-08 at 17 49 39

To test:

  • Zoom a panel, then Edit ID to replace image with another.
  • The zoom should remain unchanged and after de-selecting and re-selecting the panel (to trigger update of right panels) the zoom controls, viewport x, y, width, height etc should be rendered correctly.

cc @Rdornier

@will-moore
Copy link
Member Author

@jburel - Any idea what's failing in the above job? Seems that npm isn't installed.

Same error on #549 (and I assume on all other PRs when they next try to run checks):

+ python setup.py sdist
running sdist
npm install
error: command 'npm' failed: No such file or directory

Thanks

@Rdornier
Copy link
Contributor

Rdornier commented Apr 9, 2024

I can indeed reproduce the bug.
I didn't thought about testing this feature when I added this feature.
Thanks for reporting and fixing it.

@pwalczysko
Copy link
Member

Reproduced on demo (without this PR). See it fixed on merge-ci.

@will-moore will-moore merged commit 7f8b301 into ome:master Apr 14, 2024
1 check passed
@will-moore will-moore added this to the 6.2.1 milestone Apr 19, 2024
@will-moore
Copy link
Member Author

Released in omero-figure 6.2.1

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.

3 participants