kgabryje commented on code in PR #33271:
URL: https://github.com/apache/superset/pull/33271#discussion_r2073776275
##########
superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx:
##########
@@ -352,8 +396,11 @@ export default function PluginFilterSelect(props:
PluginFilterSelectProps) {
validateStatus={filterState.validateStatus}
extra={formItemExtra}
>
- <StyledSpace $inverseSelection={inverseSelection}>
- {inverseSelection && (
+ <StyledSpace
+ $appSection={appSection}
+ $inverseSelection={inverseSelection}
Review Comment:
Could you remove the `$` from the props name? We don't use that convention
anywhere else
##########
superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx:
##########
@@ -158,6 +166,30 @@ export default function PluginFilterSelect(props:
PluginFilterSelectProps) {
: filterState?.excludeFilterValues,
);
+ const prevExcludeFilterValues = useRef(excludeFilterValues);
+
+ const hasOnlyOrientationChanged = useRef(false);
+
+ useEffect(() => {
+ // Get previous orientation for this specific filter
+ const previousOrientation = orientationMap.get(formData.nativeFilterId);
Review Comment:
why do we need it to be a map? Can't we simply preserve the previous value
with a ref? We also have a helper hook `usePrevious` for cases like this -
`const prevOrientation = usePrevious(filterBarOrientation)` (prevOrientation
will be undefined on the first render)
##########
superset-frontend/src/dataMask/reducer.ts:
##########
@@ -106,10 +114,27 @@ function updateDataMaskForFilterChanges(
});
filterChanges.modified.forEach((filter: Filter) => {
+ const existingFilter = draftDataMask[filter.id] as FilterWithExtaFromData;
+
+ // Check if targets are equal
+ const areTargetsEqual = isEqual(existingFilter?.targets, filter?.targets);
+
+ // Preserve state only if filter exists, has enableEmptyFilter=true and
targets match
+ const shouldPreserveState =
+ existingFilter &&
+ areTargetsEqual &&
+ (filter.controlValues?.enableEmptyFilter === true ||
Review Comment:
```suggestion
(filter.controlValues?.enableEmptyFilter ||
```
--
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]
