Skip to content

Firefly-1889: update table row viewer with improved masking#1873

Merged
robyww merged 2 commits intodevfrom
FIREFLY-1889-masking
Nov 4, 2025
Merged

Firefly-1889: update table row viewer with improved masking#1873
robyww merged 2 commits intodevfrom
FIREFLY-1889-masking

Conversation

@robyww
Copy link
Contributor

@robyww robyww commented Oct 30, 2025

Firefly-1889: update table row viewer with improved masking

  • fix issues with TableRowViewer masking that was not right in my last PR
  • Create a more PlotView like look during working
  • refactor working task management to work better with promises
    • task are added with promises that are removed when the promises fulfills. The is not longer a remove task call.
    • removed code that is now unnecessary

Testing

@robyww robyww added this to the 2025.5 milestone Oct 30, 2025
@robyww robyww self-assigned this Oct 30, 2025
@robyww robyww added the UI Client side UI changes not related to any of the visualizers label Oct 30, 2025
@robyww robyww force-pushed the FIREFLY-1889-masking branch from 10abbff to 5cf5535 Compare October 31, 2025 16:24
@kpuriIpac
Copy link
Contributor

kpuriIpac commented Nov 3, 2025

I looked at the old PR and this PR. It's of course a complex component, code would probably be better reviewed by @jaladh-singhal when he's back (not blocking for this PR, I just mean in a future iteration of work with this code).

I think the masking is working better, but I did notice a few small issues:

searching

This looks. fine as Searching Images is centered here. But after these images load, and you slide right by a few images really quick, you may get something like:

3

I am not suggesting this necessarily needs fixing, but just wanted to highlight it (the fact that only the image on the far right says Searching Image)

Looks good other than that!

@kpuriIpac kpuriIpac self-requested a review November 3, 2025 19:04
Copy link
Contributor

@kpuriIpac kpuriIpac left a comment

Choose a reason for hiding this comment

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

Looks good! Spent time trying to study code in the old PR and this one. Might need a deeper review eventually as well, but looks good to me.

@robyww robyww force-pushed the FIREFLY-1889-masking branch from 62ede9e to 6feb1ea Compare November 4, 2025 16:39
@robyww robyww merged commit 2b985ce into dev Nov 4, 2025
@robyww robyww deleted the FIREFLY-1889-masking branch November 4, 2025 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

UI Client side UI changes not related to any of the visualizers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants