Skip to content

Commit b05f8f8

Browse files
zubronzubron
authored
Fix UI showing wrong field removed when deleting metadata field (#9669)
Fixes bug where deleting a metadata key-value pair appeared to remove the last field in the list instead of the one that had been selected. The issue was caused by using array indices as React keys, which caused DOM element reuse when fields were removed. This change extracts the `MetadataFields` component into its own file and changes the component to be controlled rather than uncontrolled by setting `value` instead of `defaultValue` in the inputs. Also remove `useCallback` from `onChangeKey`, `onChangeValue`, and `onRemovePair` as they return new closures on each render, making memoization redundant.
1 parent ae4e31c commit b05f8f8

File tree

5 files changed

+72
-65
lines changed

5 files changed

+72
-65
lines changed

‎webui/src/lib/components/repository/changes.jsx‎

Lines changed: 1 addition & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,18 @@
1-
import React, {useCallback, useEffect, useState} from "react";
1+
import React, {useEffect, useState} from "react";
22

33
import {
44
ClockIcon, FileDirectoryFillIcon, FoldDownIcon, FoldUpIcon,
5-
PlusIcon,
6-
XIcon
75
} from "@primer/octicons-react";
86

97
import {useAPIWithPagination} from "../../hooks/api";
108
import {useExpandCollapseDirs} from "../../hooks/useExpandCollapseDirs";
119
import {AlertError, TooltipButton} from "../controls";
1210
import {ObjectsDiff} from "./ObjectsDiff";
1311
import {ObjectTreeEntryRow, PrefixTreeEntryRow} from "./treeRows";
14-
import Button from "react-bootstrap/Button";
1512
import Card from "react-bootstrap/Card";
1613
import Table from "react-bootstrap/Table";
1714
import Alert from "react-bootstrap/Alert";
1815
import {refs} from "../../api";
19-
import Form from "react-bootstrap/Form";
20-
import Row from "react-bootstrap/Row";
21-
import Col from "react-bootstrap/Col";
2216

2317
/**
2418
* Tree item is a node in the tree view. It can be expanded to multiple TreeEntryRow:
@@ -253,58 +247,3 @@ export const ChangesTreeContainer = ({results, delimiter, uriNavigator,
253247
export const defaultGetMoreChanges = (repo, leftRefId, rightRefId, delimiter) => (afterUpdated, path, useDelimiter= true, amount = -1) => {
254248
return refs.diff(repo.id, leftRefId, rightRefId, afterUpdated, path, useDelimiter ? delimiter : "", amount > 0 ? amount : undefined);
255249
};
256-
257-
export const MetadataFields = ({ metadataFields, setMetadataFields, ...rest}) => {
258-
const onChangeKey = useCallback((i) => {
259-
return e => {
260-
const key = e.currentTarget.value;
261-
setMetadataFields(prev => [...prev.slice(0,i), {...prev[i], key}, ...prev.slice(i+1)]);
262-
e.preventDefault()
263-
};
264-
}, [setMetadataFields]);
265-
266-
const onChangeValue = useCallback((i) => {
267-
return e => {
268-
const value = e.currentTarget.value;
269-
setMetadataFields(prev => [...prev.slice(0,i), {...prev[i], value}, ...prev.slice(i+1)]);
270-
};
271-
}, [setMetadataFields]);
272-
273-
const onRemovePair = useCallback((i) => {
274-
return () => setMetadataFields(prev => [...prev.slice(0, i), ...prev.slice(i + 1)])
275-
}, [setMetadataFields])
276-
277-
const onAddPair = useCallback(() => {
278-
setMetadataFields(prev => [...prev, {key: "", value: ""}])
279-
}, [setMetadataFields])
280-
281-
return (
282-
<div className="mt-3 mb-3" {...rest}>
283-
{metadataFields.map((f, i) => {
284-
return (
285-
<Form.Group key={`commit-metadata-field-${i}`} className="mb-3">
286-
<Row>
287-
<Col md={{span: 5}}>
288-
<Form.Control type="text" placeholder="Key" defaultValue={f.key} onChange={onChangeKey(i)}/>
289-
</Col>
290-
<Col md={{span: 5}}>
291-
<Form.Control type="text" placeholder="Value" defaultValue={f.value} onChange={onChangeValue(i)}/>
292-
</Col>
293-
<Col md={{span: 1}}>
294-
<Form.Text>
295-
<Button size="sm" variant="secondary" onClick={onRemovePair(i)}>
296-
<XIcon/>
297-
</Button>
298-
</Form.Text>
299-
</Col>
300-
</Row>
301-
</Form.Group>
302-
)
303-
})}
304-
<Button onClick={onAddPair} size="sm" variant="secondary">
305-
<PlusIcon/>{' '}
306-
Add Metadata field
307-
</Button>
308-
</div>
309-
)
310-
}

‎webui/src/lib/components/repository/compareBranchesActionBar.jsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import React, {useCallback, useRef, useState} from "react";
22
import {refs as refsAPI} from "../../../lib/api";
33
import {RefTypeBranch} from "../../../constants";
44
import {ActionGroup, ActionsBar, AlertError, RefreshButton} from "../controls";
5-
import {MetadataFields} from "./changes";
5+
import {MetadataFields} from "./metadata";
66
import {GitMergeIcon} from "@primer/octicons-react";
77
import Button from "react-bootstrap/Button";
88
import Modal from "react-bootstrap/Modal";

‎webui/src/lib/components/repository/metadata.jsx‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
import React from "react";
2+
3+
import {PlusIcon, XIcon} from "@primer/octicons-react";
4+
import Button from "react-bootstrap/Button";
5+
import Form from "react-bootstrap/Form";
6+
import Row from "react-bootstrap/Row";
7+
import Col from "react-bootstrap/Col";
8+
9+
/**
10+
* MetadataFields is a component that allows the user to add/remove key-value pairs of metadata.
11+
* @param {Array<{key: string, value: string}>} metadataFields - current metadata fields to display
12+
* @param {Function} setMetadataFields - callback to update the metadata fields
13+
* @param rest - any other props to pass to the component
14+
*/
15+
export const MetadataFields = ({ metadataFields, setMetadataFields, ...rest}) => {
16+
const onChangeKey = (i) => {
17+
return e => {
18+
const newKey = e.currentTarget.value;
19+
setMetadataFields(prev => [...prev.slice(0,i), {...prev[i], key: newKey}, ...prev.slice(i+1)]);
20+
};
21+
};
22+
23+
const onChangeValue = (i) => {
24+
return e => {
25+
const newValue = e.currentTarget.value;
26+
setMetadataFields(prev => [...prev.slice(0,i), {...prev[i], value: newValue}, ...prev.slice(i+1)]);
27+
};
28+
};
29+
30+
const onRemoveKeyValue = (i) => {
31+
return () => setMetadataFields(prev => [...prev.slice(0, i), ...prev.slice(i + 1)]);
32+
};
33+
34+
const onAddKeyValue = () => {
35+
setMetadataFields(prev => [...prev, {key: "", value: ""}])
36+
};
37+
38+
return (
39+
<div className="mt-3 mb-3" {...rest}>
40+
{metadataFields.map((f, i) => {
41+
return (
42+
<Form.Group key={`commit-metadata-field-${i}`} className="mb-3">
43+
<Row>
44+
<Col md={{span: 5}}>
45+
<Form.Control type="text" placeholder="Key" value={f.key} onChange={onChangeKey(i)}/>
46+
</Col>
47+
<Col md={{span: 5}}>
48+
<Form.Control type="text" placeholder="Value" value={f.value} onChange={onChangeValue(i)}/>
49+
</Col>
50+
<Col md={{span: 1}}>
51+
<Form.Text>
52+
<Button size="sm" variant="secondary" onClick={onRemoveKeyValue(i)}>
53+
<XIcon/>
54+
</Button>
55+
</Form.Text>
56+
</Col>
57+
</Row>
58+
</Form.Group>
59+
)
60+
})}
61+
<Button onClick={onAddKeyValue} size="sm" variant="secondary">
62+
<PlusIcon/>{' '}
63+
Add Metadata field
64+
</Button>
65+
</div>
66+
)
67+
}

‎webui/src/pages/repositories/repository/objects.jsx‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,8 @@ import { getRepoStorageConfig } from "./utils";
4444
import {useDropzone} from "react-dropzone";
4545
import pMap from "p-map";
4646
import {formatAlertText} from "../../../lib/components/repository/errors";
47-
import {ChangesTreeContainer, MetadataFields} from "../../../lib/components/repository/changes";
47+
import {ChangesTreeContainer} from "../../../lib/components/repository/changes";
48+
import {MetadataFields} from "../../../lib/components/repository/metadata";
4849
import {ConfirmationModal} from "../../../lib/components/modals";
4950
import { Link } from "../../../lib/components/nav";
5051
import Card from "react-bootstrap/Card";

‎webui/src/pages/repositories/services/import_data.jsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import React, {useState} from "react";
77
import Button from "react-bootstrap/Button";
88
import Alert from "react-bootstrap/Alert";
99
import Form from "react-bootstrap/Form";
10-
import {MetadataFields} from "../../../lib/components/repository/changes";
10+
import {MetadataFields} from "../../../lib/components/repository/metadata";
1111

1212
const ImportPhase = {
1313
NotStarted: 0,

0 commit comments

Comments
 (0)