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

Block UI for some long running tasks #543

Merged
merged 9 commits into from
May 17, 2024
Merged

Conversation

knabar
Copy link
Member

@knabar knabar commented Mar 13, 2024

The "Save to all" function in the Preview panel can take a long time to complete for datasets or plates that have lots of images. The user may refresh or reload the page before everything (including thumbnail refresh) completes, exacerbating the already high load on the server.

The same applies to copying and pasting rendering settings using the right-click menu in the tree.

This PR introduces a UI blocking dialog until thumbnails are reloaded:

Screenshot 2024-03-13 at 15 03 59

The UI blocking is generic and can be used anywhere where a Promise is available:

OME.progress_overlay(
  new Promise((resolve, reject) => {
    // some long running task
  }),
  'Message for UI blocking dialog');
});

The dialog disappears automatically when the Promise resolves or rejects.

By default the duration of the UI blocking is logged to the console:
Screenshot 2024-04-12 at 13 12 25

@knabar knabar changed the title Make long-running tasks like "Save to all" more robust Block UI for some long running tasks Apr 12, 2024
@knabar knabar marked this pull request as ready for review April 12, 2024 11:14
@will-moore
Copy link
Member

This looks good and is working fine in testing.

I wonder if we want to consolidate this with function gs_modalJson (url, data, callback) { which is used for the "Save" rendering settings button and gives a different look:

Screenshot 2024-04-15 at 14 15 08

Having the 2 different blocking UIs for adjacent buttons is slightly unexpected, but certainly not a blocker.

@will-moore
Copy link
Member

In testing, I noticed that "Save to All" functionality appears to ignore LUTs. The LUTs are used to generate thumbnails but are not actually saved, leading to a case where the thumbnails (with LUTs) are out of sync with the image rendering settings (without LUTs):
However, this isn't coming from this PR - same behaviour before.

Screenshot 2024-04-15 at 15 09 23

@knabar
Copy link
Member Author

knabar commented Apr 16, 2024

This looks good and is working fine in testing.

I wonder if we want to consolidate this with function gs_modalJson (url, data, callback) { which is used for the "Save" rendering settings button and gives a different look:

Having the 2 different blocking UIs for adjacent buttons is slightly unexpected, but certainly not a blocker.

Thanks for pointing that out - I'll dig in a bit and see if I can clean it up.

@knabar
Copy link
Member Author

knabar commented Apr 16, 2024

In testing, I noticed that "Save to All" functionality appears to ignore LUTs. The LUTs are used to generate thumbnails but are not actually saved, leading to a case where the thumbnails (with LUTs) are out of sync with the image rendering settings (without LUTs): However, this isn't coming from this PR - same behaviour before.

Opened a new issue #552

@knabar
Copy link
Member Author

knabar commented Apr 22, 2024

I wonder if we want to consolidate this with function gs_modalJson (url, data, callback) { which is used for the "Save" rendering settings button and gives a different look:

gs_modalJson was only used in one location, it's now replaced with the new functionality.

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.

Code removal / update looks good and functionality is working, Thanks

@knabar knabar added this to the 5.26.0 milestone May 7, 2024
@knabar knabar merged commit e3b4f23 into ome:master May 17, 2024
10 checks passed
@knabar knabar deleted the fix-save-to-all branch May 21, 2024 10:13
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.

2 participants