Skip to content

group mutual texts with or without id #95

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
mir-dvlp wants to merge 1 commit into
base: main
Choose a base branch
from
Open

group mutual texts with or without id #95

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 5 additions & 0 deletions src/ui/components/NodeList/NodeRow.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { HtmlText } from "../Shared/HtmlText";

type Props = {
node: PartialNodeInfo;
duplicatesCount?: number;
action?: ComponentChildren;
keyComponent?: ComponentChildren;
nsComponent?: ComponentChildren;
Expand All @@ -18,6 +19,7 @@ type Props = {

export const NodeRow = ({
node,
duplicatesCount,
action,
keyComponent,
nsComponent,
Expand Down Expand Up @@ -64,6 +66,9 @@ export const NodeRow = ({
{Object.keys(node.paramsValues ?? {}).length > 0 && (
<Badge>parameters</Badge>
)}
{typeof duplicatesCount === "number" && duplicatesCount > 1 && (
<Badge>{`${duplicatesCount}x`}</Badge>
)}
</div>
)}
<div className={styles.action} data-cy="general_node_list_row_action">
Expand Down
63 changes: 32 additions & 31 deletions src/ui/views/Connect/Connect.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,16 @@ import { useAllTranslations } from "@/ui/hooks/useAllTranslations";

type Props = RouteParam<"connect">;

export const Connect = ({ node }: Props) => {
export const Connect = ({ nodes }: Props) => {
const { setRoute } = useGlobalActions();
const config = useGlobalState((c) => c.config);

const language = useGlobalState((c) => c.config?.language);

const [search, setSearch] = useState(node.key || node.characters);
const primaryNode = nodes[0];
const [search, setSearch] = useState(
primaryNode?.key || primaryNode?.characters || ""
);

const [debouncedSearch] = useDebounce(search, 1000);

Expand Down Expand Up @@ -59,6 +62,10 @@ export const Connect = ({ node }: Props) => {
ns: string | undefined,
translation: string | undefined
) => {
if (nodes.length === 0) {
setRoute("index");
return;
}
if (
!allTranslationsLoadable.isLoading &&
allTranslationsLoadable.translationsData == null
Expand All @@ -72,46 +79,40 @@ export const Connect = ({ node }: Props) => {
if (tolgeeTranslation) {
translation = tolgeeTranslation.translation;
await setNodesDataMutation.mutateAsync({
nodes: [
{
...node,
translation: tolgeeTranslation.translation || node.characters,
isPlural: tolgeeTranslation.keyIsPlural,
pluralParamValue: tolgeeTranslation.keyPluralArgName,
key,
ns: ns || "",
connected: true,
},
],
nodes: nodes.map((node) => ({
...node,
translation: tolgeeTranslation.translation || node.characters,
isPlural: tolgeeTranslation.keyIsPlural,
pluralParamValue: tolgeeTranslation.keyPluralArgName,
key,
ns: ns || "",
connected: true,
})),
});
setRoute("index");
return;
}
}
await setNodesDataMutation.mutateAsync({
nodes: [
{
...node,
translation: translation || node.characters,
key,
ns: ns || "",
connected: true,
},
],
nodes: nodes.map((node) => ({
...node,
translation: translation || node.characters,
key,
ns: ns || "",
connected: true,
})),
});
setRoute("index");
};

const handleRemoveConnection = async () => {
await setNodesDataMutation.mutateAsync({
nodes: [
{
...node,
key: "",
ns: undefined,
connected: false,
},
],
nodes: nodes.map((node) => ({
...node,
key: "",
ns: undefined,
connected: false,
})),
});
setRoute("index");
};
Expand Down Expand Up @@ -158,7 +159,7 @@ export const Connect = ({ node }: Props) => {
))}
</div>
<ActionsBottom>
{node.connected && (
{primaryNode?.connected && (
<Button secondary onClick={handleRemoveConnection}>
Remove connection
</Button>
Expand Down
46 changes: 38 additions & 8 deletions src/ui/views/Index/Index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import styles from "./Index.css";
import { ListItem } from "./ListItem";
import { useEditorMode } from "../../hooks/useEditorMode";
import { useHasNamespacesEnabled } from "../../hooks/useHasNamespacesEnabled";
import { NodeInfo } from "@/types";

export const Index = () => {
const selectionLoadable = useSelectedNodes();
Expand Down Expand Up @@ -93,6 +94,31 @@ export const Index = () => {

const nothingSelected = !selectionLoadable.data?.somethingSelected;

const groupedSelection = useMemo(() => {
const groups = new Map<string, NodeInfo[]>();
selection.forEach((node) => {
const textKey = node.characters.trim();
const groupKey = node.connected
? `connected:${node.key}::${node.ns ?? ""}::${textKey}`
: `unconnected:${textKey}`;
Comment on lines +97 to +103
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 24, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Don't group whitespace-distinct texts together.

Using trim() here makes "Hello" and "Hello " share one row, but the rest of the pipeline still treats characters as exact text. That lets bulk edits/connects fan out across nodes that can still conflict on push. Group on the raw characters value, or normalize whitespace consistently everywhere.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/ui/views/Index/Index.tsx` around lines 97 - 103, The grouping uses
node.characters.trim() which merges distinct-whitespace text nodes; change
groupedSelection's grouping to use the raw node.characters (no trim) so groupKey
is built from node.characters (and existing connected branch
`connected:${node.key}::${node.ns ?? ""}::${node.characters}`) to ensure
grouping matches the rest of the pipeline that expects exact characters; update
the useMemo grouping logic that references selection and node.characters
accordingly.

const group = groups.get(groupKey);
if (group) {
group.push(node);
} else {
groups.set(groupKey, [node]);
}
});

return Array.from(groups.values())
.map((nodes, index) => ({
id: `group-${index}-${nodes[0]?.id ?? ""}`,
key: nodes[0]?.characters.trim() ?? "",
nodes,
duplicatesCount: nodes.length,
}))
.sort((a, b) => a.key.localeCompare(b.key));
}, [selection]);

const handleLanguageChange = (lang: string) => {
if (lang !== language) {
setRoute("pull", { lang });
Expand All @@ -102,10 +128,12 @@ export const Index = () => {
const defaultNamespace = useGlobalState((c) => c.config?.namespace);

const handlePush = () => {
const subjectNodes = selection.map((node) => ({
...node,
ns: node.ns ?? defaultNamespace,
}));
const subjectNodes = groupedSelection
.flatMap((group) => group.nodes)
.map((node) => ({
...node,
ns: node.ns ?? defaultNamespace,
}));
const conflicts = getConflictingNodes(subjectNodes);
if (conflicts.length > 0) {
const keys = Array.from(new Set(conflicts.map((n) => n.key)));
Expand All @@ -131,7 +159,7 @@ export const Index = () => {

useEffect(() => {
setError(undefined);
}, [selection]);
}, [groupedSelection]);

const editorMode = useEditorMode();

Expand Down Expand Up @@ -235,11 +263,13 @@ export const Index = () => {
</Container>
) : (
<NodeList
items={selection}
row={(node) => (
items={groupedSelection}
row={(group) => (
<ListItem
hasNamespacesEnabled={hasNamespacesEnabled}
node={node}
node={group.nodes[0]}
groupedNodes={group.nodes}
duplicatesCount={group.duplicatesCount}
loadedNamespaces={allAvailableNamespaces.map((name) => ({
name,
}))}
Expand Down
68 changes: 54 additions & 14 deletions src/ui/views/Index/ListItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,29 @@

type Props = {
node: NodeInfo;
groupedNodes?: NodeInfo[];
duplicatesCount?: number;
loadedNamespaces: UsedNamespaceModel[] | undefined;
hasNamespacesEnabled: boolean;
onRefreshNamespaces?: () => void;
};

export const ListItem = ({
node,
groupedNodes,
duplicatesCount,
loadedNamespaces,
hasNamespacesEnabled,
onRefreshNamespaces,
}: Props) => {
const nodeId = node?.id ?? "";
const effectiveNodes = useMemo(
() =>

Check failure on line 37 in src/ui/views/Index/ListItem.tsx

View workflow job for this annotation

GitHub Actions / Static check 🪲 

Replace `⏎······groupedNodes·&&·groupedNodes.length·>·0⏎········?·groupedNodes⏎········:·[node]` with `·(groupedNodes·&&·groupedNodes.length·>·0·?·groupedNodes·:·[node])`
groupedNodes && groupedNodes.length > 0
? groupedNodes
: [node],
[groupedNodes, node]
);
Comment on lines +36 to +42
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 24, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Avoid bulk-rewriting grouped nodes on initial render.

With the new grouping in src/ui/views/Index/Index.tsx, unconnected duplicates can share a row even when their draft key/ns values already differ. This effect will then overwrite every node in the group to match the first node's keyName after the debounce, without any user edit in this row. Please gate the fan-out behind an explicit edit, or only bulk-sync groups whose current key/ns already agree.

Also applies to: 75-92

🧰 Tools
🪛 GitHub Check: Static check 🪲

[failure] 37-37:
Replace ⏎······groupedNodes·&&·groupedNodes.length·>·0⏎········?·groupedNodes⏎········:·[node] with ·(groupedNodes·&&·groupedNodes.length·>·0·?·groupedNodes·:·[node])

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/ui/views/Index/ListItem.tsx` around lines 36 - 42, The current useMemo
that creates effectiveNodes from groupedNodes and node causes silent
bulk-overwrites of all members in a group when debounced edits occur; change the
logic in ListItem so that groupedNodes are only expanded into per-row edits when
the user explicitly edits this row (e.g., an edit flag/state set in your
onChange/onBlur handlers) or only bulk-sync when every member in groupedNodes
already has identical key and ns values to node before applying the fan-out;
update the effectiveNodes computation and any code paths that currently write
back to grouped members (the groupedNodes, node and effectiveNodes logic and the
edit handlers used in this component) to gate the fan-out behind that
explicit-edit flag or the equality check.

const tolgeeConfig = useGlobalState((c) => c.config);

const prefilledKey = usePrefilledKey(
Expand Down Expand Up @@ -62,15 +73,26 @@

// Debounced mutation: only update Figma nodes after user stops typing
useEffect(() => {
if (!node.connected && debouncedKeyName !== (node.key || "")) {
const hasConnected = effectiveNodes.some((current) => current.connected);
if (hasConnected) {
return;
}
const shouldUpdate = effectiveNodes.some(
(current) => debouncedKeyName !== (current.key || "")
);
if (shouldUpdate) {
setNodesDataMutation.mutate({
nodes: [{ ...node, key: debouncedKeyName, ns: namespace }],
nodes: effectiveNodes.map((current) => ({
...current,
key: debouncedKeyName,
ns: namespace,
})),
});
}
}, [debouncedKeyName, namespace, node, node.connected]);
}, [debouncedKeyName, namespace, effectiveNodes]);

const handleConnect = (node: NodeInfo) => {
setRoute("connect", { node });
const handleConnect = () => {
setRoute("connect", { nodes: effectiveNodes });
};

const namespaces = useMemo(
Expand All @@ -89,25 +111,43 @@
};

useEffect(() => {
if (!node.connected && keyName && namespace !== node.ns) {
const hasConnected = effectiveNodes.some((current) => current.connected);
if (hasConnected || !keyName) {
return;
}
const shouldUpdate = effectiveNodes.some(
(current) => current.ns !== namespace
);
if (shouldUpdate) {
setNodesDataMutation.mutate({
nodes: [{ ...node, key: keyName, ns: namespace }],
nodes: effectiveNodes.map((current) => ({
...current,
key: keyName,
ns: namespace,
})),
});
}
}, [namespace, node.connected]);
}, [namespace, keyName, effectiveNodes]);

const handleNsChange = (node: NodeInfo) => (value: string) => {
const handleNsChange = () => (value: string) => {
setNamespace(value);
setNodesDataMutation.mutate({
nodes: [{ ...node, key: keyName, ns: value }],
nodes: effectiveNodes.map((current) => ({
...current,
key: keyName,
ns: value,
})),
});
effectiveNodes.forEach((current) => {
current.key = keyName;
current.ns = value;
});
Comment on lines +132 to 144
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 24, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Don't mutate effectiveNodes in place.

These objects come from props/selection state. Rewriting current.key and current.ns locally makes the UI look updated before setNodesDataMutation succeeds, and it can leak stale values into later grouping/effect logic. Keep the array immutable and suppress the follow-up effect with local state/ref instead.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/ui/views/Index/ListItem.tsx` around lines 132 - 144, handleNsChange
currently mutates the objects in effectiveNodes in place; instead, avoid
in-place mutation by creating a new array of node objects and passing that to
setNodesDataMutation.mutate (use effectiveNodes.map to return {...current, key:
keyName, ns: value}) and call setNamespace(value) as before; do not assign to
current.key/current.ns, and if you need to suppress any follow-up effect that
reacts to effectiveNodes updates, track the user-intended namespace change in
local component state or a ref (e.g., a pendingNs ref) so downstream
grouping/effect logic ignores transient UI-only changes until the mutation
succeeds.

node.key = keyName;
node.ns = value;
};

return (
<NodeRow
node={node}
duplicatesCount={duplicatesCount}
keyComponent={
!node.connected && (
<KeyInput value={keyName || ""} onChange={handleKeyChange()} />
Expand All @@ -119,7 +159,7 @@
<NamespaceSelect
value={namespace ?? ""}
namespaces={namespaces}
onChange={handleNsChange(node)}
onChange={handleNsChange()}
onRefresh={onRefreshNamespaces}
/>
)
Expand All @@ -134,7 +174,7 @@
? "Connect to existing key"
: "Edit key connection"
}
onClick={() => handleConnect(node)}
onClick={handleConnect}
className={styles.connectButton}
>
{node.connected ? (
Expand Down
2 changes: 1 addition & 1 deletion src/ui/views/routes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ export type Route =
| ["push"]
| ["string_details", { node: NodeInfo }]
| ["pull", { lang: string }]
| ["connect", { node: NodeInfo }]
| ["connect", { nodes: NodeInfo[] }]
| ["create_copy"];

export type RouteKey = Route[0];
Expand Down
Loading