villebro commented on a change in pull request #13529:
URL: https://github.com/apache/superset/pull/13529#discussion_r590430602
##########
File path:
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterBar.tsx
##########
@@ -212,11 +208,10 @@ const FilterBar: React.FC<FiltersBarProps> = ({
if (isInitialized) {
return;
}
- const areFiltersInitialized = filtersArray.every(filterConfig =>
- areObjectsEqual(
- filterConfig.defaultValue,
+ const areFiltersInitialized = filtersArray.every(
+ filterConfig =>
+ filterConfig.defaultValue ===
filterData[filterConfig.id]?.currentState?.value,
- ),
Review comment:
This is probably a leftover from before the previous PR (the change
should be reverted)
##########
File path:
superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterSets/FilterSets.tsx
##########
@@ -66,27 +77,50 @@ type FilterSetsProps = {
const DEFAULT_FILTER_SET_NAME = t('New filter set');
const FilterSets: React.FC<FilterSetsProps> = ({
+ filterData,
disabled,
onFilterSelectionChange,
- dataMaskState,
}) => {
const dispatch = useDispatch();
const [filterSetName, setFilterSetName] = useState(DEFAULT_FILTER_SET_NAME);
const [editMode, setEditMode] = useState(false);
+ const dataMaskApplied = useDataMask();
const filterSets = useFilterSets();
const filterSetsArray = Object.values(filterSets);
- const dataMask = useDataMask();
const filters = Object.values(useFilters());
const [selectedFiltersSetId, setSelectedFiltersSetId] = useState<
string | null
>(null);
- const takeFilterSet = (value: string) => {
- setSelectedFiltersSetId(value);
- if (!value) {
+ useEffect(() => {
+ const foundFilterSet = filterSetsArray.find(({ dataMask }) => {
+ if (dataMask?.nativeFilters) {
+ return Object.values(dataMask?.nativeFilters).every(
+ filterFromFilterSet => {
+ let currentValueFromFiltersTab =
+ dataMaskApplied[filterFromFilterSet.id]?.currentState ?? {};
+ if (filterData[filterFromFilterSet.id]) {
+ currentValueFromFiltersTab =
+ filterData[filterFromFilterSet.id]?.currentState;
+ }
+ return areObjectsEqual(
+ filterFromFilterSet.currentState ?? {},
+ currentValueFromFiltersTab,
+ );
+ },
+ );
+ }
+ return false;
+ });
+ setSelectedFiltersSetId(foundFilterSet?.id ?? null);
+ }, [dataMaskApplied, filterData, filterSetsArray]);
Review comment:
This is pretty heavy reading. I know this is a pretty loaded context,
but I would urge paying special attention to naming to avoid ambiguity; for
instance, `filterData` can mean very many things in this context. Also, it
isn't directly obvious in this context that `filterSetsArray` is in fact
`Object.values(filterSets)` - a more appropriate name could be
`filterSetFilterValues` to convey that it's the state from the filter sets.
----------------------------------------------------------------
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]