Codecov Report

Attention: 29 lines in your changes are missing coverage. Please review.

Comparison is base (db9aea7) 77.33% compared to head (0217a51) 76.57%.

@@            Coverage Diff             @@
##             main     #291      +/-   ##
==========================================
- Coverage   77.33%   76.57%   -0.76%     
==========================================
  Files          16       16              
  Lines        1897     1921      +24     
==========================================
+ Hits         1467     1471       +4     
- Misses        430      450      +20     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Hi! Thanks for testing. The implementation is only done for histograms, not for point clouds

Hi! Thanks for testing. The implementation is only done for histograms, not for point clouds

Okay. Good to know.

I tested with histograms and there it works as expected. The only thing, I would suggest: when using Ctrl + Click outside any cluster, it would find it more intuitive, if nothing changes, instead of the deletion of all clusters. The latter is the standard behavior, when just clicking. In a File Explorer (e.g. in Windows), if you select multiple files/folders with Ctrl + Click and then click in an empty area, the selection of files/folder does not change (i.e. is not cleared). I would recommend to follow to this pattern, since it might be intuitive to users, since they are used to this pattern.

From a user perspective, it would of course be nice to have the same behavior also in point scatter plots. Users may interpret it otherwise not as a missing feature for the scatter plot, but as a bug and report it as such.

Regarding the code: I do not feel confident enough to give feedback regarding the code.

Hi!

Now nothing happens when control is pressed. Good point!

I agree that it should also work for point scatter plots. However, the current implementation depends on the histograms. We can implement a more generalized mechanism, by saving the individual paths for the lassos, but the current implementation depends on the histogram overlay and is not easy to generalize.

Any chances to see that merged without the additional implementation for scatter + histograms? As far as I can see this would require a bigger refactoring.

Hey @thorstenwagner,
I am unsure if we should add this feature here if it is only a histogram specific one. I generally love the functionality but we are currently also planning a rehaul of the code structure and maybe it just might be better to add a more generalized version then.. maybe @zoccoler and @jo-mueller can also chime in?

Hey @thorstenwagner, I am unsure if we should add this feature here if it is only a histogram specific one. I generally love the functionality but we are currently also planning a rehaul of the code structure and maybe it just might be better to add a more generalized version then.. maybe @zoccoler and @jo-mueller can also chime in?

Its a very useful but not critical feature for me. So its fine for me if we add this later.