villebro commented on a change in pull request #13031:
URL: https://github.com/apache/superset/pull/13031#discussion_r577402225
##########
File path:
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.tsx
##########
@@ -269,6 +320,40 @@ const FilterBar: React.FC<FiltersBarProps> = ({
{t('Apply')}
</Button>
</ActionButtons>
+ {isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS_SET) && (
+ <ActionButtons>
+ <FilterSet>
+ <StyledTitle>
+ <div>{t('Choose filters set')}</div>
+ <Select size="small" allowClear onChange={takeFiltersSet}>
Review comment:
Let's add a placeholder showing the available option count.
##########
File path:
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.tsx
##########
@@ -227,6 +274,10 @@ const FilterBar: React.FC<FiltersBarProps> = ({
});
};
+ const takeFiltersSet = (value: SelectValue) => {
+ dispatch(setFiltersState(filterSets[value as string]?.filtersState));
Review comment:
Could we rather do `String(value)` here instead of casting?
##########
File path: superset-frontend/src/dashboard/actions/nativeFilters.ts
##########
@@ -103,6 +103,20 @@ export interface SetExtraFormData {
currentState: CurrentFilterState;
}
+export const SAVE_FILTERS_SET = 'SAVE_FILTERS_SET';
Review comment:
I believe it should be "filter set"/"filter sets", not "filters
set"/"filters sets" (This comment applies to all occurrences of "filters set")
##########
File path:
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.tsx
##########
@@ -269,6 +320,40 @@ const FilterBar: React.FC<FiltersBarProps> = ({
{t('Apply')}
</Button>
</ActionButtons>
+ {isFeatureEnabled(FeatureFlag.DASHBOARD_NATIVE_FILTERS_SET) && (
+ <ActionButtons>
+ <FilterSet>
+ <StyledTitle>
+ <div>{t('Choose filters set')}</div>
+ <Select size="small" allowClear onChange={takeFiltersSet}>
+ {Object.values(filterSets).map(({ name, id }) => (
+ <Select.Option value={id}>{name}</Select.Option>
+ ))}
+ </Select>
+ </StyledTitle>
+ <StyledTitle>
+ <div>{t('Name')}</div>
+ <Input
+ size="small"
+ value={filtersSetName}
+ onChange={({
+ target: { value },
+ }: ChangeEvent<HTMLInputElement>) => {
+ setFiltersSetName(value);
+ }}
+ />
Review comment:
A placeholder here would also be nice, something like "Enter filter set
name"
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
