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

feat: Add filter prop to picker #2125

Closed

Conversation

AkshatJawne
Copy link
Contributor

@AkshatJawne AkshatJawne commented Jul 4, 2024

No description provided.

@AkshatJawne AkshatJawne self-assigned this Jul 4, 2024
@AkshatJawne AkshatJawne marked this pull request as draft July 4, 2024 14:50
@AkshatJawne AkshatJawne changed the title WIP debugging issues with filter not being applied feat: Add filter prop to picker Jul 4, 2024
Copy link
Member

Choose a reason for hiding this comment

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

Okay, so to sum up the problem a bit:

  • Lets say we have three partition columns A,B, which results in the following table with four keys, E.g.
A B
0 0
0 1
1 0
1 2
  • If you haven't selected anything yet, the options for A should be 0, 1, and B should be 0, 1, 2 (size 0 is duplicated)
  • If you've selected A=1, then the options for B should just be 0, 2
  • We do a selectDistinct with a partition column, so that we only get the unique options for that partition instead of duplicating items (like 0 in B)
    • After you do a .selectDistinct to get the options for a partition column, we won't be able to filter it after that based on other partition columns, since we've only got that column in the outputted table
  • We should call applyFilter before we call selectDistinct.

So I think this needs to be re-factored a bit. Right now we're building the partitionTables in componentDidMount only, but we should also be updating them after handlePartitionSelect. Roughly what we should be doing...

  • Change updatePartitionFilters to updatePartitionOptions, and call it from componentDidMount instead of initializing partitionTables and partitionFilters in there (though we could still initialize the keysTable, and keep that around)
  • In updatePartitionFilters, actually build the partitionTables by applying a filter to the keysTable first, then doing a .selectDistinct (I think you can just do applyFilter and then call .selectDistinct to get a new table)
  • Just pass those partitionTables down, no need other filters to be passed down (since it's already passed in).

Let me know if that works.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changes implemented in latest push to #2110

@AkshatJawne AkshatJawne requested a review from mofojed July 4, 2024 19:42
@AkshatJawne
Copy link
Contributor Author

Can close this PR, implemented changes in Improvements PR linked in comment, so that could test with all other code changes

@AkshatJawne AkshatJawne closed this Jul 8, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jul 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants