Conversation
Console (appwrite/console)Project ID: Tip Roll back Sites deployments instantly by switching between versions |
WalkthroughThis pull request introduces a new dropdown-style filter UI option for the messaging module. The QuickFilters component gains a Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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/layout/responsiveContainerHeader.svelte`:
- Around line 105-109: The mobile path currently renders the generic
filtersButton without any active-filter indicator when filtersStyle ===
'dropdown' and hasFilters is true; update the filtersButton snippet (the
element/render block referenced by filtersButton) to conditionally show a
badge/indicator whenever hasFilters is true and filtersStyle === 'dropdown' (use
the same badge markup/props used by QuickFilters/ParsedTagList to keep visuals
consistent), or alternatively render <QuickFilters> on small viewports when
filtersStyle === 'dropdown' by reusing the existing QuickFilters component
logic; focus your change on the filtersButton/QuickFilters conditional logic
around filtersStyle, hasFilters, ParsedTagList and showFilters so mobile users
see the active-filter badge.
🧹 Nitpick comments (1)
src/lib/layout/responsiveContainerHeader.svelte (1)
140-146:filterColsis pre-filtered here, butQuickFiltersalso filters internally.Line 145 passes
filterCols.filter((f) => f?.options), butQuickFiltersre-applies the same.filter((f) => f?.options)on Line 47 ofquickFilters.svelte. Not a bug, but a minor redundancy — you could passfilterColsas-is and letQuickFiltershandle the filtering, or remove the internal filter. Low priority.
| {#if hasFilters && filtersStyle === 'chips'} | ||
| <div style="overflow-x: auto;"> | ||
| <ParsedTagList {columns} {analyticsSource} /> | ||
| </div> | ||
| {/if} |
There was a problem hiding this comment.
Small viewport in 'dropdown' mode has no visual indicator for active filters.
When filtersStyle === 'dropdown', this guard hides the ParsedTagList chips on small viewports. However, the small viewport path doesn't render QuickFilters (which carries the active-filter badge) either — it only shows the generic filtersButton snippet (Line 195), which has no badge. This means users on mobile won't see any indication that filters are currently applied.
Consider either:
- Adding a badge to the
filtersButtonsnippet whenfiltersStyle === 'dropdown', or - Rendering
QuickFilterson small viewports in dropdown mode as well.
#!/bin/bash
# Check if filtersButton snippet or the small viewport filter path has any badge/indicator logic
rg -n 'filtersButton\|showFilters\|parsedTags' src/lib/layout/responsiveContainerHeader.svelte🤖 Prompt for AI Agents
In `@src/lib/layout/responsiveContainerHeader.svelte` around lines 105 - 109, The
mobile path currently renders the generic filtersButton without any
active-filter indicator when filtersStyle === 'dropdown' and hasFilters is true;
update the filtersButton snippet (the element/render block referenced by
filtersButton) to conditionally show a badge/indicator whenever hasFilters is
true and filtersStyle === 'dropdown' (use the same badge markup/props used by
QuickFilters/ParsedTagList to keep visuals consistent), or alternatively render
<QuickFilters> on small viewports when filtersStyle === 'dropdown' by reusing
the existing QuickFilters component logic; focus your change on the
filtersButton/QuickFilters conditional logic around filtersStyle, hasFilters,
ParsedTagList and showFilters so mobile users see the active-filter badge.
2 participants
What does this PR do?
before:
after :
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
Release Notes