geido commented on a change in pull request #17410:
URL: https://github.com/apache/superset/pull/17410#discussion_r749341818
##########
File path:
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FilterConfigPane.test.tsx
##########
@@ -93,22 +94,41 @@ test('remove filter', async () => {
}),
);
});
- expect(defaultProps.onEdit).toHaveBeenCalledWith('NATIVE_FILTER-2',
'remove');
+ expect(defaultProps.onRemove).toHaveBeenCalledWith('NATIVE_FILTER-2');
});
test('add filter', async () => {
defaultRender();
// First trash icon
- const removeFilterIcon = screen.getByText('Add filter')!;
+ const addButton = screen.getByText('Add')!;
+ fireEvent.mouseOver(addButton);
+ const addFilterButton = await screen.findByText('Filter');
+
await act(async () => {
fireEvent(
- removeFilterIcon,
+ addFilterButton,
new MouseEvent('click', {
bubbles: true,
cancelable: true,
}),
);
});
+ expect(defaultProps.onAdd).toHaveBeenCalledWith('NATIVE_FILTER');
+});
- expect(defaultProps.onEdit).toHaveBeenCalledWith('', 'add');
+test('add divider', async () => {
+ defaultRender();
+ const addButton = screen.getByText('Add')!;
+ fireEvent.mouseOver(addButton);
+ const addFilterButton = await screen.findByText('Divider');
+ await act(async () => {
+ fireEvent(
+ addFilterButton,
+ new MouseEvent('click', {
+ bubbles: true,
+ cancelable: true,
+ }),
+ );
+ });
+ expect(defaultProps.onAdd).toHaveBeenCalledWith('DIVIDER');
});
Review comment:
Is it possible to use the `userEvent` methods instead of `fireEvent`?
##########
File path:
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/index.tsx
##########
@@ -320,7 +323,14 @@ const FilterBar: React.FC<FiltersBarProps> = ({
activeKey={editFilterSetId ? TabIds.AllFilters : undefined}
>
<Tabs.TabPane
- tab={t(`All Filters (${filterValues.length})`)}
+ tab={t(
+ `All Filters (${
+ filterValues.filter(
+ filterValue =>
+ filterValue.type === NativeFilterType.NATIVE_FILTER,
+ ).length
+ })`,
+ )}
Review comment:
Can we move the filtering part in a const outside for the sake of
readability? Also, the naming convention we adopted is `All filters`, it would
be great to fix it here. Finally, I think the length itself should be outside
of the localization function as it is a non-translatable value.
--
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]
