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

ROI popup dialog fix #374

Merged
merged 1 commit into from
Mar 25, 2021
Merged

ROI popup dialog fix #374

merged 1 commit into from
Mar 25, 2021

Conversation

will-moore
Copy link
Member

Fixes #346

This stores the enable_shape_popup on the viewer, not on regions.
This means that enable_shape_popup flag won't get lost each time we reload Regions.

To test:

  • Load an image with paginated ROIs (over 500)
  • Select an ROI, then disable the Shape popup (X)
  • Load a different 'page' of ROIs
  • Select an ROI - popup should remain disabled.

This means that enable_shape_popup flag won't get lost each time we reload Regions
@will-moore will-moore changed the title Store enable_shape_popup on viewer, not regions ROI popup dialog fix Mar 17, 2021
@pwalczysko
Copy link
Member

Atm, struggling to find an image with enough ROIs on one plane, will try tomorrow or later.

@will-moore
Copy link
Member Author

This one has 150K ROIs https://merge-ci.openmicroscopy.org/web/webclient/?show=image-105665 (user-3)

@pwalczysko
Copy link
Member

pwalczysko commented Mar 19, 2021

This one has 150K ROIs https://merge-ci.openmicroscopy.org/web/webclient/?show=image-105665 (user-3)

Sorry to be so choosy, but this one has so many ROIs that it does not load in iviewer for more than 10 minutes now...
I will try to put an intermediate number of ROIs on some image...

Edit: It finally loaded, but then I hit a problem:

  1. on the first ROI page, select a ROI in the image
  2. disable the popup
  3. drag the slider on the ROI pagination a bit to the right
  4. observe that the slider you just dragged completely vanished (no idea on which page I am)
  5. observe that you can still select ROIs in the table and in the image, and indeed, there is no popup but
  6. the message says "0 ROIs on the current plane", see screenshot below (but this image has just one z and one t plane, so the message is errorneous)

Screenshot 2021-03-19 at 17 08 51

@pwalczysko
Copy link
Member

at present, on merge-ci, there is following behaviour. Probably because of #351 (comment) , when I have an image with, say 563 ROIs, the natural expectation is that on first page, I will have 500 ROIs, and on second page, 63. But the #351 (comment) is causing that the ROIs are just being added to both display and the ROI table (so first, I have 500 ROIs on page 1, but when I move to page 2, I can see 563 ROIs. Thus, it is impossible to figure out which ROIs are on page 1 and which on page 2. Thus it is very hard to test this PR.

@will-moore
Copy link
Member Author

@pwalczysko I have closed #351 for now, so it won't interfere with testing of this PR.

@pwalczysko
Copy link
Member

Yes, now I can see that the ROI loading is per page, and understand which ROIs are on which page, thanks.

The ROI popup works as expected. Ready to merge fmpov

@jburel jburel merged commit 4f5af1d into ome:master Mar 25, 2021
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.

ROI interaction
3 participants