Skip to content

Conversation

@HarshMN2345
Copy link
Member

@HarshMN2345 HarshMN2345 commented Jan 12, 2026

What does this PR do?

image image image image

for ipad
image

for mobile
image

Test Plan

(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)

Related PRs and Issues

(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)

Have you read the Contributing Guidelines on issues?

(Write your answer here.)

Summary by CodeRabbit

  • New Features

    • Interactive tag parts with per-segment menus to add/remove filters and dismiss placeholders
    • Dynamic placeholders alongside active tags to apply or hide suggested filters
  • UI/UX Updates

    • Improved header layout to better position tags, filters, and search
    • Filters control made more accessible (icon-first, aria label) and visual tweaks to button styling

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 12, 2026

Walkthrough

Reworks the tag/filter UI across three components. parsedTagList.svelte now composes tags with CompoundTagRoot/CompoundTagChild, parses tag strings into operator and bolded-property parts, renders per-part ActionMenu-based menus to add/modify filters, computes FilterData from columns, exposes placeholders with dismiss/apply actions, and adds handlers to remove individual or all parsed tags. quickFilters.svelte replaces a secondary-styled Button with an ariaLabel-driven Button and moves the Icon into the button body. responsiveContainerHeader.svelte inlines ParsedTagList into the small-viewport flow, adjusts header alignment and stacks for large viewports, removes the QuickFilters import, and changes the Filters button to text style.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Title check ⚠️ Warning The title claims to add new filters for 'sites' and 'functions', but the actual changes refactor the tag filtering UI components (ParsedTagList, QuickFilters, ResponsiveContainerHeader) without adding new filter types. Revise the title to reflect the actual changes, such as: 'refactor: improve parsed tag filtering UI with compound tag composition' or 'feat: enhance tag filtering interface with interactive menu actions'.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 999f5a0 and 26e1b61.

📒 Files selected for processing (1)
  • src/lib/layout/responsiveContainerHeader.svelte
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx,svelte}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx,js,jsx,svelte}: Import reusable modules from the src/lib directory using the $lib alias
Use minimal comments in code; reserve comments for TODOs or complex logic explanations
Use $lib, $routes, and $themes aliases instead of relative paths for module imports

Files:

  • src/lib/layout/responsiveContainerHeader.svelte
**/*.{ts,tsx,js,jsx,svelte,json}

📄 CodeRabbit inference engine (AGENTS.md)

Use 4 spaces for indentation, single quotes, 100 character line width, and no trailing commas per Prettier configuration

Files:

  • src/lib/layout/responsiveContainerHeader.svelte
**/*.svelte

📄 CodeRabbit inference engine (AGENTS.md)

Use Svelte 5 + SvelteKit 2 syntax with TypeScript for component development

Files:

  • src/lib/layout/responsiveContainerHeader.svelte
🧠 Learnings (1)
📚 Learning: 2025-11-25T03:15:27.539Z
Learnt from: CR
Repo: appwrite/console PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:15:27.539Z
Learning: Applies to **/*.svelte : Use Svelte 5 + SvelteKit 2 syntax with TypeScript for component development

Applied to files:

  • src/lib/layout/responsiveContainerHeader.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: build
  • GitHub Check: e2e
🔇 Additional comments (4)
src/lib/layout/responsiveContainerHeader.svelte (4)

102-104: LGTM!

The horizontal overflow wrapper is a good UX pattern for mobile viewports, allowing users to scroll through tags when they exceed the available width.


107-125: LGTM!

The layout restructuring properly handles flex behavior with min-width: 0 to prevent overflow and wrap="wrap" to allow multi-line tag display. The nested stack structure cleanly separates the search/tags area from the right-aligned controls.


126-137: LGTM!

The right-aligned stack correctly uses align-self: flex-start to keep controls anchored at the top when tags wrap to multiple lines, maintaining a clean visual hierarchy.


178-181: Verify intentional button styling inconsistency.

The filtersButton now uses text styling while searchButton (line 163) and settingsButton (line 172) retain secondary styling. In the small viewport layout (lines 87-95), all three buttons render together, which could create visual inconsistency.

If this is intentional per the new design, consider adding a brief comment explaining the different treatment of the Filters button.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In @src/lib/components/filters/parsedTagList.svelte:
- Around line 172-191: The tag menu handlers call addFilterAndApply with a
hardcoded empty string for analyticsSource instead of using the component's
analyticsSource prop; update all addFilterAndApply invocations in the
tag/menu-related handlers (the calls currently passing '' — the ones near the
other tag option branches and the later tag removal branch) to pass the
analyticsSource prop value so they match the QuickFilters usage that already
passes analyticsSource to addFilterAndApply.
🧹 Nitpick comments (3)
src/lib/layout/responsiveContainerHeader.svelte (1)

52-59: Suspicious self-assignment of filterCols.

Line 58 assigns filterCols to itself. Since filterCols is a $derived value (line 48-50), this assignment has no effect—derived values are read-only and automatically recompute when their dependencies change. This line can be removed.

🔧 Suggested fix
     afterNavigate((p) => {
         if (!hasFilters) return;
         const paramQueries = p.to.url.searchParams.get('query');
         const localQueries = queryParamToMap(paramQueries || '[]');
         const localTags = Array.from(localQueries.keys());
         setFilters(localTags, filterCols, $columns);
-        filterCols = filterCols;
     });
src/lib/components/filters/parsedTagList.svelte (2)

87-93: Unnecessary double type cast.

The $columns as unknown as Column[] pattern (also at lines 97-100) suggests a type mismatch. Since columns is typed as Writable<Column[]>, $columns should already be Column[]. If the types don't match, consider fixing the root cause rather than using unknown casts.

🔧 Suggested simplification
     function getFilterFor(title: string): FilterData | null {
         if (!columns) return null;
-        const col = ($columns as unknown as Column[]).find((c) => c.title === title);
+        const col = $columns.find((c) => c.title === title);
         if (!col) return null;
         const filter = buildFilterCol(col);
         return filter ?? null;
     }

Similarly for lines 97-100:

     let availableFilters = $derived(
-        ($columns as unknown as Column[] | undefined)?.length
-            ? (($columns as unknown as Column[])
+        $columns?.length
+            ? ($columns
                   .map((c) => (c.filter !== false ? buildFilterCol(c) : null))
                   .filter((f) => f && f.options) as FilterData[])
             : []
     );

243-249: Array mutation with workaround—prefer immutable update.

Using push() on a $state array followed by a manual version counter increment (line 248) is a workaround pattern. In Svelte 5, prefer immutable updates for clearer reactivity:

🔧 Suggested fix
                             on:click={(e) => {
                                 e.stopPropagation();
-                                if (!hiddenPlaceholders.includes(filter.title)) {
-                                    hiddenPlaceholders.push(filter.title);
-                                }
-                                placeholderVersion++;
+                                if (!hiddenPlaceholders.includes(filter.title)) {
+                                    hiddenPlaceholders = [...hiddenPlaceholders, filter.title];
+                                }
                             }}>

This also allows removing the placeholderVersion state variable (line 110) and the {#key} wrapper (lines 232, 279) since Svelte will properly track the array reference change.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between eef3f68 and 8b1d5eb.

📒 Files selected for processing (3)
  • src/lib/components/filters/parsedTagList.svelte
  • src/lib/components/filters/quickFilters.svelte
  • src/lib/layout/responsiveContainerHeader.svelte
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx,svelte}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx,js,jsx,svelte}: Import reusable modules from the src/lib directory using the $lib alias
Use minimal comments in code; reserve comments for TODOs or complex logic explanations
Use $lib, $routes, and $themes aliases instead of relative paths for module imports

Files:

  • src/lib/layout/responsiveContainerHeader.svelte
  • src/lib/components/filters/quickFilters.svelte
  • src/lib/components/filters/parsedTagList.svelte
**/*.{ts,tsx,js,jsx,svelte,json}

📄 CodeRabbit inference engine (AGENTS.md)

Use 4 spaces for indentation, single quotes, 100 character line width, and no trailing commas per Prettier configuration

Files:

  • src/lib/layout/responsiveContainerHeader.svelte
  • src/lib/components/filters/quickFilters.svelte
  • src/lib/components/filters/parsedTagList.svelte
**/*.svelte

📄 CodeRabbit inference engine (AGENTS.md)

Use Svelte 5 + SvelteKit 2 syntax with TypeScript for component development

Files:

  • src/lib/layout/responsiveContainerHeader.svelte
  • src/lib/components/filters/quickFilters.svelte
  • src/lib/components/filters/parsedTagList.svelte
src/lib/components/**/*.svelte

📄 CodeRabbit inference engine (AGENTS.md)

Use PascalCase for component file names and place them in src/lib/components/[feature]/ directory structure

Files:

  • src/lib/components/filters/quickFilters.svelte
  • src/lib/components/filters/parsedTagList.svelte
🧠 Learnings (1)
📚 Learning: 2025-11-25T03:15:27.539Z
Learnt from: CR
Repo: appwrite/console PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:15:27.539Z
Learning: Applies to **/*.svelte : Use Svelte 5 + SvelteKit 2 syntax with TypeScript for component development

Applied to files:

  • src/lib/layout/responsiveContainerHeader.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: build
  • GitHub Check: e2e
🔇 Additional comments (5)
src/lib/components/filters/quickFilters.svelte (1)

25-31: LGTM! Good accessibility improvement.

The addition of ariaLabel="Filters" improves screen reader support for the icon-only button. The badge logic for displaying the active filter count is correctly preserved.

src/lib/layout/responsiveContainerHeader.svelte (1)

104-135: Layout restructure looks good.

The consolidated header layout correctly:

  • Uses flex-start alignment to handle variable tag heights
  • Applies min-width: 0 to prevent flex overflow issues
  • Wraps the tag list appropriately for responsive behavior
  • Separates left (search + filters) and right (display settings + children) clusters cleanly
src/lib/components/filters/parsedTagList.svelte (3)

214-220: Potentially over-broad tag removal logic.

The filter t.tag.includes(tag.tag.split(' ')[0]) matches any tag containing the first word of the current tag. This could unintentionally remove tags for different filters if their names share common prefixes (e.g., "Status" matching "StatusCode" tags).

Consider whether this should use a more precise match, such as checking the property name extracted via firstBoldText:

🔧 Potential fix
                         on:click={() => {
-                            const t = $tags.filter((t) =>
-                                t.tag.includes(tag.tag.split(' ')[0])
-                            );
+                            const property = firstBoldText(tag.tag);
+                            const t = $tags.filter((t) =>
+                                property && firstBoldText(t.tag) === property
+                            );
                             t.forEach((t) => (t ? queries.removeFilter(t) : null));

124-294: Overall structure looks good.

The composition using CompoundTagRoot/CompoundTagChild with nested menus provides a clean interactive filter UI. The layout correctly handles wrapping and the conditional rendering of placeholders and QuickFilters is well-structured.


172-180: No changes needed - conditional rendering already guards against undefined columns.

Lines 172-180 and 182-190 are protected by the {#if filter} block, which only renders when getFilterFor() successfully returns a filter—which requires columns to be defined (line 88 checks if (!columns) return null).

Similarly, lines 259-267 are protected by {#if placeholders?.length}, and placeholders is derived from availableFilters, which evaluates to an empty array when columns is undefined (line 97 uses optional chaining ?.length).

The code already safely handles the case where columns is undefined without needing additional guards.

Comment on lines +172 to +191
addFilterAndApply(
filter.id,
filter.title,
filter.operator,
null,
next,
$columns,
''
);
} else {
addFilterAndApply(
filter.id,
filter.title,
filter.operator,
option.value,
[],
$columns,
''
);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

analyticsSource prop not used in tag menu handlers.

The addFilterAndApply calls at lines 179, 189, and 266 pass an empty string '' for analyticsSource, while the component receives analyticsSource as a prop. This means filter changes made via tag menus won't be tracked, unlike those from QuickFilters (line 282) which correctly uses the prop.

🔧 Suggested fix
                                                 addFilterAndApply(
                                                     filter.id,
                                                     filter.title,
                                                     filter.operator,
                                                     null,
                                                     next,
                                                     $columns,
-                                                    ''
+                                                    analyticsSource
                                                 );

Apply the same change to lines 189 and 266.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
addFilterAndApply(
filter.id,
filter.title,
filter.operator,
null,
next,
$columns,
''
);
} else {
addFilterAndApply(
filter.id,
filter.title,
filter.operator,
option.value,
[],
$columns,
''
);
}
addFilterAndApply(
filter.id,
filter.title,
filter.operator,
null,
next,
$columns,
analyticsSource
);
} else {
addFilterAndApply(
filter.id,
filter.title,
filter.operator,
option.value,
[],
$columns,
analyticsSource
);
}
🤖 Prompt for AI Agents
In @src/lib/components/filters/parsedTagList.svelte around lines 172 - 191, The
tag menu handlers call addFilterAndApply with a hardcoded empty string for
analyticsSource instead of using the component's analyticsSource prop; update
all addFilterAndApply invocations in the tag/menu-related handlers (the calls
currently passing '' — the ones near the other tag option branches and the later
tag removal branch) to pass the analyticsSource prop value so they match the
QuickFilters usage that already passes analyticsSource to addFilterAndApply.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 Fix all issues with AI agents
In @src/lib/components/filters/parsedTagList.svelte:
- Around line 172-190: The calls to addFilterAndApply in parsedTagList.svelte
are passing empty strings for the analyticsSource argument, which drops the
component's analyticsSource prop; update both call sites (the branch that passes
null for value/next and the branch that passes option.value/[]) to forward the
component's analyticsSource prop instead of '' so analyticsSource is sent
through addFilterAndApply.
- Around line 259-267: The placeholder filter action call to addFilterAndApply
is missing the analyticsSource argument; update the call in parsedTagList.svelte
so addFilterAndApply(filter.id, filter.title, filter.operator, filter?.array ?
null : option.value, filter?.array ? [option.value] : [], $columns, '',
analyticsSource) includes the analyticsSource parameter (or the appropriate
variable name used elsewhere) to match other calls to addFilterAndApply.
- Around line 243-249: The click handler mutates hiddenPlaceholders with .push()
and manually increments placeholderVersion which can break Svelte reactivity;
replace the mutation with an immutable assignment like setting
hiddenPlaceholders = [...hiddenPlaceholders, filter.title] (only if filter.title
not already included) and remove the placeholderVersion++; after confirming
immutable updates work you can also remove placeholderVersion state and the
{#key placeholderVersion} wrapper (ensure you update the on:click handler
references and any code that reads hiddenPlaceholders accordingly).

In @src/lib/layout/responsiveContainerHeader.svelte:
- Around line 113-121: ParsedTagList is being rendered unconditionally inside
the Layout.Stack for the small-viewport branch which can show filter UI even
when hasFilters is false; update the rendering to conditionally render
ParsedTagList only when hasFilters && $columns?.length (same condition used
elsewhere) so wrap the Layout.Stack/ParsedTagList block with that conditional
check to match the intended behavior.
🧹 Nitpick comments (4)
src/lib/components/filters/parsedTagList.svelte (4)

19-19: Use $lib alias for consistency.

Per coding guidelines, use the $lib alias instead of relative paths for module imports.

Proposed fix
-    import Menu from '$lib/components/menu/menu.svelte';
+    import { Menu } from '$lib/components';

Note: Verify the actual export path in the components index file.


87-93: Consider simplifying type handling.

The double cast ($columns as unknown as Column[]) appears multiple times and suggests a type mismatch. Since $columns should already be Column[] | undefined when dereferencing a Writable<Column[]>, consider adding a null check instead:

Proposed fix
     function getFilterFor(title: string): FilterData | null {
         if (!columns) return null;
-        const col = ($columns as unknown as Column[]).find((c) => c.title === title);
+        const col = $columns?.find((c) => c.title === title);
         if (!col) return null;
         const filter = buildFilterCol(col);
         return filter ?? null;
     }

214-220: Fragile tag matching logic.

The removal logic uses tag.tag.split(' ')[0] to find related tags, which could accidentally match unrelated tags if the first word is common (e.g., "is", "the"). Consider using a more robust approach like matching by the filter ID or the bold property name:

Proposed fix using property extraction
                         on:click={() => {
-                            const t = $tags.filter((t) =>
-                                t.tag.includes(tag.tag.split(' ')[0])
-                            );
+                            const property = firstBoldText(tag.tag);
+                            const t = property
+                                ? $tags.filter((t) => firstBoldText(t.tag) === property)
+                                : [];
                             t.forEach((t) => (t ? queries.removeFilter(t) : null));
                             queries.apply();
                             parsedTags.update((tags) => tags.filter((t) => t.tag !== tag.tag));
                         }}>

104-105: Redundant derived state.

filterCols is just an alias for availableFilters. Consider using availableFilters directly in the template or rename the original if the name is preferred.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8b1d5eb and 999f5a0.

📒 Files selected for processing (3)
  • src/lib/components/filters/parsedTagList.svelte
  • src/lib/components/filters/quickFilters.svelte
  • src/lib/layout/responsiveContainerHeader.svelte
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx,svelte}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx,js,jsx,svelte}: Import reusable modules from the src/lib directory using the $lib alias
Use minimal comments in code; reserve comments for TODOs or complex logic explanations
Use $lib, $routes, and $themes aliases instead of relative paths for module imports

Files:

  • src/lib/layout/responsiveContainerHeader.svelte
  • src/lib/components/filters/quickFilters.svelte
  • src/lib/components/filters/parsedTagList.svelte
**/*.{ts,tsx,js,jsx,svelte,json}

📄 CodeRabbit inference engine (AGENTS.md)

Use 4 spaces for indentation, single quotes, 100 character line width, and no trailing commas per Prettier configuration

Files:

  • src/lib/layout/responsiveContainerHeader.svelte
  • src/lib/components/filters/quickFilters.svelte
  • src/lib/components/filters/parsedTagList.svelte
**/*.svelte

📄 CodeRabbit inference engine (AGENTS.md)

Use Svelte 5 + SvelteKit 2 syntax with TypeScript for component development

Files:

  • src/lib/layout/responsiveContainerHeader.svelte
  • src/lib/components/filters/quickFilters.svelte
  • src/lib/components/filters/parsedTagList.svelte
src/lib/components/**/*.svelte

📄 CodeRabbit inference engine (AGENTS.md)

Use PascalCase for component file names and place them in src/lib/components/[feature]/ directory structure

Files:

  • src/lib/components/filters/quickFilters.svelte
  • src/lib/components/filters/parsedTagList.svelte
🧠 Learnings (1)
📚 Learning: 2025-11-25T03:15:27.539Z
Learnt from: CR
Repo: appwrite/console PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:15:27.539Z
Learning: Applies to **/*.svelte : Use Svelte 5 + SvelteKit 2 syntax with TypeScript for component development

Applied to files:

  • src/lib/layout/responsiveContainerHeader.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: build
  • GitHub Check: e2e
🔇 Additional comments (4)
src/lib/components/filters/quickFilters.svelte (1)

25-31: LGTM!

The button refactoring to an icon-only style with ariaLabel for accessibility is appropriate. The text and icon props correctly configure the button appearance while maintaining screen reader support.

src/lib/layout/responsiveContainerHeader.svelte (2)

104-135: Layout restructuring looks good.

The flex layout with space-between and separate left/right clusters is well-structured. The inline styles for min-width: 0 and flex: 1 1 auto correctly handle overflow behavior in flex containers.


175-178: Consistent button styling with quickFilters.svelte.

The text prop change aligns with the related changes in quickFilters.svelte, maintaining visual consistency across filter buttons.

src/lib/components/filters/parsedTagList.svelte (1)

124-294: Template structure is well-organized.

The rendering logic clearly separates active tags, placeholders, the clear button, and the QuickFilters component. The compound tag composition with per-part interactive menus provides good UX for filter management.

Comment on lines +172 to +190
addFilterAndApply(
filter.id,
filter.title,
filter.operator,
null,
next,
$columns,
''
);
} else {
addFilterAndApply(
filter.id,
filter.title,
filter.operator,
option.value,
[],
$columns,
''
);
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

analyticsSource prop not passed to addFilterAndApply.

The analyticsSource prop is received but hardcoded empty strings are passed to addFilterAndApply calls. This will break analytics tracking for filter interactions.

Proposed fix
                                                     addFilterAndApply(
                                                         filter.id,
                                                         filter.title,
                                                         filter.operator,
                                                         null,
                                                         next,
                                                         $columns,
-                                                        ''
+                                                        analyticsSource
                                                     );
                                                 } else {
                                                     addFilterAndApply(
                                                         filter.id,
                                                         filter.title,
                                                         filter.operator,
                                                         option.value,
                                                         [],
                                                         $columns,
-                                                        ''
+                                                        analyticsSource
                                                     );
                                                 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
addFilterAndApply(
filter.id,
filter.title,
filter.operator,
null,
next,
$columns,
''
);
} else {
addFilterAndApply(
filter.id,
filter.title,
filter.operator,
option.value,
[],
$columns,
''
);
addFilterAndApply(
filter.id,
filter.title,
filter.operator,
null,
next,
$columns,
analyticsSource
);
} else {
addFilterAndApply(
filter.id,
filter.title,
filter.operator,
option.value,
[],
$columns,
analyticsSource
);
🤖 Prompt for AI Agents
In @src/lib/components/filters/parsedTagList.svelte around lines 172 - 190, The
calls to addFilterAndApply in parsedTagList.svelte are passing empty strings for
the analyticsSource argument, which drops the component's analyticsSource prop;
update both call sites (the branch that passes null for value/next and the
branch that passes option.value/[]) to forward the component's analyticsSource
prop instead of '' so analyticsSource is sent through addFilterAndApply.

Comment on lines +243 to +249
on:click={(e) => {
e.stopPropagation();
if (!hiddenPlaceholders.includes(filter.title)) {
hiddenPlaceholders.push(filter.title);
}
placeholderVersion++;
}}>
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

Direct array mutation may cause reactivity issues.

In Svelte 5, while array mutations can trigger reactivity in some cases, the pattern of directly mutating hiddenPlaceholders with .push() combined with a manual placeholderVersion++ increment suggests instability. Use immutable updates for reliable reactivity:

Proposed fix
                             on:click={(e) => {
                                 e.stopPropagation();
-                                if (!hiddenPlaceholders.includes(filter.title)) {
-                                    hiddenPlaceholders.push(filter.title);
-                                }
-                                placeholderVersion++;
+                                if (!hiddenPlaceholders.includes(filter.title)) {
+                                    hiddenPlaceholders = [...hiddenPlaceholders, filter.title];
+                                }
                             }}>

If reactivity works correctly with immutable updates, you can also remove the placeholderVersion state and the {#key placeholderVersion} wrapper (lines 110 and 232).

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
on:click={(e) => {
e.stopPropagation();
if (!hiddenPlaceholders.includes(filter.title)) {
hiddenPlaceholders.push(filter.title);
}
placeholderVersion++;
}}>
on:click={(e) => {
e.stopPropagation();
if (!hiddenPlaceholders.includes(filter.title)) {
hiddenPlaceholders = [...hiddenPlaceholders, filter.title];
}
}}>
🤖 Prompt for AI Agents
In @src/lib/components/filters/parsedTagList.svelte around lines 243 - 249, The
click handler mutates hiddenPlaceholders with .push() and manually increments
placeholderVersion which can break Svelte reactivity; replace the mutation with
an immutable assignment like setting hiddenPlaceholders =
[...hiddenPlaceholders, filter.title] (only if filter.title not already
included) and remove the placeholderVersion++; after confirming immutable
updates work you can also remove placeholderVersion state and the {#key
placeholderVersion} wrapper (ensure you update the on:click handler references
and any code that reads hiddenPlaceholders accordingly).

Comment on lines +259 to +267
addFilterAndApply(
filter.id,
filter.title,
filter.operator,
filter?.array ? null : option.value,
filter?.array ? [option.value] : [],
$columns,
''
);
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Same issue: analyticsSource not passed in placeholder filter actions.

Apply the same fix here for consistency:

Proposed fix
                                             addFilterAndApply(
                                                 filter.id,
                                                 filter.title,
                                                 filter.operator,
                                                 filter?.array ? null : option.value,
                                                 filter?.array ? [option.value] : [],
                                                 $columns,
-                                                ''
+                                                analyticsSource
                                             );
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
addFilterAndApply(
filter.id,
filter.title,
filter.operator,
filter?.array ? null : option.value,
filter?.array ? [option.value] : [],
$columns,
''
);
addFilterAndApply(
filter.id,
filter.title,
filter.operator,
filter?.array ? null : option.value,
filter?.array ? [option.value] : [],
$columns,
analyticsSource
);
🤖 Prompt for AI Agents
In @src/lib/components/filters/parsedTagList.svelte around lines 259 - 267, The
placeholder filter action call to addFilterAndApply is missing the
analyticsSource argument; update the call in parsedTagList.svelte so
addFilterAndApply(filter.id, filter.title, filter.operator, filter?.array ? null
: option.value, filter?.array ? [option.value] : [], $columns, '',
analyticsSource) includes the analyticsSource parameter (or the appropriate
variable name used elsewhere) to match other calls to addFilterAndApply.

Comment on lines +113 to +121
<!-- Tags with Filters button (rendered inside ParsedTagList) -->
<Layout.Stack
direction="row"
alignItems="center"
gap="s"
wrap="wrap"
style={`min-width: 0;`}>
<ParsedTagList {columns} {analyticsSource} />
</Layout.Stack>
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Missing hasFilters conditional check for ParsedTagList.

In the small viewport case, filter rendering is gated by hasFilters && $columns?.length, but here ParsedTagList is rendered unconditionally. This could display filter UI when hasFilters is false.

Proposed fix
-                   <!-- Tags with Filters button (rendered inside ParsedTagList) -->
-                   <Layout.Stack
-                       direction="row"
-                       alignItems="center"
-                       gap="s"
-                       wrap="wrap"
-                       style={`min-width: 0;`}>
-                       <ParsedTagList {columns} {analyticsSource} />
-                   </Layout.Stack>
+                   {#if hasFilters && $columns?.length}
+                       <!-- Tags with Filters button (rendered inside ParsedTagList) -->
+                       <Layout.Stack
+                           direction="row"
+                           alignItems="center"
+                           gap="s"
+                           wrap="wrap"
+                           style={`min-width: 0;`}>
+                           <ParsedTagList {columns} {analyticsSource} />
+                       </Layout.Stack>
+                   {/if}
🤖 Prompt for AI Agents
In @src/lib/layout/responsiveContainerHeader.svelte around lines 113 - 121,
ParsedTagList is being rendered unconditionally inside the Layout.Stack for the
small-viewport branch which can show filter UI even when hasFilters is false;
update the rendering to conditionally render ParsedTagList only when hasFilters
&& $columns?.length (same condition used elsewhere) so wrap the
Layout.Stack/ParsedTagList block with that conditional check to match the
intended behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants