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.

Sign up for GitHub

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

Jump to bottom

feat: Add filter prop to picker #2125

Closed
wants to merge 1 commit into from
Closed

feat: Add filter prop to picker #2125

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 32 additions & 21 deletions packages/iris-grid/src/IrisGridPartitionSelector.tsx
View file
Open in desktop
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

Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import Log from '@deephaven/log';
import { Picker } from '@deephaven/jsapi-components';
import type { dh } from '@deephaven/jsapi-types';
import { TableUtils } from '@deephaven/jsapi-utils';
import { FilterConditionFactory, TableUtils } from '@deephaven/jsapi-utils';
import { assertNotNull, Pending, PromiseUtils } from '@deephaven/utils';
import './IrisGridPartitionSelector.scss';
import { PartitionConfig, PartitionedGridModel } from './PartitionedGridModel';
Expand All @@ -26,7 +26,7 @@
partitionTables: dh.Table[] | null;

/** The filters to apply to each partition table */
partitionFilters: dh.FilterCondition[][] | null;
partitionFilters: FilterConditionFactory[][] | null;
}
class IrisGridPartitionSelector extends Component<
IrisGridPartitionSelectorProps,
Expand Down Expand Up @@ -145,19 +145,13 @@
index: number,
selectedValue: unknown
): Promise<void> {
const { model, partitionConfig: prevConfig } = this.props;
const { partitionConfig: prevConfig } = this.props;

log.debug('handlePartitionSelect', index, selectedValue, prevConfig);

const newPartitions = [...prevConfig.partitions];
newPartitions[index] = selectedValue;

// If it's the last partition changed, we know it's already a valid value, just emit it
if (index === model.partitionColumns.length - 1) {
this.sendUpdate({ partitions: newPartitions, mode: 'partition' });
return;
}

const { keysTable } = this.state;
// Otherwise, we need to get the value from a filtered key table
assertNotNull(keysTable);
Expand All @@ -184,7 +178,7 @@
// Core JSAPI returns undefined for null table values,
// coalesce with null to differentiate null from no selection in the dropdown
// https://github.com/deephaven/deephaven-core/issues/5400
partitions: t.columns.map(column => data.rows[0].get(column) ?? null),
partitions: t.columns.map(column => data.rows[0].get(column)),
mode: 'partition',
};
t.close();
Expand Down Expand Up @@ -247,10 +241,11 @@
this.setState({ partitionFilters });
}

getPartitionFilters(partitionTables: dh.Table[]): dh.FilterCondition[][] {
getPartitionFilters(partitionTables: dh.Table[]): FilterConditionFactory[][] {
const { model, partitionConfig } = this.props;
const { partitions } = partitionConfig;
log.debug('getPartitionFilters', partitionConfig);
console.log('getPartitionFilters', partitionConfig);

Check warning on line 248 in packages/iris-grid/src/IrisGridPartitionSelector.tsx

View workflow job for this annotation

GitHub Actions / unit 

Unexpected console statement

Check warning on line 248 in packages/iris-grid/src/IrisGridPartitionSelector.tsx

View workflow job for this annotation

GitHub Actions / unit 

Unexpected console statement

if (partitions.length !== partitionTables.length) {
throw new Error(
Expand All @@ -259,7 +254,7 @@
}

// The filters are applied in order, so we need to build up the filters for each partition
const partitionFilters: dh.FilterCondition[][] = [];
const partitionFilters: FilterConditionFactory[][] = [];
for (let i = 0; i < partitions.length; i += 1) {
if (i === 0) {
// There's no reason to ever filter the first table
Expand All @@ -270,10 +265,23 @@
const previousColumn = model.partitionColumns[i - 1];
const partitionFilter = [
...previousFilter,
this.tableUtils.makeNullableEqFilter(
previousColumn,
previousPartition
),
(table: dh.Table | dh.TreeTable | null | undefined) => {
if (table == null) {
return null;
}
const filterConditionTest = this.tableUtils.makeNullableEqFilter(
table.findColumn(previousColumn.name),
// previousColumn,
previousPartition
);
console.log(

Check warning on line 277 in packages/iris-grid/src/IrisGridPartitionSelector.tsx

View workflow job for this annotation

GitHub Actions / unit 

Unexpected console statement

Check warning on line 277 in packages/iris-grid/src/IrisGridPartitionSelector.tsx

View workflow job for this annotation

GitHub Actions / unit 

Unexpected console statement
'tester',
previousColumn,
previousPartition,
filterConditionTest
);
return filterConditionTest;
},
];
partitionFilters.push(partitionFilter);
}
Expand All @@ -296,11 +304,13 @@

const { partitions } = partitionConfig;

if (partitionFilters !== null && partitionTables !== null) {
partitionFilters.forEach((filter, index) => {
partitionTables[index].applyFilter(filter as dh.FilterCondition[]);
});
}
// if (partitionFilters !== null && partitionTables !== null) {
// partitionFilters.forEach((filter, index) => {
// partitionTables[index].applyFilter(filter as dh.FilterCondition[]);
// // setting the viewport 0,50
// });
// }
console.log(partitionTables, 'partition table');

Check warning on line 313 in packages/iris-grid/src/IrisGridPartitionSelector.tsx

View workflow job for this annotation

GitHub Actions / unit 

Unexpected console statement

Check warning on line 313 in packages/iris-grid/src/IrisGridPartitionSelector.tsx

View workflow job for this annotation

GitHub Actions / unit 

Unexpected console statement
const partitionSelectors = model.partitionColumns.map((column, index) => (
<div key={`selector-${column.name}`} className="column-selector">
{partitionTables?.[index] && (
Expand All @@ -318,6 +328,7 @@
isDisabled={
(index > 0 && partitionConfig.mode !== 'partition') || isLoading
}
additionalFilterFactories={partitionFilters?.[index]}
/>
)}
{model.partitionColumns.length - 1 === index || (
Expand Down
4 changes: 3 additions & 1 deletion packages/jsapi-components/src/spectrum/PickerProps.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {
SpectrumPickerProps,
} from '@deephaven/components';
import { dh as DhType } from '@deephaven/jsapi-types';
import { Settings } from '@deephaven/jsapi-utils';
import { FilterConditionFactory, Settings } from '@deephaven/jsapi-utils';

export type PickerWithTableProps<TProps> = Omit<
PickerPropsT<TProps>,
Expand All @@ -20,6 +20,8 @@ export type PickerWithTableProps<TProps> = Omit<
iconColumn?: string;

settings?: Settings;

additionalFilterFactories?: FilterConditionFactory[];
};

export type PickerProps = PickerWithTableProps<
Expand Down
13 changes: 12 additions & 1 deletion packages/jsapi-components/src/spectrum/utils/usePickerProps.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
| 'labelColumn'
| 'iconColumn'
| 'settings'
| 'additionalFilterFactories'
| 'onChange'
| 'onSelectionChange'
>;
Expand All @@ -53,6 +54,7 @@
keyColumn: keyColumnName,
labelColumn: labelColumnName,
iconColumn: iconColumnName,
additionalFilterFactories,
settings,
onChange,
onSelectionChange,
Expand All @@ -74,9 +76,18 @@
// applied in `useSearchableViewportData`.)
const { data: tableCopy } = usePromiseFactory(
TableUtils.copyTableAndApplyFilters,
[tableSource]
[tableSource, ...(additionalFilterFactories ?? [])]
);

useEffect(() => {
console.log(

Check warning on line 83 in packages/jsapi-components/src/spectrum/utils/usePickerProps.ts

View workflow job for this annotation

GitHub Actions / unit 

Unexpected console statement

Check warning on line 83 in packages/jsapi-components/src/spectrum/utils/usePickerProps.ts

View workflow job for this annotation

GitHub Actions / unit 

Unexpected console statement
'[TESTING] additionalFilters',
additionalFilterFactories,
tableCopy?.columns,
tableCopy?.size
);
}, [additionalFilterFactories, tableCopy]);

const keyColumn = useMemo(
() =>
tableCopy == null ? null : getItemKeyColumn(tableCopy, keyColumnName),
Expand Down
Loading