Skip to content

Commit

Permalink
Fix: update other radios tracked value in one group when one is updat…
Browse files Browse the repository at this point in the history
…ed to be checked
  • Loading branch information
zhengjitf committed Jun 29, 2023
1 parent 2153a29 commit 7e23144
Show file tree
Hide file tree
Showing 2 changed files with 121 additions and 21 deletions.
64 changes: 43 additions & 21 deletions packages/react-dom-bindings/src/client/ReactDOMInput.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,31 @@ import escapeSelectorAttributeValueInsideDoubleQuotes from './escapeSelectorAttr
let didWarnValueDefaultValue = false;
let didWarnCheckedDefaultChecked = false;

function findRadioGroup(node: HTMLInputElement) {
let queryRoot: Element = node;

while (queryRoot.parentNode) {
queryRoot = ((queryRoot.parentNode: any): Element);
}

// If `node.form` was non-null, then we could try `form.elements`,
// but that sometimes behaves strangely in IE8. We could also try using
// `form.getElementsByName`, but that will only return direct children
// and won't include inputs that use the HTML5 `form=` attribute. Since
// the input might not even be in a form. It might not even be in the
// document. Let's just use the local `querySelectorAll` to ensure we don't
// miss anything.
if (__DEV__) {
checkAttributeStringCoercion(node.name, 'name');
}

return queryRoot.querySelectorAll(
'input[name="' +
escapeSelectorAttributeValueInsideDoubleQuotes('' + node.name) +
'"][type="radio"]',
);
}

/**
* Implements an <input> host component that allows setting these optional
* props: `checked`, `value`, `defaultChecked`, and `defaultValue`.
Expand Down Expand Up @@ -175,8 +200,10 @@ export function updateInput(
}
}

let needUpdateRadioGroupTrackedValue = false;
if (checked != null && node.checked !== !!checked) {
node.checked = checked;
needUpdateRadioGroupTrackedValue = type === 'radio' && !!checked;
}

if (
Expand All @@ -189,6 +216,21 @@ export function updateInput(
checkAttributeStringCoercion(name, 'name');
}
node.name = toString(getToStringValue(name));

if (needUpdateRadioGroupTrackedValue) {
const group = findRadioGroup(node);

for (let i = 0; i < group.length; i++) {
const otherNode = ((group[i]: any): HTMLInputElement);
if (otherNode === node || otherNode.form !== node.form) {
continue;
}

// We need update the tracked value on the named cousin since the value
// was changed but the input saw no event or value set
updateValueIfChanged(otherNode);
}
}
} else {
node.removeAttribute('name');
}
Expand Down Expand Up @@ -348,27 +390,7 @@ export function restoreControlledInputState(element: Element, props: Object) {
);
const name = props.name;
if (props.type === 'radio' && name != null) {
let queryRoot: Element = rootNode;

while (queryRoot.parentNode) {
queryRoot = ((queryRoot.parentNode: any): Element);
}

// If `rootNode.form` was non-null, then we could try `form.elements`,
// but that sometimes behaves strangely in IE8. We could also try using
// `form.getElementsByName`, but that will only return direct children
// and won't include inputs that use the HTML5 `form=` attribute. Since
// the input might not even be in a form. It might not even be in the
// document. Let's just use the local `querySelectorAll` to ensure we don't
// miss anything.
if (__DEV__) {
checkAttributeStringCoercion(name, 'name');
}
const group = queryRoot.querySelectorAll(
'input[name="' +
escapeSelectorAttributeValueInsideDoubleQuotes('' + name) +
'"][type="radio"]',
);
const group = findRadioGroup(rootNode);

for (let i = 0; i < group.length; i++) {
const otherNode = ((group[i]: any): HTMLInputElement);
Expand Down
78 changes: 78 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMInput-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2358,4 +2358,82 @@ describe('ReactDOMInput', () => {
expect(log).toEqual(['']);
expect(node.value).toBe('a');
});

it('#26876: After the `checked` props updated not in the event handler task, `onChange` should still be called when the unchecked radio clicked', async () => {
const eventHandler = jest.fn();
ReactDOM.render(
<div>
<input
type="radio"
name="a"
value="1"
checked={true}
onChange={eventHandler}
/>
<input
type="radio"
name="a"
value="2"
checked={false}
onChange={eventHandler}
/>
</div>,
container,
);

ReactDOM.render(
<div>
<input
type="radio"
name="a"
value="1"
checked={false}
onChange={eventHandler}
/>
<input
type="radio"
name="a"
value="2"
checked={true}
onChange={eventHandler}
/>
</div>,
container,
);

ReactDOM.render(
<div>
<input
type="radio"
name="a"
value="1"
checked={true}
onChange={eventHandler}
/>
<input
type="radio"
name="a"
value="2"
checked={false}
onChange={eventHandler}
/>
</div>,
container,
);

const radio1 = container.querySelector('input[name="a"][value="1"]');
const radio2 = container.querySelector('input[name="a"][value="2"]');

expect(eventHandler).not.toHaveBeenCalled();

// This next line isn't necessary in a proper browser environment, but
// jsdom doesn't uncheck the others in a group (because they are not yet
// sharing a parent), which makes this whole test a little less effective.
setUntrackedChecked.call(radio1, false);
setUntrackedChecked.call(radio2, true);

dispatchEventOnNode(radio2, 'click');

expect(eventHandler).toHaveBeenCalled();
});
});

0 comments on commit 7e23144

Please sign in to comment.