ome / omero-iviewer

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

Tooltip with shape comment displayed on mouse hover even if "Show comments" is not checked #442

Closed hpawe01 closed 1 year ago

hpawe01 commented 1 year ago

I don't know what is the expected behavior for the "Show comments" checkbox on the "ROIs" tab and I couldn't find something relevant in the code:

Here https://github.com/ome/omero-iviewer/blob/master/src/viewers/viewer/source/Regions.js#L114 it says, that this flag should determine if texts for shapes other than labels should be displayed. Is "text" == "comment" in this context?

The only place where this flag is used is here https://github.com/ome/omero-iviewer/blob/master/src/viewers/viewer/utils/Style.js#L258, but I don't see how this effects the visibility for comments of shapes in the viewer.

I noticed, that comments for shapes (other than labels) are displayed in a small tooltip on mouse hover if the shape is not selected.

Now I would expect that those mouse hover tooltips are only displayed on mouse over if "Show comments" is checked. That is not the case at the moment. The tooltips are displayed always.

I added a check for this.regions_.show_comments_ here https://github.com/ome/omero-iviewer/blob/master/src/viewers/viewer/interaction/Hover.js#L88 and now it works as expected. Should I create a pull request for this?

will-moore commented 1 year ago

Hi - thanks for your query.

The "show comments" checkbox toggles the display of a label for a shape on the image. I think that the text is set for the style at https://github.com/ome/omero-iviewer/blob/b4b7b2bae474ddd122ac158d52284f85f1ef3ece/src/viewers/viewer/utils/Style.js#L268 if show_comments is True and the shape isn't a Text shape (label).

Screenshot 2023-01-11 at 16 01 11

Screenshot 2023-01-11 at 16 01 21

I don't think the checkbox is supposed to change the visibility of the tooltip.

hpawe01 commented 1 year ago

Hi @will-moore

thank you for the quick and informative reply. I tried it and at first nothing happened. Than I realized, that I had to increase the font size of the comment to 1000 to make it visible at the zoom level where I needed to show the comments.

I understand that the comment that is shown in the viewer is just a text shape that needs to have a specific size set. But I expected the comment to be visible independent of the zoom level in the same size. Similar to the arrow head I want the comment text shape to be always visible in the size that we define (in a way it should "grow" when we zoom out or "shrink" when we zoom in, but actually it keeps a fixed size on the screen).

Why do I need to use it like this? We have a group of students that should mark a specific region (e.g. a cell of a type). We use arrows for that (they should point to the relevant region) because the arrow head is always visible, so we can get an overview of all the regions that where marked when we look at the full picture. But we also zoom to specific regions to see, if the students pointed to the correct one. Now the comment attribute is used to show which student drew which arrow. I want to see/show this information when looking at the whole image but also when I zoom to a specific location. That is why I need to have a fixed size comment in a way that it stays the same size independent of the zoom level.

Maybe we could add a flag per comment to toggle fixed size on and off and set "off" as default to keep the current behavior. Or do you have another idea?

will-moore commented 1 year ago

I actually agree that in most cases the text should just appear at a readable size, and if you set it to 12px then it should just show at that size regardless of zoom. This seems to be the default behaviour for openlayers (e.g. https://openlayers.org/en/latest/examples/vector-labels.html) but it will need a bit of digging to work out what to change in iviewer.

will-moore commented 1 year ago

OK, so I figured out at least where to control this (see #443) but I haven't done the configuration yet

hpawe01 commented 1 year ago

Thank you for digging into that and creating a pull request to change the default behavior!

For my use case I will use now this viewer method to control the scale https://github.com/ome/omero-iviewer/blob/4f5af1d6a14e8d2921e97b44840988a0b1e3775a/src/viewers/viewer/Viewer.js#L959 (thanks for the hint in your pull request). And I will implement a custom event for handling the tooltip visibility.

will-moore commented 1 year ago

Hi @hpawe01 - For that PR, I was planning on adding configuration so that you can disable the Text scaling on zoom (with the default behaviour staying the same - so it's not a breaking change).

But if I understand correctly, you will have your own solution and are not depending on that PR (?)

hpawe01 commented 1 year ago

Hi @will-moore, for now I will use the mentioned method to control to text scaling behavior. But as soon as the PR is merged, I will switch to this configuration solution. So I am depending on this PR in the long(er) run to keep the differences between my and the official version as minor as possible.