Skip to content

Commit

Permalink
lib: Fix DynamicListForm state confusion on removals
Browse files Browse the repository at this point in the history
When deleting an item, set it to `undefined` (i.e. keep it as a hole in
`list`) instead of completely removing it. Before, removing the first
item shifted all the indexes around, which broke keys/ids and the state
handling, so that it appeared as if the wrong element got deleted.

With that, `list.length` becomes meaningless. Adjust the emptiness check
in the render function. Entirely drop `itemCount` -- nothing uses that,
and it's a bit expensive to compute, so let's only introduce this when
there is an actual use case.

https://issues.redhat.com/browse/RHEL-19598
  • Loading branch information
martinpitt committed Jan 9, 2024
1 parent 5bf7980 commit e4dcf6b
Showing 1 changed file with 6 additions and 3 deletions.
9 changes: 6 additions & 3 deletions pkg/lib/cockpit-components-dynamic-list.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ export class DynamicListForm extends React.Component {

this.setState(state => {
const items = [...state.list];
items.splice(idx, 1);
// keep the list structure, otherwise all the indexes shift and the ID/key mapping gets broken
delete items[idx];

return { list: items };
}, () => this.props.onChange(this.state.list));
Expand Down Expand Up @@ -79,9 +80,12 @@ export class DynamicListForm extends React.Component {
/>
} className={"dynamic-form-group " + formclass}>
{
dialogValues.list.length
dialogValues.list.some(item => item !== undefined)
? <>
{dialogValues.list.map((item, idx) => {
if (item === undefined)
return null;

return React.cloneElement(this.props.itemcomponent, {
idx,
item,
Expand All @@ -91,7 +95,6 @@ export class DynamicListForm extends React.Component {
removeitem: this.removeItem,
additem: this.addItem,
options: this.props.options,
itemCount: Object.keys(dialogValues.list).length,
validationFailed: validationFailed && validationFailed[idx],
onValidationChange: value => {
// Dynamic list consists of multiple rows. Therefore validationFailed object is presented as an array where each item represents a row
Expand Down

0 comments on commit e4dcf6b

Please sign in to comment.