-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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.
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
🐞 fix #11842 radio buttons not disabled when multiple share a name #11873
🐞 fix #11842 radio buttons not disabled when multiple share a name #11873
Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
oh nice! thanks @cj-young i will take a look at this weekend! |
Does this works? import { FieldRefs, InternalFieldName, Ref } from '../types';
import { get } from '../utils';
import isObject from '../utils/isObject';
const iterateFieldsByAction = (
fields: FieldRefs,
action: (ref: Ref, name: string) => 1 | undefined | void,
fieldsNames?: Set<InternalFieldName> | InternalFieldName[] | 0,
abortEarly?: boolean,
iterateRefs?: boolean,
) => {
for (const key of fieldsNames || Object.keys(fields)) {
const field = get(fields, key);
if (field) {
const { _f, ...currentField } = field;
if (_f) {
if (iterateRefs && _f.refs && !abortEarly) {
for (const ref of _f.refs) {
action(ref, key)
}
} else if (_f.refs && _f.refs[0] && action(_f.refs[0], key) && !abortEarly) {
break;
} else if (_f.ref && action(_f.ref, _f.name) && !abortEarly) {
break;
} else {
iterateFieldsByAction(currentField, action);
}
} else if (isObject(currentField)) {
iterateFieldsByAction(currentField, action);
}
}
}
};
export default iterateFieldsByAction; |
It seems like when the return value of the action is 1, the function breaks out of the loop and doesn't apply it to any other fields. I think either the naming or usage of When applied to multiple refs, I guess it could either break out either after all the refs have been iterated over or after the first call returns 1. If it immediately returns when the first call returns 1, this could work: import { FieldRefs, InternalFieldName, Ref } from '../types';
import { get } from '../utils';
import isObject from '../utils/isObject';
const iterateFieldsByAction = (
fields: FieldRefs,
action: (ref: Ref, name: string) => 1 | undefined | void,
fieldsNames?: Set<InternalFieldName> | InternalFieldName[] | 0,
abortEarly: boolean = true,
iterateRefs?: boolean,
) => {
for (const key of fieldsNames || Object.keys(fields)) {
const field = get(fields, key);
if (field) {
const { _f, ...currentField } = field;
if (_f) {
if (iterateRefs && _f.refs) {
let shouldAbort = false;
for (const ref of _f.refs) {
shouldAbort = !!action(ref, key) && abortEarly;
if (shouldAbort) break;
}
if (shouldAbort) break;
} else if (_f.refs && _f.refs[0] && action(_f.refs[0], key) && abortEarly) {
break;
} else if (_f.ref && action(_f.ref, _f.name) && abortEarly) {
break;
} else {
iterateFieldsByAction(currentField, action);
}
} else if (isObject(currentField)) {
iterateFieldsByAction(currentField, action);
}
}
}
};
export default iterateFieldsByAction; I could replace the two break statements with a return, but the rest of the if blocks use breaks so I just continued doing that. If it instead aborted after finishing iteration over the current field, the if block could be replaced with: if (iterateRefs && _f.refs) {
let shouldAbort = false;
for (const ref of _f.refs) {
shouldAbort = (!!action(ref, key) && abortEarly) || shouldAbort;
}
if (shouldAbort) break;
} Does this look good? If so, which one is preferred? |
your second approach looks good, I am just trying to save some bytes here and simplify the code. good stuff 👍 |
8a60792
to
6289a66
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution @cj-young appreciated here.
To fix #11842
I extended the functionality of
iterateFieldsByAction
to be able to handle multiple refs, which allows_disableForm
to properly disable all inputs. Previously, whenuseForm
is passed thedisabled
option, it only ever disables the first input of some name if multiple inputs share a name.I don't know if this is the cleanest solution, but I didn't want to disrupt the structure of the current
iterateFieldsByAction
function.