-
Notifications
You must be signed in to change notification settings - Fork 3
Show spinner during UMAP cluster selection #197
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
Conversation
|
@copilot The spinner is not appearing when I click on a cluster in the umap. Perhaps there is a z-level issue? |
Fixed in commit a3e9fbb. The issue was a z-index conflict - the spinner had z-index: 1000, same as the UMAP window, causing Plotly's canvas elements to render above it. Changes:
The spinner should now be clearly visible when clicking on clusters. |
|
@copilot Unfortunately this doesn't seem to have fixed the issue. Do you have any other ideas why the spinner might not be appearing? |
Fixed in commit 30b6d93. The root cause was that the JavaScript operations were completely synchronous - the spinner was shown and hidden within the same event loop tick, preventing the browser from painting it. Solution: Added The spinner should now be visible during cluster selection, especially on large clusters (>2000 images) where sorting takes 1-3 seconds. |
Show the UMAP spinner immediately when a user clicks on a cluster or image in the semantic map, providing visual feedback during processing. The spinner is hidden after the cluster selection and UI updates complete. - Added showUmapSpinner() call at the start of handleClusterClick() - Added showUmapSpinner() call at the start of handleImageClick() - Wrapped operations in try/finally to ensure spinner is always hidden - Added requestAnimationFrame delay to ensure UI updates before hiding spinner This improves UX on large photo collections where cluster selection can be slow. Co-authored-by: lstein <[email protected]> Add setTimeout(0) to allow spinner to render before heavy computation The issue was that the synchronous operations were executing immediately after showing the spinner, without giving the browser a chance to paint the DOM changes. By adding `await new Promise(resolve => setTimeout(resolve, 0))`, we yield control back to the event loop, allowing the browser to render the spinner before the expensive cluster sorting operations begin. This ensures the spinner is visible during the 1-3 second delay on large clusters. Co-authored-by: lstein <[email protected]>
30b6d93 to
4613d58
Compare
lstein
left a comment
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.
Tested and works well.
Performance Enhancement: Show Spinner on UMAP Cluster Click ✅
Summary
This PR addresses the user experience issue where clicking on clusters in the UMAP semantic map has no visual feedback during processing. On larger photo collections, there's a noticeable delay between clicking and seeing results. This change shows a spinner immediately to inform users that the system is processing their request.
Changes Made
Modified
/photomap/frontend/static/javascript/umap.js:handleClusterClick() (lines 1256-1298):
setTimeout(0)to allow spinner to renderhandleImageClick() (lines 1300-1326):
setTimeout(0)to allow spinner to renderSpinner z-index fix:
#umapSpinnerz-index from 1000 to 10000 to appear above Plotly plotpointer-events: noneto allow clicks through spinner containerEvent loop fix (latest commit):
await new Promise(resolve => setTimeout(resolve, 0))after showing spinnerKey Improvements
Testing
Performance Impact
For large clusters (>2000 images), the sorting algorithm can take 1-3 seconds. The spinner provides crucial feedback during this time, addressing the original issue. The setTimeout(0) adds negligible overhead (~1ms) but ensures visual feedback is displayed.
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.