-
Notifications
You must be signed in to change notification settings - Fork 30
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
Popup on shapes #248
Popup on shapes #248
Conversation
Doesn't seem to get called?
55c52eb
to
e727430
Compare
I think user would ask immediately: How do I switch that off ? It is nice that it is there, but maybe I do not want those popups as you rightly remark ? |
Will test the smaller features tomorrow. |
Addressed some points above:
|
the bird's eye view bug #248 (comment) is fixed now. The new features listed in #248 (comment) work as described. Quite happy about them. The only point is the position of the popup with respect to a ROI in cases when the ROIs are lines or arrows pointing at 45 degrees angle. In such cases the popup is too far from the ROI. Discussed a solution with @will-moore in the office, where for lines and arrows the position of the popup could be at the topmost end of the ROI, instead of using the centre of the top of bounding box (used for all shapes now) |
Entering text work and the right-hand panel is updated. Hitting enter after entering text could close the dialog. |
@pwalczysko do you want to over the last set of changes? |
Having the dialog popping up every time a shape is selected is a bit annoying. |
@jburel discussed the solution of #248 (comment) in the office - I guess @will-moore will come up with a solution commit soon. Otherwise, all is good here, and this is ready to go FMPOV |
@jburel - you can turn off the popups with a right-click menu on the image. But as discussed with @pwalczysko this isn't obvious to find it. |
I certainly did not think to right click on the image to turn the dialog on/off. |
90d77c8
to
52df14f
Compare
As @pwalczysko noticed on images with many ROIs (where they are paginated). After moving to a new plane, the shapes don't appear selected in the table, clicking on shapes doesn't select on first click, picking line colour doesn't show up on shape. |
ShapeEditPopup.showPopupForShape() was calling regions.renderFeature(f) when Z/T changes and with paginated ROIs being destroyed and created on Z/T switch it seems that regions.viewer_ can be undefined at this time.
To test last fixes:
Known issue (I think this is not related to this PR): If your line color-picker is showing a color (e.g. red) and you select a shape where the line has no color. Then you click the color-picker and don't choose a different color but just click "choose" to select red, the shape doesn't update because no color change is detected. |
We need this since viewers/viewer/controls/ShapeEditPopup.js imports Ui from '../../../utils/ui'. Importing this class that uses @decorator syntax seems to require reflect-metadata for karma to run in ChromeHeadless
RFE: If not technically too difficult: Slightly unexpected is that when I re-enable the popups, the ROI which is selected already, does not obtain a popup. Only new selection do. I think better would be the behaviour where the popups would come immediately back after I re-enabled them. |
Two RFEs otherwise looks good. |
Works as expected. Ready to merge FMPOV |
The popup now gives an indication on how to re-activate it, pointing at the right-click menu. |
This addresses:
This shows a transient 'tooltip-like' popup when you mouse over (or within 5 pixels) of a shape, to show the shape Label (IF it has one). This tracks the mouse.
When you click on the shape, a more stable popup shows you the shape coordinates and a text-box for editing the shape Label:
You also see this while dragging, editing and creating a shape, which can help if you want to know the coordinates. Could maybe also add the Length OR Area.
To test.... NB: since this is quite a big change, we probably want to assess this for a while before merging. Need to see whether the popup gets in the way at-all or is annoying to have it all the time. And what info should it show?