-
Notifications
You must be signed in to change notification settings - Fork 208
feat: added new filters for sites and functions #2779
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughReworks 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)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (3)**/*.{ts,tsx,js,jsx,svelte}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx,js,jsx,svelte,json}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.svelte📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (1)📚 Learning: 2025-11-25T03:15:27.539ZApplied to files:
⏰ 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)
🔇 Additional comments (4)
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. Comment |
There was a problem hiding this 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 offilterCols.Line 58 assigns
filterColsto itself. SincefilterColsis a$derivedvalue (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. Sincecolumnsis typed asWritable<Column[]>,$columnsshould already beColumn[]. If the types don't match, consider fixing the root cause rather than usingunknowncasts.🔧 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$statearray 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
placeholderVersionstate 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
📒 Files selected for processing (3)
src/lib/components/filters/parsedTagList.sveltesrc/lib/components/filters/quickFilters.sveltesrc/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.sveltesrc/lib/components/filters/quickFilters.sveltesrc/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.sveltesrc/lib/components/filters/quickFilters.sveltesrc/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.sveltesrc/lib/components/filters/quickFilters.sveltesrc/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.sveltesrc/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-startalignment to handle variable tag heights- Applies
min-width: 0to 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/CompoundTagChildwith 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 whengetFilterFor()successfully returns a filter—which requirescolumnsto be defined (line 88 checksif (!columns) return null).Similarly, lines 259-267 are protected by
{#if placeholders?.length}, andplaceholdersis derived fromavailableFilters, which evaluates to an empty array whencolumnsis undefined (line 97 uses optional chaining?.length).The code already safely handles the case where
columnsis undefined without needing additional guards.
| addFilterAndApply( | ||
| filter.id, | ||
| filter.title, | ||
| filter.operator, | ||
| null, | ||
| next, | ||
| $columns, | ||
| '' | ||
| ); | ||
| } else { | ||
| addFilterAndApply( | ||
| filter.id, | ||
| filter.title, | ||
| filter.operator, | ||
| option.value, | ||
| [], | ||
| $columns, | ||
| '' | ||
| ); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
| 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.
There was a problem hiding this 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$libalias for consistency.Per coding guidelines, use the
$libalias 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$columnsshould already beColumn[] | undefinedwhen dereferencing aWritable<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.
filterColsis just an alias foravailableFilters. Consider usingavailableFiltersdirectly in the template or rename the original if the name is preferred.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/lib/components/filters/parsedTagList.sveltesrc/lib/components/filters/quickFilters.sveltesrc/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.sveltesrc/lib/components/filters/quickFilters.sveltesrc/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.sveltesrc/lib/components/filters/quickFilters.sveltesrc/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.sveltesrc/lib/components/filters/quickFilters.sveltesrc/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.sveltesrc/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
ariaLabelfor accessibility is appropriate. Thetextandiconprops 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-betweenand separate left/right clusters is well-structured. The inline styles formin-width: 0andflex: 1 1 autocorrectly handle overflow behavior in flex containers.
175-178: Consistent button styling withquickFilters.svelte.The
textprop change aligns with the related changes inquickFilters.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.
| addFilterAndApply( | ||
| filter.id, | ||
| filter.title, | ||
| filter.operator, | ||
| null, | ||
| next, | ||
| $columns, | ||
| '' | ||
| ); | ||
| } else { | ||
| addFilterAndApply( | ||
| filter.id, | ||
| filter.title, | ||
| filter.operator, | ||
| option.value, | ||
| [], | ||
| $columns, | ||
| '' | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
| 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.
| on:click={(e) => { | ||
| e.stopPropagation(); | ||
| if (!hiddenPlaceholders.includes(filter.title)) { | ||
| hiddenPlaceholders.push(filter.title); | ||
| } | ||
| placeholderVersion++; | ||
| }}> |
There was a problem hiding this comment.
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.
| 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).
| addFilterAndApply( | ||
| filter.id, | ||
| filter.title, | ||
| filter.operator, | ||
| filter?.array ? null : option.value, | ||
| filter?.array ? [option.value] : [], | ||
| $columns, | ||
| '' | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
| 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.
| <!-- 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> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
What does this PR do?
for ipad

for mobile

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
UI/UX Updates
✏️ Tip: You can customize this high-level summary in your review settings.