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

Thumbnail zooming #411

Merged
merged 4 commits into from
Mar 15, 2023
Merged

Thumbnail zooming #411

merged 4 commits into from
Mar 15, 2023

Conversation

will-moore
Copy link
Member

@will-moore will-moore commented Sep 30, 2022

This PR cleans-up the thumbnail zooming logic in webclient.

Instead of using the sizeX and sizeY values to calculate the aspect ratio for every thumbnail and JavaScript sets the width and height on the fly while dragging the thumbnail slider, we now use CSS to do achieve the same effect which is much simpler and might be faster/smoother.

To test:

  • Should see no change in behaviour for thumbnail zooming
  • The table layout icon has a small fix (see screenshots below)

NB: Previously this PR didn't load sizeXYZ values from OMERO until the user clicked on table layout to display them, but this was added complexity for negligible performance gain so it has been removed.

@pwalczysko
Copy link
Member

Tested on
Screenshot 2022-12-16 at 11 17 10

As discussed with @will-moore ^^^, the necessity of the refresh is not a great user experience, and the performance enhancement we saw on merge-ci is less than 10%, with total times well under 1 sec, i.e. not humanly perceptible.
The message wording ...the data... is confusing, I would remove this, but most probably, removing the whole necessity of the refresh is the better option.

Copy link
Member

@pwalczysko pwalczysko left a comment

Choose a reason for hiding this comment

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

some suggestions in #411 (comment)

@will-moore
Copy link
Member Author

Thanks @pwalczysko: fixed and re-deployed (and updated description).
Now, the only visible change in behaviour is the tiny fix to layout buttons. When on table layout:

Before:

Screenshot 2022-12-16 at 13 15 37

After:

Screenshot 2022-12-16 at 13 19 48

@jburel
Copy link
Member

jburel commented Feb 14, 2023

exclude label removed

@will-moore will-moore changed the title Don't load sizeX sizeY for thumbnail zooming Thumbnail zooming Feb 17, 2023
@pwalczysko
Copy link
Member

lgtm

@knabar knabar added this to the 5.19.0 milestone Mar 10, 2023
@knabar knabar merged commit 46196a1 into ome:master Mar 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants