sadpandajoe commented on code in PR #44880:
URL: https://github.com/apache/superset/pull/44880#discussion_r4174345085
##########
superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx:
##########
@@ -525,8 +588,26 @@ export default function PluginFilterSelect(props:
PluginFilterSelectProps) {
});
updateDataMask(null);
- setSearch('');
+ settleSearch.cancel();
+ settleLikeValue.cancel();
+ // A search the user already ran left a server-side subset in ownState;
+ // publish the empty search so the cleared filter lists every option.
+ const {
+ searchAllOptions: currentSearchAllOptions,
+ dispatchDataMask: currentDispatchDataMask,
+ initialColtypeMap: currentInitialColtypeMap,
+ } = searchDispatchRef.current;
+ if (hasSearchedRef.current && currentSearchAllOptions) {
+ currentDispatchDataMask({
+ type: 'ownState',
+ ownState: { coltypeMap: currentInitialColtypeMap, search: '' },
+ });
Review Comment:
The Clear All tests use LIKE input with server search disabled, so they
would still pass if this empty-search publication were removed and the cleared
dropdown retained its old server-side subset. Could a regression settle an `ab`
search with an active selection, clear and acknowledge it, then exhaust timers
and assert the final mask has `ownState.search === ''`?
##########
superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx:
##########
@@ -564,32 +645,31 @@ export default function PluginFilterSelect(props:
PluginFilterSelectProps) {
updateDataMaskRef.current = updateDataMask;
}, [updateDataMask]);
- const debouncedLikeChange = useMemo(
- () =>
- debounce((text: string) => {
- if (text) {
- updateDataMaskRef.current([text]);
- } else {
- updateDataMaskRef.current(null);
- }
- }, Constants.SLOW_DEBOUNCE),
- [],
- );
+ // Any match-type change (including between two LIKE modes) discards the
+ // pending edit, so text typed under the previous mode is never replayed and
+ // re-entering the same text afterwards registers as a fresh edit.
+ useEffect(() => {
+ settleLikeValue.cancel();
+ setPendingLikeValue(null);
Review Comment:
With an unset default, typing `Jen` under Contains and switching to Starts
with before the 500 ms debounce expires leaves `Jen` displayed but no default
committed, so required-value validation still fails. `filterState.value` stays
`undefined`, so its sync effect does not clear the input; could discarding the
pending edit also clear the displayed text?
##########
superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx:
##########
@@ -200,11 +221,23 @@ export default function PluginFilterSelect(props:
PluginFilterSelectProps) {
const [likeInputValue, setLikeInputValue] = useState<string>(
filterState.value?.[0] != null ? String(filterState.value[0]) : '',
);
+ // null until the user types in the LIKE input; distinguishes a real edit
+ // from the clearAllTrigger reset below, which also clears likeInputValue
+ // but must not re-trigger a dispatch once the deferred value catches up.
+ const [pendingLikeValue, setPendingLikeValue] = useState<string |
null>(null);
+ const settleLikeValue = useMemo(
+ () => debounce(setPendingLikeValue, Constants.SLOW_DEBOUNCE),
+ [],
+ );
+ useEffect(() => () => settleLikeValue.cancel(), [settleLikeValue]);
useEffect(() => {
const externalValue =
filterState.value?.[0] != null ? String(filterState.value[0]) : '';
setLikeInputValue(externalValue);
+ // A parent-driven value change supersedes any settled edit, so re-entering
+ // the same text afterwards registers as a fresh edit.
+ setPendingLikeValue(null);
Review Comment:
The rerender test only changes a label while the value stays `undefined`, so
removing this invalidation would still pass while identical LIKE re-entry after
a parent reset breaks again. Could a regression settle `Jen`, clear the default
through the multi-select toggle, then re-enter `Jen` and assert a new committed
Contains mask after the debounce?
--
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]
