feat: added new filters for sites and functions #2779
Conversation
WalkthroughReworks tag/filter UI across three components. parsedTagList.svelte was restructured to render tags as compound parts with per-part menus, added parseTagParts and firstBoldText helpers, derived filter metadata from columns, introduced dismissible placeholders with per-placeholder menus and apply actions, changed tag dismissal to operate via queries and updated parsedTags, and integrated QuickFilters rendering alongside tags. quickFilters.svelte changed the Filters Button to use ariaLabel, enabled text+icon mode, and moved the Icon into the Button body. responsiveContainerHeader.svelte inlines ParsedTagList for small viewports, adjusts stacks and alignment for large viewports, removes the QuickFilters import, and switches 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. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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.
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.
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.
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.
🛠️ 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.
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.
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.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/lib/components/filters/parsedTagList.svelte`:
- Around line 46-49: The code is deriving column identity from display
formatting (firstBoldText and tag.tag.split(' ')), which is brittle; instead add
a stable identifier (e.g., parsedTag.id or parsedTag.title) to each parsedTags
item and wire all logic to use that property. Update the parsedTags construction
(where parsedTags is built) to include a dedicated id/title field, replace all
calls to firstBoldText(...) and tag.tag.split(' ')[0] with references to
parsedTag.id or parsedTag.title, and change dismissal/routing logic (the
functions that read the bold text or first word) to operate on that stable
property; remove or deprecate firstBoldText once callers are migrated. Ensure
multi-word titles are preserved by using the new field rather than splitting the
display string.
- Around line 23-26: The component currently treats the optional prop "columns"
as if it always exists (uses $columns and passes columns to QuickFilters), which
will crash if the parent omits it; fix by giving "columns" a default writable
store (e.g., import { writable } from 'svelte/store' and initialize columns to
writable<Column[]>([]) in the destructure) and update the prop typing to
non-optional Writable<Column[]> so all uses of $columns and the QuickFilters
prop receive a valid store even when the parent doesn't provide one.
♻️ Duplicate comments (2)
src/lib/components/filters/parsedTagList.svelte (2)
142-160: ForwardanalyticsSourcein tag and placeholder menu actions.
addFilterAndApplystill receives''instead of theanalyticsSourceprop, so tag menu interactions won’t be tracked. This was flagged earlier.Also applies to: 229-237
73-75: Prefer immutable updates forhiddenPlaceholders(reactivity).Direct
.push()plus a manualplaceholderVersion++is brittle in Svelte reactivity. Use immutable assignment and drop the version key if possible.♻️ Suggested change
- let hiddenPlaceholders: string[] = $state([]); - let placeholderVersion = $state(0); // used to force keyed re-render when needed + let hiddenPlaceholders: string[] = $state([]);- if (!hiddenPlaceholders.includes(filter.title)) { - hiddenPlaceholders.push(filter.title); - } - placeholderVersion++; + if (!hiddenPlaceholders.includes(filter.title)) { + hiddenPlaceholders = [...hiddenPlaceholders, filter.title]; + }Also applies to: 213-218
| let { | ||
| columns, | ||
| analyticsSource = '' | ||
| }: { columns: Writable<Column[]> | undefined; analyticsSource?: string } = $props(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd /tmp/repo 2>/dev/null || cd /
# First, find the file
find . -name "parsedTagList.svelte" -type f 2>/dev/null | head -5Repository: appwrite/console
Length of output: 127
🏁 Script executed:
# List the file to understand its size before reading
wc -l src/lib/components/filters/parsedTagList.svelte 2>/dev/null || echo "File not found, searching..."Repository: appwrite/console
Length of output: 111
🏁 Script executed:
# Search for the file in the repository
find . -path "*filters*parsedTagList*" -type f 2>/dev/nullRepository: appwrite/console
Length of output: 109
🏁 Script executed:
cat -n src/lib/components/filters/parsedTagList.svelteRepository: appwrite/console
Length of output: 15676
🏁 Script executed:
rg "parsedTagList" --type svelte -A 3 -B 3 | head -50Repository: appwrite/console
Length of output: 90
🏁 Script executed:
# Check QuickFilters to see if it expects optional columns
head -100 src/lib/components/filters/quickFilters.svelteRepository: appwrite/console
Length of output: 2578
🏁 Script executed:
# Check addFilterAndApply function
rg "export.*function addFilterAndApply" -A 15Repository: appwrite/console
Length of output: 1353
🏁 Script executed:
# Find where parsedTagList component is imported and used
rg "parsedTagList" -t ts -t tsx -t js -t jsxRepository: appwrite/console
Length of output: 87
🏁 Script executed:
# More direct search without file type
rg "import.*parsedTagList\|<parsedTagList\|<ParsedTagList"Repository: appwrite/console
Length of output: 42
🏁 Script executed:
# Check the file that imports parsedTagList
rg "parsedTagList" --files-with-matchesRepository: appwrite/console
Length of output: 95
🏁 Script executed:
# Check the index file
cat src/lib/components/filters/index.tsRepository: appwrite/console
Length of output: 535
🏁 Script executed:
# Search for imports of parsedTagList more broadly
rg "ParsedTagList|parsedTagList" -A 2 -B 2Repository: appwrite/console
Length of output: 2488
🏁 Script executed:
# Search for where this component is actually used in the application
rg "ParsedTagList" --no-filenameRepository: appwrite/console
Length of output: 446
🏁 Script executed:
# Check the responsiveContainerHeader to see if columns is always defined
cat src/lib/layout/responsiveContainerHeader.svelte | head -100Repository: appwrite/console
Length of output: 4048
🏁 Script executed:
# Get the full context around where ParsedTagList is used
rg "ParsedTagList" -B 20 -A 5 src/lib/layout/responsiveContainerHeader.svelte | head -80Repository: appwrite/console
Length of output: 2443
🏁 Script executed:
# Check the exact lines where $columns is passed without guards in parsedTagList
sed -n '145,160p' src/lib/components/filters/parsedTagList.svelteRepository: appwrite/console
Length of output: 1340
🏁 Script executed:
# Also check lines around 235
sed -n '225,240p' src/lib/components/filters/parsedTagList.svelteRepository: appwrite/console
Length of output: 1145
🏁 Script executed:
# Check the responsiveContainerHeader to understand the flow with columns
sed -n '38,55p' src/lib/layout/responsiveContainerHeader.svelteRepository: appwrite/console
Length of output: 759
🏁 Script executed:
cat src/lib/components/filters/quickFilters.ts | grep -A 20 "export function addFilterAndApply"Repository: appwrite/console
Length of output: 841
Dereference of undefined store will cause runtime errors.
columns is typed as optional but the component attempts to dereference it with $columns in multiple places (lines 148, 158, 235) and passes it to QuickFilters (line 263), which expects a required Writable<Column[]>. If the parent (responsiveContainerHeader) doesn't always provide columns, the component will crash when trying to subscribe or pass the store to child components.
Provide a default empty writable store to make columns required:
🔧 Suggested fix
+ import { writable, type Writable } from 'svelte/store';
- import type { Writable } from 'svelte/store';
let {
- columns,
+ columns = writable<Column[]>([]),
analyticsSource = ''
- }: { columns: Writable<Column[]> | undefined; analyticsSource?: string } = $props();
+ }: { columns?: Writable<Column[]>; analyticsSource?: string } = $props();🤖 Prompt for AI Agents
In `@src/lib/components/filters/parsedTagList.svelte` around lines 23 - 26, The
component currently treats the optional prop "columns" as if it always exists
(uses $columns and passes columns to QuickFilters), which will crash if the
parent omits it; fix by giving "columns" a default writable store (e.g., import
{ writable } from 'svelte/store' and initialize columns to
writable<Column[]>([]) in the destructure) and update the prop typing to
non-optional Writable<Column[]> so all uses of $columns and the QuickFilters
prop receive a valid store even when the parent doesn't provide one.
| function firstBoldText(tagString: string): string | null { | ||
| const m = /\*\*(.*?)\*\*/.exec(tagString); | ||
| return m ? m[1] : null; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/lib/components/filters/parsedTagList.svelteRepository: appwrite/console
Length of output: 15676
Avoid deriving filter identity from tag string formatting.
The code uses two brittle approaches to extract column titles: firstBoldText() at lines 97 and 77 relies on bold markers (**...**), while line 186 uses tag.tag.split(' ')[0] to extract the first word. These assume a specific tag format; any change breaks the logic. Multi-word column titles also fail with word splitting—only the first word is extracted. Instead, carry a stable column id/title identifier directly in parsedTags items to avoid parsing the display string and risking menu misrouting or incorrect filter dismissals.
Applies to lines 46–49, 96–99, 185–188.
🤖 Prompt for AI Agents
In `@src/lib/components/filters/parsedTagList.svelte` around lines 46 - 49, The
code is deriving column identity from display formatting (firstBoldText and
tag.tag.split(' ')), which is brittle; instead add a stable identifier (e.g.,
parsedTag.id or parsedTag.title) to each parsedTags item and wire all logic to
use that property. Update the parsedTags construction (where parsedTags is
built) to include a dedicated id/title field, replace all calls to
firstBoldText(...) and tag.tag.split(' ')[0] with references to parsedTag.id or
parsedTag.title, and change dismissal/routing logic (the functions that read the
bold text or first word) to operate on that stable property; remove or deprecate
firstBoldText once callers are migrated. Ensure multi-word titles are preserved
by using the new field rather than splitting the display string.
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.