-
Notifications
You must be signed in to change notification settings - Fork 31
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
Manual zoom level #517
Manual zoom level #517
Conversation
Hi @Rdornier - see Rdornier#1 for some fixes |
Fix event handling of manual-zoom-level
Hi @will-moore Many thanks for your corrective input ; it perfectly works 👍 ! |
|
||
// update the figure image | ||
this.zoom_avg = value; | ||
var to_save = {'zoom': value}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Rdornier Just testing this, and I realise that the value
never gets converted from a string to an Integer.
JavaScript works OK with the string as it casts it on the fly for e.g. value < minVal
etc.
But when we save it to Figure JSON model, it gets saved as a string (since we don't have any validation of the model).
This means that when you subsequently select multiple panels, the average zoom values aren't calculated properly. E.g. "100" + "200" = "100200"
!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@will-moore Ok, I've just added a commit to convert the value coming from the UI from string to int (in the listener function and the slider implementation)
Looking good - I think it could do with a CSS tweak. Trying this:
gives a nicer layout: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
This pull request has been mentioned on Image.sc Forum. There might be relevant details there: https://forum.image.sc/t/omero-figure-crop-uses-original-image-aspect-ratio-after-rotation/89444/2 |
Hello,
I try to make to necessary changes to add manually the zoom level (fix part of #428).
However, I struggle a lot to test the changes in my dev env (i.e. changes in the code are not always taken into account when I test them).
So, feel free to test and to give me feedback.
Rémy.