MILAB-3726: advanced filter component #1127

erohinaelena · 2025-09-03T17:46:08Z

No description provided.

changeset-bot · 2025-09-03T17:46:12Z

⚠️ No Changeset found

Latest commit: 39c398c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

notion-workspace · 2025-09-03T17:46:13Z

2 [sdk/ui-vue] Create advanced filter form component

gemini-code-assist

Summary of Changes

Hello @erohinaelena, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request delivers a new advanced filtering system, allowing users to construct sophisticated data queries through a user-friendly interface. The changes encompass the creation of a dedicated advanced filter component, along with necessary enhancements to foundational UI components to support its dynamic behavior and improved visual presentation. This provides a robust and extensible solution for complex data filtering needs across the application.

Highlights

New Advanced Filter Component: Introduced a new PlAdvancedFilter component, providing a flexible and powerful user interface for building complex data filters with support for various conditions and logical operators.
Enhanced UI Components: Updated existing PlDropdown and PlElementList components to include new props and slots, enabling more granular control over error message display and item styling/content placement, which are crucial for the advanced filter's functionality.
Expanded Filter Capabilities: Extended the FilterUi model with new filter types, including patternMatchesRegularExpression, patternFuzzyContainSubsequence, numberEquals, and numberNotEquals, significantly broadening the range of filtering operations available.
Integration into UI Examples: Integrated the new PlAdvancedFilter component into the UI examples application, adding a dedicated page (PlAdvancedFilterPage.vue) to demonstrate its usage, including drag-and-drop functionality and various filter state examples.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request introduces a new advanced filter component, along with an example page to demonstrate its usage. Several existing components like PlDropdown and PlElementList have been extended to support the new filter's requirements. The changes are well-structured, with clear separation of concerns into different files for types, constants, and utility functions. My review focuses on a few minor bugs, mostly copy-paste errors in the new code, and a potential TypeScript type issue. I've also suggested a small readability improvement in one of the Vue templates.

gemini-code-assist · 2025-09-03T17:48:15Z

sdk/model/src/components/PlAnnotations/filters_ui.ts

+      type: {
+        label: 'Predicate',
+        fieldType: 'FilterUiType',
+        defaultValue: () => 'numberEquals',


There seems to be a copy-paste error. The defaultValue for the type field in the numberNotEquals filter metadata is set to 'numberEquals'. It should be 'numberNotEquals' to match the filter type.

Suggested change

defaultValue: () => 'numberEquals',

defaultValue: () => 'numberNotEquals',

gemini-code-assist · 2025-09-03T17:48:15Z

sdk/ui-vue/src/components/PlAdvancedFilter/constants.ts

+  numberEquals: { type: 'numberEquals', sourceId: '', reference: undefined },
+  numberNotEquals: { type: 'numberNotEquals', sourceId: '', reference: undefined },
+  InSet: { type: 'InSet', sourceId: '', reference: [] },
+  NotInSet: { type: 'InSet', sourceId: '', reference: [] },


There seems to be a copy-paste error. The default filter for NotInSet has its type set to 'InSet'. It should be 'NotInSet'.

Suggested change

NotInSet: { type: 'InSet', sourceId: '', reference: [] },

NotInSet: { type: 'NotInSet', sourceId: '', reference: [] },

gemini-code-assist · 2025-09-03T17:48:15Z

sdk/ui-vue/src/components/PlAdvancedFilter/utils.ts

+function toOuterFilterGroup(m: Group): FilterUi {
+  const res: FilterUi = {
+    type: m.operand,
+    filters: m.filters.map(toOuterFilter).filter((v) => v !== null),


The .filter((v) => v !== null) call doesn't inform TypeScript that null values are removed from the array. This can lead to a type mismatch, as the resulting array is of type (FilterUi | null)[] while FilterUi[] is expected. Using a type guard (v): v is FilterUi => v !== null will resolve this typing issue.

Suggested change

filters: m.filters.map(toOuterFilter).filter((v) => v !== null),

filters: m.filters.map(toOuterFilter).filter((v): v is FilterUi => v !== null),

gemini-code-assist · 2025-09-03T17:48:15Z

etc/blocks/ui-examples/ui/src/pages/PlAdvancedFilterPage.vue

+  overflow: auto;
+}
+.columnChip {
+  background: '#fff';


The value for background is a string '#fff', which is not valid CSS. It should be the hex color code directly without quotes.

background: #fff;

gemini-code-assist · 2025-09-03T17:48:15Z

sdk/ui-vue/src/components/PlAdvancedFilter/PlAdvancedFilter.vue

+        >
+          <PlCheckbox v-model="item.not">NOT</PlCheckbox>
+          <FilterComponent
+            v-for="filterIdx of new Array(item.filters.length).fill(0).map((_v, idx)=> idx)"


This v-for loop is difficult to read and unidiomatic. You can iterate directly over item.filters to make the code more readable and maintainable.

v-for="(filter, filterIdx) in item.filters"

AStaroverov · 2025-09-04T13:01:31Z

etc/blocks/ui-examples/ui/src/pages/PlAdvancedFilterPage.vue

+          column: 'someColumn' as SUniversalPColumnId,
+        },
+        {
+          type: 'isNotNA' as const,


let's create objet with types for reusing it

const FilterUiTypes = { IsNa: 'isNa' ... } as const

AStaroverov · 2025-09-04T13:02:18Z

etc/blocks/ui-examples/ui/src/pages/PlAdvancedFilterPage.vue

+];
+
+watch(() => filtersModel.value, (m) => {
+  console.log('m', m);


AStaroverov · 2025-09-04T13:03:54Z

etc/blocks/ui-examples/ui/src/pages/PlAdvancedFilterPage.vue

+      <PlDropdown v-model="selectedSavedFilters" :options="filterStatesOptions" label="Examples" :style="{width: '300px'}" />
+    </div>
+    <div :class="$style.block">
+      <div v-if="dndMode" class="d-flex flex-column gap-16" :class="$style.leftColumn">


please don't use global styles

AStaroverov · 2025-09-04T13:06:26Z

lib/ui/uikit/src/components/PlDropdown/PlDropdown.vue

+    /**
+     * If false there is only red border with non-empty error field, without a message
+     */
+    showErrorMessage?: boolean;


I don't think that you should add flag that control another. MB errorStatus: true|status: 'error'|'idle' will be better?

AStaroverov · 2025-09-04T13:08:41Z

lib/ui/uikit/src/components/PlElementList/PlElementList.vue

        <template v-if="slots['item-content']" #content="{ item, index }">
          <slot :index="index" :item="item" name="item-content" />
        </template>
+        <template v-if="slots['item-after']" #after="{ item, index }">


why content part not enough?

AStaroverov · 2025-09-04T13:09:06Z

lib/ui/uikit/src/components/PlElementList/PlElementList.vue

+    return props.itemClassBody(item, index);
+  }
+  return props.itemClassBody ?? null;
+};


3 equal functions

AStaroverov · 2025-09-04T13:10:53Z

lib/ui/uikit/src/components/PlElementList/PlElementListItem.vue

    <div
      v-if="hasContentSlot && props.isExpanded"
-      :class="[$style.body, { [$style.disabled]: props.isToggled }]"
+      :class="[$style.body, bodyClass, { [$style.disabled]: props.isToggled }]"


contentClass it's more consistent with slot['content']

AStaroverov · 2025-09-04T13:13:11Z

sdk/model/src/components/PlAnnotations/filters_ui.ts

 } satisfies CreateFilterUiMetadataMap<FilterUiType>;

+// exist in PlAdvancedFilter
+const ignoredFilters = new Set<FilterUiType>(['numberEquals', 'numberNotEquals', 'patternFuzzyContainSubsequence', 'patternMatchesRegularExpression']);


let's create white list instead black list. If somebody add another filter it don't broke this component

AStaroverov · 2025-09-04T13:13:29Z

sdk/model/src/components/PlAnnotations/filters_ui.ts

+    || ui.type === 'numberNotEquals'
+    || ui.type === 'patternFuzzyContainSubsequence'
+    || ui.type === 'patternMatchesRegularExpression') {
+    return {


AStaroverov · 2025-09-04T13:14:43Z

sdk/ui-vue/src/components/PlAdvancedFilter/Filter.vue

+
+<style lang="scss" module>
+.filter_wrapper {
+  --errorColor: #FF5C5C;


error-color, let's be consistent with another css vars

AStaroverov · 2025-09-04T13:15:41Z

sdk/ui-vue/src/components/PlAdvancedFilter/Filter.vue

+  color: var(--txt-01);
+  text-overflow: ellipsis;
+  white-space: nowrap;
+  font-family: Manrope;


should we remove it?

AStaroverov · 2025-09-04T13:16:09Z

sdk/ui-vue/src/components/PlAdvancedFilter/Filter.vue

+  line-height: 20px;
+}
+
+.column_chip {


kek, why snake_case?

AStaroverov · 2025-09-04T13:19:31Z

sdk/ui-vue/src/components/PlAdvancedFilter/Filter.vue

+    <div v-else :class="$style.top" >
+      <PlDropdown
+        v-model="model.sourceId"
+        :error="currentError ? ' ' : undefined"


why we cast error to space?

AStaroverov · 2025-09-04T13:22:49Z

sdk/ui-vue/src/components/PlAdvancedFilter/Filter.vue

+          || model.type === 'lessThan'
+          || model.type === 'lessThanOrEqual'
+          || model.type === 'greaterThan'
+          || model.type === 'greaterThanOrEqual'


let's incapsulate it predicate

AStaroverov · 2025-09-04T13:25:55Z

sdk/ui-vue/src/components/PlAdvancedFilter/OperandButton.vue

+  justify-content: center;
+}
+.operand {
+    border-radius: 120px;


let's change it to 100%

AStaroverov · 2025-09-04T13:30:43Z

sdk/ui-vue/src/components/PlAdvancedFilter/PlAdvancedFilter.vue

+      :get-item-key="(group) => group.id"
+
+      :item-class="$style.filterGroup"
+      :item-class-body="$style.filterGroup__body"


filterGroupBody

AStaroverov · 2025-09-04T13:31:40Z

sdk/ui-vue/src/components/PlAdvancedFilter/PlAdvancedFilter.vue

+
+      :is-expanded="(group) => expanded[group.id] ?? false"
+      :is-expandable="() => true"
+      :is-removable="() => true"


Are you sure that you need predicates like this () => true?

AStaroverov · 2025-09-04T13:34:53Z

sdk/ui-vue/src/components/PlAdvancedFilter/Filter.vue

+  sourceInfoBySourceId: SourceOptionsInfo;
+  uniqueValuesBySourceId: UniqueValuesInfo;
+  searchOptions: (id: string, str: string) => Promise<ListOptionBase<string | number>[]>;
+  searchModel: (id: string, str: string) => Promise<ListOptionBase<string | number>>;


let's allow this predicate return sync result

AStaroverov · 2025-09-04T13:36:26Z

sdk/ui-vue/src/components/PlAdvancedFilter/PlAdvancedFilter.vue

+      :disableToggling="true"
+      :disablePinning="true"
+
+      @expand="(group) => {expanded[group.id] = !expanded[group.id]}"


PlElementList don't have expand event, only props on-expand

AStaroverov · 2025-09-04T13:36:48Z

sdk/ui-vue/src/components/PlAdvancedFilter/Filter.vue

+
+const model = defineModel<Filter>({ required: true });
+
+defineEmits<{


can we use only props and avoid events?

AStaroverov · 2025-09-04T13:39:57Z

sdk/ui-vue/src/components/PlAdvancedFilter/types.ts

+
+// Not supported: topN, bottomN, lessThanColumn, lessThanColumnOrEqual
+// or, and, not - in groups
+export type SupportedFilterTypes = Exclude<FilterUi['type'], 'topN' | 'bottomN' | 'lessThanColumn' | 'lessThanColumnOrEqual' | 'or' | 'and' | 'not' | undefined>;


AStaroverov · 2025-09-04T13:46:56Z

sdk/ui-vue/src/components/PlAdvancedFilter/types.ts

+
+export type Operand = 'or' | 'and';
+
+interface FilterBase {


let's rewrite with usage FilterUI

AStaroverov · 2025-09-04T14:09:12Z

sdk/ui-vue/src/components/PlAdvancedFilter/Filter.vue

+  try {
+    new RegExp(model.value.reference);
+    return false;
+  } catch (_err) {


you can avoid _err just use } catch {

AStaroverov · 2025-10-03T11:12:12Z

etc/blocks/ui-examples/ui/src/pages/PlAdvancedFilterPage.vue

+  return { value: modelStr, label: `Label ${modelStr}` };
+}
+
+const errorState = {


can you highlight where exactly mistake in this state?

AStaroverov · 2025-10-03T11:15:55Z

sdk/ui-vue/src/components/PlAdvancedFilter/constants.ts

+const LOCAL_FILTER_TYPES: FilterType[] = ['inSet', 'notInSet'];
+export const ALL_FILTER_TYPES = new Set<FilterType>([...SUPPORTED_FILTER_TYPES, ...LOCAL_FILTER_TYPES]);
+
+export const LOCAL_FILTERS_METADATA: Record<'inSet' | 'notInSet', { label: string; supportedFor: (spec: NormalizedSpecData) => boolean }> = {


what mean "local_filters"?

"inSet" and "notInSet" are made from combination of 'Or' and "Equals", but it must look like a regular filter in the interface; so I don't add them to UiFilters, just make "local" ones only for component

AStaroverov · 2025-10-03T11:17:48Z

sdk/ui-vue/src/components/PlAdvancedFilter/Filter.vue

+  last: boolean;
+  sourceInfoBySourceId: SourceOptionsInfo;
+  uniqueValuesBySourceId: UniqueValuesInfo;
+  searchOptionsFn?: (id: string, str: string) => (Promise<ListOptionBase<string | number>[]>) | ((id: string, str: string) => ListOptionBase<string | number>[]);


getSearchOptions, getSearchModel

AStaroverov · 2025-10-03T11:19:04Z

sdk/ui-vue/src/components/PlAdvancedFilter/Filter.vue

+  const searchFn = props.searchModelFn;
+  return searchFn ? searchFn(id, v) as Promise<ListOptionBase<string>> : Promise.resolve({ label: '', value: '' });
+}
+async function optionsSearch(id: string, str: string): Promise<ListOptionBase<string>[]> {


every function must start from verb

AStaroverov · 2025-10-03T11:27:57Z

sdk/ui-vue/src/components/PlAdvancedFilter/Filter.vue

+  onChangeOperand: (op: Operand) => void;
+}>();
+
+const model = defineModel<Filter>({ required: true });


let's rename to filter , this is a more meaningful name

AStaroverov · 2025-10-03T11:31:47Z

lib/ui/uikit/src/components/PlNumberField/PlNumberField.vue

  /** Additional validity check for input value that must return an error text if failed */
  validate?: (v: number) => string | undefined;
+  /** Makes some of corners not rounded */
+  position?: 'top' | 'bottom' | 'left' | 'right' | 'top-left' | 'top-right' | 'bottom-left' | 'bottom-right' | 'middle';


mb better to rename it to group-position? when I see position I think about how we should position this element

edge-position? position-in-combination? border-shape-by-position?

AStaroverov · 2025-10-03T12:51:37Z

lib/ui/uikit/src/utils/DoubleContour.vue


 <template>
-  <div class="double-contour">
+  <div class="double-contour" :class="{ [position]: true }">


can be simplified :class="props.position"

AStaroverov · 2025-10-03T12:52:17Z

lib/ui/uikit/src/utils/DoubleContour.vue

    </div>
  </div>
 </template>
+<style>


lets use <style module>

AStaroverov · 2025-10-03T12:53:20Z

sdk/model/src/components/PlAnnotations/filters_ui.ts

 } satisfies CreateFilterUiMetadataMap<FilterUiType>;

+// exist in PlAdvancedFilter
+const filtersInPlAnnotation = new Set<FilterUiType>([


this should be defined inside PlAdvancedFilter

AStaroverov · 2025-10-03T12:54:42Z

sdk/model/src/components/PlAnnotations/filters_ui.ts

  }

-  return Object.entries(filterUiMetadata).filter(([_, metadata]) => metadata.supportedFor(columnSpec)).map(([type, metadata]) => ({
+  return Object.entries(filterUiMetadata).filter(([filterName, metadata]) => !filtersInPlAnnotation.has(filterName as FilterUiType)


this code shouldn't specific for some component

AStaroverov · 2025-10-03T12:55:20Z

sdk/model/src/components/PlAnnotations/filters_ui.ts

+    || ui.type === 'numberNotEquals'
+    || ui.type === 'patternFuzzyContainSubsequence'
+    || ui.type === 'patternMatchesRegularExpression') {
+    throw new Error(`Filter "${ui.type}" is not supported in PlAnnotation`);


this code haven't to exist here

AStaroverov · 2025-10-03T13:14:22Z

sdk/ui-vue/src/components/PlAdvancedFilter/Filter.vue

+
+const props = defineProps<{
+  operand: Operand;
+  sourceIds: string[];


why you split sourceIds and sourceInfoBySourceId? Usually we use array of items, that will be equal SourceOptionsInfo[]

AStaroverov · 2025-10-03T13:42:53Z

sdk/ui-vue/src/components/PlAdvancedFilter/Filter.vue

+const wildcardOptions = computed(() => {
+  if (model.value.type === 'patternFuzzyContainSubsequence') {
+    const alphabet = currentSpec.value ? readDomain(currentSpec.value, Domain.Alphabet) ?? readAnnotation(currentSpec.value, Annotation.Alphabet) : null;
+    if (alphabet === 'nucleotide') {


this is super specific code, it should be implemented outside this component

gemini-code-assist bot reviewed Sep 3, 2025

View reviewed changes

AStaroverov reviewed Sep 4, 2025

View reviewed changes

sdk/ui-vue/src/components/PlAdvancedFilter/Filter.vue Outdated

line-height: 20px;

}

.column_chip {

Copy link

Contributor

AStaroverov Sep 4, 2025

Choose a reason for hiding this comment

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

kek, why snake_case?

AStaroverov reviewed Sep 4, 2025

View reviewed changes

AStaroverov reviewed Oct 3, 2025

View reviewed changes

lib/ui/uikit/src/utils/DoubleContour.vue

</div>

</div>

</template>

<style>

Copy link

Contributor

AStaroverov Oct 3, 2025

Choose a reason for hiding this comment

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

lets use <style module>

AStaroverov reviewed Oct 3, 2025

View reviewed changes

erohinaelena added 7 commits October 27, 2025 17:43

pl advanced filter

ceccbf6

use filtersUi

fbf3c6d

add 'position' for using dropdowns in 'stacks'

1785beb

review fixes

b1a3410

rename model -> filter

36d3f36

review fixes

80d5c34

review fixes

39c398c

erohinaelena force-pushed the erohinaelena/MILAB-3726 branch from e9b044b to 39c398c Compare October 27, 2025 15:53

	defaultValue: () => 'numberEquals',
	defaultValue: () => 'numberNotEquals',

	NotInSet: { type: 'InSet', sourceId: '', reference: [] },
	NotInSet: { type: 'NotInSet', sourceId: '', reference: [] },

	filters: m.filters.map(toOuterFilter).filter((v) => v !== null),
	filters: m.filters.map(toOuterFilter).filter((v): v is FilterUi => v !== null),


		const model = defineModel<Filter>({ required: true });

		defineEmits<{

Uh oh!

MILAB-3726: advanced filter component #1127

Are you sure you want to change the base?

MILAB-3726: advanced filter component #1127

Uh oh!

Conversation

erohinaelena commented Sep 3, 2025

Uh oh!

changeset-bot bot commented Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ No Changeset found

Uh oh!

notion-workspace bot commented Sep 3, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

AStaroverov Sep 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Sep 3, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AStaroverov Sep 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

changeset-bot bot commented Sep 3, 2025 •

edited

Loading

AStaroverov Sep 4, 2025 •

edited

Loading

AStaroverov Sep 4, 2025 •

edited

Loading