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

Right-click filtering should filter column to all values in selection, not just the row clicked #1233

Closed
rbasralian opened this issue Apr 18, 2023 · 3 comments · Fixed by #1736
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@rbasralian
Copy link

When multiple rows are selected, right-clicking and using "filter by value" should filter based on all selected values, not only the row that was right-clicked.

This is how it looks now — I can only filter to one row, even though I selected three:

image

In the Swing UI, the dialog changes to "Filter by Values", and allows you to filter based on multiple rows at once:

image

This is extremely useful when drilling into big tables (e.g. selecting a handful of stock ticker symbols to look at, or selecting a handful of ID numbers from some state log). It also makes it easy to filter to a subset of data so you can look at summary statistics from the column headers.

@rbasralian rbasralian added enhancement New feature or request triage Issue requires triage labels Apr 18, 2023
@vbabich vbabich added this to the May 2023 milestone Apr 19, 2023
@vbabich vbabich removed the triage Issue requires triage label Apr 19, 2023
@dsmmcken
Copy link
Contributor

dsmmcken commented Apr 21, 2023

Changes we want to make:

  • Right-clicking on a row that isn’t selected should immediately change the selection to that row and present the menu options for that row
  • Right-clicking inside an existing selection should present the filter by value menu based on the selection size. If there's only one selected row, it should function as it does currently. If there are multiple selected rows, it should present a "Filter by Values" (plural) menu. The preview at the top should show several values together to show that it is filtering by multiple values (although that may not be possible if the values are too long to show more than one). The UI will need to take a snapshot of the selection to get any values part of an offscreen selection. The snapshot should happen when the user performs the initial right-click, just as it does for a single row. We should limit the snapshot to the first 1000 values or something as to not kill the client if the user has a whole table selected. For input tables where you might have multiple individual cells selected in different columns, use only the selected cells from the column that was clicked.

Display the number of rows selected below the values when there are multiple rows selected as shown:
image

@dsmmcken dsmmcken modified the milestones: May 2023, June 2023 May 16, 2023
@mofojed mofojed modified the milestones: June 2023, July 2023 Jun 28, 2023
@mofojed mofojed modified the milestones: July 2023, August 2023 Aug 28, 2023
@mofojed mofojed modified the milestones: August 2023, September 2023, October 2023 Sep 11, 2023
@mofojed mofojed modified the milestones: October 2023, December 2023 Dec 4, 2023
@mofojed mofojed modified the milestones: December 2023, January 2024 Jan 9, 2024
@wusteven815
Copy link
Contributor

A few questions regarding the filter's behaviour:

  • What should happen when null is part of the selection range? For example, for strings, should it use the null menu or the string menu? If string menu, what would happen to with the "contains", "starts with", and "ends with" actions? (And similarly with numbers and those comparisons)
  • What should the logic be with number comparisons? For example, if 1, 2, 3 are selected, what would the "greater than" comparison mean (>1 or >3)?

@dsmmcken
Copy link
Contributor

  1. If the user right clicks on a null value or boolean value directly, still show the null/boolean menus. It would be bad performance to snapshot and iterate client-side an entire selection before showing a menu. If the user wants a filter by all, they will need to click a non-null row.
  2. For contains, starts with, ends with, remove null values when setting the filter. So if the selection was "a", "b", "c" and null, it should be just contains a, b, c. To be clear, equals/not equals should be ORs and contains/starts with/ends with should be ANDs
  3. Logic for numbers same as strings. OR for equal/not equals, AND for everything else.

I think that makes sense.

@wusteven815 wusteven815 linked a pull request Jan 19, 2024 that will close this issue
wusteven815 added a commit that referenced this issue Feb 6, 2024
- Feature: #1233
  - Moved cell filter actions to its own `async` function that gets a
snapshot to supports multiple values
  - A maximum of 1000 (`MAX_MULTISELECT_ROWS`) rows will be snapshotted
for unique values. The reason why its rows and not unique values is to
prevent the case of a very large amount of rows with a very small amount
of unique values.
  - Add tests for above by running E2E tests on all the filters
- Changed right-click behaviour such that if right-clicked outside of
the current selection, the new selection will be the row the cursor is
on
  - Add tests for above, add function to right-click in Grid tests
- Add assert to check if an array/map is non-empty, and if an number is
not NaN
  - Add tests for above

---------

Co-authored-by: Mike Bender <mikebender@deephaven.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants