Analytics filter refinements#49
Analytics filter refinements#49tomerqodo wants to merge 10 commits intocoderabbit_full_base_analytics_filter_refinements_pr11from
Conversation
WalkthroughThe changes introduce a new optional Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/shade/src/components/ui/filters.tsx (1)
105-122: Avoid empty default Add Filter labels (accessibility regression).
SettingDEFAULT_I18N.addFilter/addFilterTitleto''makes the default add button icon-only with no accessible name for consumers that don’t override i18n. Consider keeping non-empty defaults or add a fallback label elsewhere.🔧 Suggested fix
- addFilter: '', + addFilter: 'Add filter', ... - addFilterTitle: '', + addFilterTitle: 'Add filter'apps/stats/src/views/Stats/components/stats-filter.tsx (1)
448-456: Use canonical operator key for audience default operator.
FilterOperatorvalues in the filters UI are snake_case (e.g.,is_any_of). Using'is any of'won’t match and can break operator lookups downstream.🐛 Suggested fix
- defaultOperator: 'is any of', + defaultOperator: 'is_any_of',
🤖 Fix all issues with AI agents
In `@apps/shade/src/components/ui/filters.tsx`:
- Around line 2098-2101: The onClose handler for the add-filter popover
currently calls setAddFilterOpen(false) and setSelectedFieldKeyForOptions(null)
but does not clear tempSelectedValues, which allows stale selections to persist
and leak into subsequent fields (affecting autoCloseOnSelect and
single/multi-select behavior); update the onClose callback to also reset
tempSelectedValues (e.g., call setTempSelectedValues([]) or the appropriate
empty value) so that when onClose is invoked the temporary selection state is
cleared along with setAddFilterOpen and setSelectedFieldKeyForOptions.
In `@apps/stats/src/views/Stats/components/stats-filter.tsx`:
- Around line 497-504: The icon-only add button currently has no accessible name
because addButtonText is empty; update the JSX where addButtonIcon/addButtonText
are set to supply an accessible label — either set addButtonText='Add filter'
(or a localized equivalent) or pass an explicit aria-label via a prop like
addButtonProps={{ 'aria-label': 'Add filter' }} on the same component so the add
button has a programmatic name; modify the snippet that sets addButtonIcon and
addButtonText in stats-filter.tsx accordingly.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/shade/src/components/ui/filters.tsxapps/stats/src/views/Stats/components/stats-filter.tsxapps/stats/src/views/Stats/layout/stats-header.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
apps/shade/src/components/ui/filters.tsx (2)
apps/shade/src/components/ui/popover.tsx (1)
PopoverContent(31-31)apps/shade/src/index.ts (1)
cn(82-82)
🔇 Additional comments (10)
apps/stats/src/views/Stats/layout/stats-header.tsx (1)
67-67: Navbar padding tweak looks good.apps/shade/src/components/ui/filters.tsx (6)
239-239: LGTM on the small input padding tweak.
750-751:autoCloseOnSelectflag addition looks good.
946-948: Operator label styling adjustment looks fine.
1049-1052: Resetting search input on close is a nice cleanup.
1125-1137: Auto-close behavior on selection is consistent.Also applies to: 1271-1274
1201-1207: Closed-state animation suppression looks consistent across popovers.Also applies to: 1644-1644, 2078-2084
apps/stats/src/views/Stats/components/stats-filter.tsx (3)
93-96: Badge text-style swap looks good.Also applies to: 159-162
386-440: UTM/source dropdown visibility tweaks look fine.Also applies to: 483-485
458-471: Post selector sizing/icon updates look good.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| onClose={() => { | ||
| setAddFilterOpen(false); | ||
| setSelectedFieldKeyForOptions(null); | ||
| }} |
There was a problem hiding this comment.
Clear tempSelectedValues when closing via onClose.
When autoCloseOnSelect closes the add-filter popover, tempSelectedValues isn’t cleared, so the next field can inherit stale values and even flip a single-select into multiselect behavior.
🐛 Suggested fix
onClose={() => {
setAddFilterOpen(false);
setSelectedFieldKeyForOptions(null);
+ setTempSelectedValues([]);
}}🤖 Prompt for AI Agents
In `@apps/shade/src/components/ui/filters.tsx` around lines 2098 - 2101, The
onClose handler for the add-filter popover currently calls
setAddFilterOpen(false) and setSelectedFieldKeyForOptions(null) but does not
clear tempSelectedValues, which allows stale selections to persist and leak into
subsequent fields (affecting autoCloseOnSelect and single/multi-select
behavior); update the onClose callback to also reset tempSelectedValues (e.g.,
call setTempSelectedValues([]) or the appropriate empty value) so that when
onClose is invoked the temporary selection state is cleared along with
setAddFilterOpen and setSelectedFieldKeyForOptions.
| addButtonIcon={<LucideIcon.FunnelPlus />} | ||
| addButtonText={filters.length ? '' : ''} | ||
| className='mb-6 [&>button]:order-last' | ||
| fields={groupedFields} | ||
| filters={filters} | ||
| showSearchInput={false} | ||
| size='sm' | ||
| onChange={handleFilterChange} |
There was a problem hiding this comment.
Provide an accessible label for the icon-only add button.
With addButtonText empty and defaults now blank, the button can end up without an accessible name. Please supply a label (or custom button with aria-label) to keep it accessible.
🔧 Example fix
- addButtonText={filters.length ? '' : ''}
+ addButtonText="Add filter"📝 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.
| addButtonIcon={<LucideIcon.FunnelPlus />} | |
| addButtonText={filters.length ? '' : ''} | |
| className='mb-6 [&>button]:order-last' | |
| fields={groupedFields} | |
| filters={filters} | |
| showSearchInput={false} | |
| size='sm' | |
| onChange={handleFilterChange} | |
| addButtonIcon={<LucideIcon.FunnelPlus />} | |
| addButtonText="Add filter" | |
| className='mb-6 [&>button]:order-last' | |
| fields={groupedFields} | |
| filters={filters} | |
| showSearchInput={false} | |
| size='sm' | |
| onChange={handleFilterChange} |
🤖 Prompt for AI Agents
In `@apps/stats/src/views/Stats/components/stats-filter.tsx` around lines 497 -
504, The icon-only add button currently has no accessible name because
addButtonText is empty; update the JSX where addButtonIcon/addButtonText are set
to supply an accessible label — either set addButtonText='Add filter' (or a
localized equivalent) or pass an explicit aria-label via a prop like
addButtonProps={{ 'aria-label': 'Add filter' }} on the same component so the add
button has a programmatic name; modify the snippet that sets addButtonIcon and
addButtonText in stats-filter.tsx accordingly.
2 participants
Benchmark PR from agentic-review-benchmarks#11
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Style
✏️ Tip: You can customize this high-level summary in your review settings.