bito-code-review[bot] commented on code in PR #40169:
URL: https://github.com/apache/superset/pull/40169#discussion_r3250030863
##########
superset-frontend/src/components/ListView/Filters/index.test.tsx:
##########
@@ -97,6 +97,63 @@ test('search filter passes autoComplete prop correctly', ()
=> {
expect(input.autocomplete).toBe('new-password');
});
+test('renders a compact pill trigger for select filters', () => {
+ const filters = [
+ {
+ Header: 'Owner',
+ key: 'owner',
+ id: 'owner',
+ input: 'select' as const,
+ operator: ListViewFilterOperator.RelationOneMany,
+ selects: [
+ { label: 'Alice', value: 1 },
+ { label: 'Bob', value: 2 },
+ ],
+ },
+ ];
+
+ render(
+ <UIFilters
+ filters={filters}
+ internalFilters={[]}
+ updateFilterValue={mockUpdateFilterValue}
+ />,
+ );
+
+ expect(screen.getByTestId('compact-filter-pill')).toBeInTheDocument();
+ expect(screen.getByText('Owner')).toBeInTheDocument();
+});
+
+test('select pill shows active state when a value is selected', () => {
+ const filters = [
+ {
+ Header: 'Owner',
+ key: 'owner',
+ id: 'owner',
+ input: 'select' as const,
+ operator: ListViewFilterOperator.RelationOneMany,
+ selects: [{ label: 'Alice', value: 1 }],
+ },
+ ];
+
+ render(
+ <UIFilters
+ filters={filters}
+ internalFilters={[
+ {
+ id: 'owner',
+ operator: ListViewFilterOperator.RelationOneMany,
+ value: { label: 'Alice', value: 1 },
+ },
+ ]}
+ updateFilterValue={mockUpdateFilterValue}
+ />,
+ );
+
+ // When a value is present, the clear (close) icon should be shown
+ expect(screen.getByRole('button', { name: /close/i })).toBeInTheDocument();
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Incomplete test assertion</b></div>
<div id="fix">
The test’s `getByRole('button', { name: /close/i })` assertion will never
match because the icon lacks an aria-label and the pill’s aria-label is the
filter label (‘Owner’). To fix, either add `aria-label="Close"` to
`Icons.CloseOutlined` in `CompactFilterTrigger.tsx`, or update the test to
target the icon element via a `data-testid` or to assert the pill’s aria-label
value matches the header text while separately verifying that the clear icon
element is rendered.
</div>
</div>
<small><i>Code Review Run #79da62</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset-frontend/src/components/ListView/Filters/index.tsx:
##########
@@ -78,50 +93,68 @@ function UIFilters(
key,
id,
input,
- optionFilterProps,
- paginate,
selects,
toolTipDescription,
onFilterUpdate,
loading,
dateFilterValueType,
min,
max,
- popupStyle,
autoComplete,
inputName,
},
index,
) => {
const initialValue = internalFilters?.[index]?.value;
if (input === 'select') {
+ const selectValue = initialValue as SelectOption | undefined;
+ // Use cached label — URL round-trip strips the label from
internalFilters,
+ // leaving only the value (e.g. {value: 1} with no label field).
+ const tooltipTitle = !!selectValue
+ ? tooltipLabels[index]
+ : undefined;
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing tests for tooltip label caching</b></div>
<div id="fix">
The tooltip label caching is core functionality — it restores display labels
after URL round-trips strip them from internalFilters. No test in
index.test.tsx (lines 100–189) verifies the tooltip renders the correct label
or that clearing the filter removes it from the cache.
</div>
</div>
<small><i>Code Review Run #79da62</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
