ome / omero-iviewer

An OMERO.web app allowing to view images
https://www.openmicroscopy.org/omero/iviewer/
Other
18 stars 29 forks source link

ROI popup dialog fix #374

Closed will-moore closed 3 years ago

will-moore commented 3 years ago

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:

pwalczysko commented 3 years ago

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

will-moore commented 3 years ago

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

pwalczysko commented 3 years ago

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 commented 3 years ago

at present, on merge-ci, there is following behaviour. Probably because of https://github.com/ome/omero-iviewer/pull/351#issuecomment-698260802 , 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 https://github.com/ome/omero-iviewer/pull/351#issuecomment-698260802 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 commented 3 years ago

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

pwalczysko commented 3 years ago

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