Skip to content

Commit

Permalink
Update tracked value after resetting radio group (#27394)
Browse files Browse the repository at this point in the history
Fixes #26876, I think. Review each commit separately (all assertions
pass in main already, except the last assertInputTrackingIsClean in
"should control radio buttons").

I'm actually a little confused on two things here:
* All the isCheckedDirty assertions are true. But I don't think we set
.checked unconditionally? So how does this happen?
* #26876 (comment)
claims that
d962f35...1f248bd contains
the faulty change, but it doesn't appear to change the restoration logic
that I've touched here. (One difference outside restoration is that
updateProperties did previously set `.checked` when `nextProp !==
lastProp` whereas the new logic in updateInput is to set it when
`node.checked !== !!checked`.)

But it seems to me like we need this call here anyway, and if it fixes
it then it fixes it? I think technically speaking we probably should do
all the updateInput() calls and then all the updateValueIfChanged()
calls—in particular I think if clicking A changed the checked radio
button from B to C then the code as I have it would be incorrect, but
that also seems unlikely so idk whether to care.

cc @zhengjitf @Luk-z who did some investigation on the original issue

DiffTrain build for [3c27178](3c27178)
  • Loading branch information
sophiebits committed Sep 21, 2023
1 parent ab9d541 commit 947dd44
Show file tree
Hide file tree
Showing 15 changed files with 1,604 additions and 1,536 deletions.
2 changes: 1 addition & 1 deletion compiled/facebook-www/REVISION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
2b3d5826836ac59f8446281976762d594e55d97e
3c27178a2f2c74f14d90613028e3929e1f06d830
2 changes: 1 addition & 1 deletion compiled/facebook-www/React-dev.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ if (
}
"use strict";

var ReactVersion = "18.3.0-www-classic-996b7d13";
var ReactVersion = "18.3.0-www-classic-2c91e2cd";

// ATTENTION
// When adding new symbols to this file,
Expand Down
2 changes: 1 addition & 1 deletion compiled/facebook-www/React-prod.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -615,4 +615,4 @@ exports.useSyncExternalStore = function (
exports.useTransition = function () {
return ReactCurrentDispatcher.current.useTransition();
};
exports.version = "18.3.0-www-modern-b27aac94";
exports.version = "18.3.0-www-modern-a19b7375";
2 changes: 1 addition & 1 deletion compiled/facebook-www/React-profiling.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -626,7 +626,7 @@ exports.useSyncExternalStore = function (
exports.useTransition = function () {
return ReactCurrentDispatcher.current.useTransition();
};
exports.version = "18.3.0-www-modern-b5b87391";
exports.version = "18.3.0-www-modern-4c57bafd";

/* global __REACT_DEVTOOLS_GLOBAL_HOOK__ */
if (
Expand Down
18 changes: 13 additions & 5 deletions compiled/facebook-www/ReactDOM-dev.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -4104,10 +4104,7 @@ function restoreControlledInputState(element, props) {
"ReactDOMInput: Mixing React and non-React radio inputs with the " +
"same `name` is not supported."
);
} // 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); // If this is a controlled radio button group, forcing the input that
} // If this is a controlled radio button group, forcing the input that
// was previously checked to update will cause it to be come re-checked
// as appropriate.

Expand All @@ -4121,6 +4118,17 @@ function restoreControlledInputState(element, props) {
otherProps.type,
otherProps.name
);
} // If any updateInput() call set .checked to true, an input in this group
// (often, `rootNode` itself) may have become unchecked

for (var _i = 0; _i < group.length; _i++) {
var _otherNode = group[_i];

if (_otherNode.form !== rootNode.form) {
continue;
}

updateValueIfChanged(_otherNode);
}
}
} // In Chrome, assigning defaultValue to certain input types triggers input validation.
Expand Down Expand Up @@ -34013,7 +34021,7 @@ function createFiberRoot(
return root;
}

var ReactVersion = "18.3.0-www-classic-bb6c815e";
var ReactVersion = "18.3.0-www-classic-bfae45bc";

function createPortal$1(
children,
Expand Down
18 changes: 13 additions & 5 deletions compiled/facebook-www/ReactDOM-dev.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -3946,10 +3946,7 @@ function restoreControlledInputState(element, props) {
"ReactDOMInput: Mixing React and non-React radio inputs with the " +
"same `name` is not supported."
);
} // 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); // If this is a controlled radio button group, forcing the input that
} // If this is a controlled radio button group, forcing the input that
// was previously checked to update will cause it to be come re-checked
// as appropriate.

Expand All @@ -3963,6 +3960,17 @@ function restoreControlledInputState(element, props) {
otherProps.type,
otherProps.name
);
} // If any updateInput() call set .checked to true, an input in this group
// (often, `rootNode` itself) may have become unchecked

for (var _i = 0; _i < group.length; _i++) {
var _otherNode = group[_i];

if (_otherNode.form !== rootNode.form) {
continue;
}

updateValueIfChanged(_otherNode);
}
}
} // In Chrome, assigning defaultValue to certain input types triggers input validation.
Expand Down Expand Up @@ -33858,7 +33866,7 @@ function createFiberRoot(
return root;
}

var ReactVersion = "18.3.0-www-modern-c7a0d8fc";
var ReactVersion = "18.3.0-www-modern-f84a446a";

function createPortal$1(
children,
Expand Down
Loading

0 comments on commit 947dd44

Please sign in to comment.