Skip to content

Fix feed fidget settings resetting when switching fidgets #1646

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Jhonattan2121 wants to merge 4 commits into
base: canary
Choose a base branch
from
+261 −58
Open

Fix feed fidget settings resetting when switching fidgets #1646

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
936303f
Fix feed fidget settings resetting when switching fidgets
Jhonattan2121 Dec 31, 2025
1c0f906
Enhance FidgetSettingsEditor and FidgetWrapper for improved filter va…
Jhonattan2121 Dec 31, 2025
e81b15d
Normalize filter settings and improve state management in FidgetSetti…
Jhonattan2121 Jan 1, 2026
c076fb8
Fix feed setting reset blocking other fidgets
Jhonattan2121 Jan 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
190 changes: 175 additions & 15 deletions src/common/components/organisms/FidgetSettingsEditor.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,22 @@ import {
tabTriggerClasses,
} from "@/common/lib/theme/helpers";
import { mergeClasses } from "@/common/lib/utils/mergeClasses";
import React, { useEffect, useMemo, useState } from "react";
import React, { useEffect, useMemo, useRef, useState } from "react";
import { FaCircleInfo, FaTrashCan } from "react-icons/fa6";
import BackArrowIcon from "../atoms/icons/BackArrow";
import { AnalyticsEvent } from "@/common/constants/analyticsEvents";
import { analytics } from "@/common/providers/AnalyticsProvider";
import { useUIColors } from "@/common/lib/hooks/useUIColors";
import { FeedType } from "@neynar/nodejs-sdk/build/api";
import { toast } from "sonner";
import { FilterType as FarcasterFilterType } from "@/fidgets/farcaster/Feed";

Comment on lines +28 to 29
Copy link

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The imports of FeedType and FarcasterFilterType suggest this component now has Farcaster-specific logic. The FidgetSettingsEditor is meant to be a generic component for editing any fidget's settings, but the validation logic (lines 177-191, 227-238, 309-332) is specific to Feed fidgets. This violates the single responsibility principle and creates tight coupling. Consider moving the Feed-specific validation logic to a separate validator that can be passed as a prop, or handle it in the FidgetWrapper or Feed component itself.

Suggested change
import { FilterType as FarcasterFilterType } from "@/fidgets/farcaster/Feed";
type FarcasterFilterType = string;

Copilot uses AI. Check for mistakes.
export type FidgetSettingsEditorProps = {
fidgetId: string;
readonly properties: FidgetProperties;
settings: FidgetSettings;
onSave: (settings: FidgetSettings, shouldUnselect?: boolean) => void;
onStateChange?: (settings: FidgetSettings) => void;
Copy link

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The onStateChange prop is defined as optional but is never passed from the FidgetWrapper component (lines 204-211). This means the callback is always undefined and the calls to onStateChange?.(...) at lines 248, 281, 302, and 329 will never execute. If this callback is not intended to be used yet, consider removing it to reduce complexity. If it is needed, it should be passed from the parent component.

Copilot uses AI. Check for mistakes.
unselect: () => void;
removeFidget: (fidgetId: string) => void;
};
Expand Down Expand Up @@ -132,13 +136,15 @@ export const FidgetSettingsGroup: React.FC<{
state: FidgetSettings;
setState: (state: FidgetSettings) => void;
onSave: (state: FidgetSettings) => void;
}> = ({ fields, state, setState, onSave, fidgetId }) => {
isActive?: () => boolean;
}> = ({ fields, state, setState, onSave, fidgetId, isActive }) => {
return (
<>
{fields.map((field, i) => {
const value =
(field.fieldName in state && state[field.fieldName]) || "";
const updateSettings = (partial: FidgetSettings) => {
if (isActive && !isActive()) return;
const data = { ...state, ...partial };
setState(data);
onSave(data);
Expand All @@ -150,6 +156,7 @@ export const FidgetSettingsGroup: React.FC<{
id={`${fidgetId}-${i}-${field.fieldName}`}
value={value}
onChange={(val) => {
if (isActive && !isActive()) return;
const data = {
...state,
[field.fieldName]: val,
Expand All @@ -167,27 +174,177 @@ export const FidgetSettingsGroup: React.FC<{
);
};

const hasFilterTarget = (settings: FidgetSettings) => {
const candidates = [
settings.users,
settings.username,
settings.channel,
settings.keyword,
];

return candidates.some(
(value) => typeof value === "string" && value.trim().length > 0,
);
};

const isInvalidFeedFilter = (settings: FidgetSettings) =>
settings.feedType === FeedType.Filter && !hasFilterTarget(settings);

const FILTER_SETTING_FIELDS: Array<keyof FidgetSettings> = [
"filterType",
"username",
"users",
"channel",
"keyword",
];

const hasValue = (value: unknown) =>
typeof value === "string" && value.trim().length > 0;

export const FidgetSettingsEditor: React.FC<FidgetSettingsEditorProps> = ({
fidgetId,
properties,
settings,
onSave,
onStateChange,
unselect,
removeFidget,
}) => {
const [state, setState] = useState<FidgetSettings>(settings);
const fillWithDefaults = (input: FidgetSettings) =>
properties.fields.reduce((acc, field) => {
const value =
input && typeof input === "object"
? (input as any)[field.fieldName]
: undefined;
const hasValue =
value !== undefined &&
value !== null &&
(typeof value !== "string" || value.trim() !== "");
Comment on lines +219 to +222
Copy link

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hasValue function defined at lines 201-202 shadows the hasValue variable defined inline at lines 219-222 within the fillWithDefaults function. While they perform similar logic, having two different implementations with the same name can be confusing and error-prone. Consider renaming one of them (e.g., hasNonEmptyValue for the outer function or using the outer function consistently).

Copilot uses AI. Check for mistakes.
acc[field.fieldName] = hasValue ? value : field.default ?? "";
Comment on lines +214 to +223
Copy link

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using (input as any)[field.fieldName] bypasses TypeScript's type checking. This is the same type safety issue found in FidgetWrapper.tsx. Consider using a type-safe property access method instead of casting to any.

Suggested change
properties.fields.reduce((acc, field) => {
const value =
input && typeof input === "object"
? (input as any)[field.fieldName]
: undefined;
const hasValue =
value !== undefined &&
value !== null &&
(typeof value !== "string" || value.trim() !== "");
acc[field.fieldName] = hasValue ? value : field.default ?? "";
properties.fields.reduce<FidgetSettings>((acc, field) => {
const key = field.fieldName as keyof FidgetSettings;
const value =
input && typeof input === "object"
? input[key]
: undefined;
const hasExistingValue =
value !== undefined &&
value !== null &&
(typeof value !== "string" || value.trim() !== "");
acc[key] = (hasExistingValue ? value : field.default ?? "") as FidgetSettings[typeof key];

Copilot uses AI. Check for mistakes.
return acc;
}, {} as FidgetSettings);

const normalizeFilterType = (input: FidgetSettings) => {
if (input.feedType !== FeedType.Filter) return input;
const allowed = new Set([
FarcasterFilterType.Users,
FarcasterFilterType.Channel,
FarcasterFilterType.Keyword,
]);
if (!allowed.has(input.filterType)) {
return { ...input, filterType: FarcasterFilterType.Users };
}
return input;
};

const normalizedSettings = useMemo(
() => normalizeFilterType(fillWithDefaults(settings)),
[settings, properties.fields],
);
Comment on lines +240 to +243
Copy link

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The useMemo dependencies are missing the function definitions. The fillWithDefaults and normalizeFilterType functions are defined inline in the component body but are referenced in the normalizedSettings useMemo. Since properties.fields is in the dependency array, these functions will be recreated on every render where properties.fields changes, which could cause the memoization to be ineffective. Consider moving these functions outside the component or adding them to useCallback with proper dependencies.

Copilot uses AI. Check for mistakes.
const [state, setState] = useState<FidgetSettings>(normalizedSettings);
const activeIdRef = useRef(fidgetId);
const uiColors = useUIColors();
const notifyStateChange = (nextState: FidgetSettings) => {
onStateChange?.(normalizeFilterType(fillWithDefaults(nextState)));
};
const setStateWithNotify = (nextState: FidgetSettings) => {
setState(nextState);
notifyStateChange(nextState);
};

const appliedSettingsSignatureRef = useRef<string>(
JSON.stringify(normalizedSettings),
);
const pendingSaveSignatureRef = useRef<string | null>(null);
const lastStateRef = useRef<FidgetSettings>(state);
const lastFidgetIdRef = useRef(fidgetId);

useEffect(() => {
lastStateRef.current = state;
}, [state]);

useEffect(() => {
setState(settings);
}, [settings, fidgetId]);
if (lastFidgetIdRef.current === fidgetId) return;
lastFidgetIdRef.current = fidgetId;
appliedSettingsSignatureRef.current = "";
pendingSaveSignatureRef.current = null;
lastStateRef.current = normalizedSettings;
}, [fidgetId, normalizedSettings]);

useEffect(() => {
const signature = JSON.stringify(normalizedSettings);

if (pendingSaveSignatureRef.current === signature) {
pendingSaveSignatureRef.current = null;
appliedSettingsSignatureRef.current = signature;
setState(normalizedSettings);
onStateChange?.(normalizedSettings);
return;
}

const hasIncomingFilters = FILTER_SETTING_FIELDS.some((field) =>
hasValue(normalizedSettings[field]),
);
const hasLocalFilters = FILTER_SETTING_FIELDS.some((field) =>
hasValue(lastStateRef.current[field]),
);

if (!hasIncomingFilters && hasLocalFilters) {
return;
}

if (appliedSettingsSignatureRef.current === signature) {
return;
}

appliedSettingsSignatureRef.current = signature;
setState(normalizedSettings);
onStateChange?.(normalizedSettings);
}, [normalizedSettings]);
Comment on lines +274 to +303
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Missing onStateChange in dependency array may cause stale closure.

Line 303 shows the effect depends only on [normalizedSettings], but onStateChange is called at lines 281 and 302. If onStateChange changes identity between renders, the effect will call a stale version.

🔎 Suggested fix 
-  }, [normalizedSettings]);
+  }, [normalizedSettings, onStateChange]);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
useEffect(() => {
const signature = JSON.stringify(normalizedSettings);
if (pendingSaveSignatureRef.current === signature) {
pendingSaveSignatureRef.current = null;
appliedSettingsSignatureRef.current = signature;
setState(normalizedSettings);
onStateChange?.(normalizedSettings);
return;
}
const hasIncomingFilters = FILTER_SETTING_FIELDS.some((field) =>
hasValue(normalizedSettings[field]),
);
const hasLocalFilters = FILTER_SETTING_FIELDS.some((field) =>
hasValue(lastStateRef.current[field]),
);
if (!hasIncomingFilters && hasLocalFilters) {
return;
}
if (appliedSettingsSignatureRef.current === signature) {
return;
}
appliedSettingsSignatureRef.current = signature;
setState(normalizedSettings);
onStateChange?.(normalizedSettings);
}, [normalizedSettings]);
useEffect(() => {
const signature = JSON.stringify(normalizedSettings);
if (pendingSaveSignatureRef.current === signature) {
pendingSaveSignatureRef.current = null;
appliedSettingsSignatureRef.current = signature;
setState(normalizedSettings);
onStateChange?.(normalizedSettings);
return;
}
const hasIncomingFilters = FILTER_SETTING_FIELDS.some((field) =>
hasValue(normalizedSettings[field]),
);
const hasLocalFilters = FILTER_SETTING_FIELDS.some((field) =>
hasValue(lastStateRef.current[field]),
);
if (!hasIncomingFilters && hasLocalFilters) {
return;
}
if (appliedSettingsSignatureRef.current === signature) {
return;
}
appliedSettingsSignatureRef.current = signature;
setState(normalizedSettings);
onStateChange?.(normalizedSettings);
}, [normalizedSettings, onStateChange]);
🤖 Prompt for AI Agents 
In src/common/components/organisms/FidgetSettingsEditor.tsx around lines 274 to
303, the effect depends only on normalizedSettings but calls onStateChange at
lines 281 and 302, so add onStateChange to the effect dependency array to avoid
a stale closure; if the parent provides a non-stable callback, either memoize
onStateChange with useCallback in the parent or capture the latest callback in a
ref (e.g., onStateChangeRef.current) and call that inside the effect, then
update the dependency array accordingly and run the linter to ensure no missing
dependencies remain.

Comment on lines +274 to +303
Copy link

Copilot AI Jan 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The useEffect has a dependency on normalizedSettings which includes fidgetId but also depends on logic that recreates on every change. When the fidgetId changes in line 267, this effect at line 274 will also run because normalizedSettings changes. This could lead to the effect running twice when switching fidgets - once from the fidgetId change effect (lines 266-272) and once from this effect. The logic in lines 285-294 tries to handle this, but the dual effect execution could still cause subtle timing issues or unnecessary work.

Copilot uses AI. Check for mistakes.

useEffect(() => {
activeIdRef.current = fidgetId;
}, [fidgetId]);

const saveWithValidation = (
nextState: FidgetSettings,
shouldUnselect?: boolean,
showAlert?: boolean,
) => {
const filledState = normalizeFilterType(fillWithDefaults(nextState));

if (isInvalidFeedFilter(filledState)) {
if (showAlert) {
toast.error(
"Add a user/FID, channel, or keyword before saving a Filter feed.",
);
}
return false;
}

const signature = JSON.stringify(filledState);
pendingSaveSignatureRef.current = signature;
appliedSettingsSignatureRef.current = signature;
setState(filledState);
onStateChange?.(filledState);
onSave(filledState, shouldUnselect);
return true;
};

const safeOnSave = (nextState: FidgetSettings) => {
if (activeIdRef.current !== fidgetId) return;
saveWithValidation(nextState);
};

const _onSave = (e) => {
e.preventDefault();
onSave(state, true);
analytics.track(AnalyticsEvent.EDIT_FIDGET, {
fidgetType: properties.fidgetName,
});
if (activeIdRef.current !== fidgetId) return;
const didSave = saveWithValidation(state, true, true);
if (didSave) {
analytics.track(AnalyticsEvent.EDIT_FIDGET, {
fidgetType: properties.fidgetName,
});
}
};

const groupedFields = useMemo(
Expand Down Expand Up @@ -232,8 +389,9 @@ export const FidgetSettingsEditor: React.FC<FidgetSettingsEditorProps> = ({
fidgetId={fidgetId}
fields={groupedFields.settings}
state={state}
setState={setState}
onSave={onSave}
setState={setStateWithNotify}
onSave={safeOnSave}
isActive={() => activeIdRef.current === fidgetId}
/>
</TabsContent>
{groupedFields.style.length > 0 && (
Expand All @@ -242,8 +400,9 @@ export const FidgetSettingsEditor: React.FC<FidgetSettingsEditorProps> = ({
fidgetId={fidgetId}
fields={groupedFields.style}
state={state}
setState={setState}
onSave={onSave}
setState={setStateWithNotify}
onSave={safeOnSave}
isActive={() => activeIdRef.current === fidgetId}
/>
</TabsContent>
)}
Expand All @@ -253,8 +412,9 @@ export const FidgetSettingsEditor: React.FC<FidgetSettingsEditorProps> = ({
fidgetId={fidgetId}
fields={groupedFields.code}
state={state}
setState={setState}
onSave={onSave}
setState={setStateWithNotify}
onSave={safeOnSave}
isActive={() => activeIdRef.current === fidgetId}
/>
</TabsContent>
)}
Expand Down
Loading