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

Manual zoom level #517

Merged
merged 15 commits into from
Oct 31, 2023
Merged

Manual zoom level #517

merged 15 commits into from
Oct 31, 2023

Conversation

Rdornier
Copy link
Contributor

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.

@will-moore
Copy link
Member

Hi @Rdornier - see Rdornier#1 for some fixes

@Rdornier
Copy link
Contributor Author

Hi @will-moore

Many thanks for your corrective input ; it perfectly works 👍 !
I've added few more lines to update the image in the figure.
I tested it and it work fine for me.


// update the figure image
this.zoom_avg = value;
var to_save = {'zoom': value};
Copy link
Member

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" !

Copy link
Contributor Author

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)

@will-moore
Copy link
Member

Looking good - I think it could do with a CSS tweak. Trying this:

$ git diff
diff --git a/omero_figure/static/figure/css/figure.css b/omero_figure/static/figure/css/figure.css
index 332b657..ad108bb 100644
--- a/omero_figure/static/figure/css/figure.css
+++ b/omero_figure/static/figure/css/figure.css
@@ -233,6 +233,11 @@
         height: 14px;
         border-radius: 5px;
     }
+    #vp_zoom_value {
+        float: right;
+        width: 50px;
+        margin-top: 5px;
+    }
     .toggle_channel {

gives a nicer layout:

Screenshot 2023-09-28 at 16 36 07

Copy link
Member

@will-moore will-moore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@jburel jburel merged commit e5b8c53 into ome:master Oct 31, 2023
1 check passed
@will-moore will-moore added this to the 6.1.0 milestone Nov 2, 2023
@imagesc-bot
Copy link

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

@Rdornier Rdornier deleted the manual-zoom-level branch June 13, 2024 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants