diff --git a/src/components/renderer/LookupButton.tsx b/src/components/renderer/LookupButton.tsx index 61554a7d..e02a5776 100644 --- a/src/components/renderer/LookupButton.tsx +++ b/src/components/renderer/LookupButton.tsx @@ -7,7 +7,6 @@ import MaterialIcon from '../MaterialIcon' type Props = { value: unknown | undefined - readOnly: boolean validationMessage: string | undefined hasMarginTop?: boolean isInputButton?: boolean @@ -17,15 +16,20 @@ type Props = { function LookupButton({ value, - readOnly, validationMessage, hasMarginTop, isInputButton, lookupButtonConfig, overrideRequiredMessage, }: Props) { - const { isLookup, onLookup, isDisabled, isLoading, allowLookupOnEmptyValue } = - useLookupNotification(value) + const { + isLookup, + onLookup, + isLookupRequestInFlight, + isLoading, + allowLookupOnEmptyValue, + areLookupsDisallowed, + } = useLookupNotification(value) if (!isLookup) { return null } @@ -46,10 +50,11 @@ function LookupButton({ )} onClick={() => onLookup()} disabled={ - // Element-level readOnly only disables the button. Auto-lookups still - // run from LookupNotification when the field is locked but has a value. - readOnly || - isDisabled || + // Deliberately not the element's own `readOnly` flag: a locked field + // populated by a previous lookup still needs its button to run the + // next lookup in the chain. + areLookupsDisallowed || + isLookupRequestInFlight || isLoading || (isEmptyValue && !allowLookupOnEmptyValue) || (!isEmptyValue && diff --git a/src/components/renderer/LookupNotification.tsx b/src/components/renderer/LookupNotification.tsx index a498b895..29602b6a 100644 --- a/src/components/renderer/LookupNotification.tsx +++ b/src/components/renderer/LookupNotification.tsx @@ -59,7 +59,8 @@ function LookupNotificationComponent({ const [hasLookupFailed, setHasLookupFailed] = React.useState(false) const [hasLookupSucceeded, setHasLookupSucceeded] = React.useState(false) const [isCancellable, setIsCancellable] = React.useState(false) - const [isDisabled, setIsDisabled] = React.useState(false) + const [isLookupRequestInFlight, setIsLookupRequestInFlight] = + React.useState(false) const [lookupErrorHTML, setLookupErrorHTML] = React.useState( null, ) @@ -174,6 +175,15 @@ function LookupNotificationComponent({ [element, injectPagesAfter, onLookup], ) + // Definition-level `readOnly` must not suppress lookups for a submitter: + // prefills and defaults still need to populate related fields, and a locked + // field can still be the input to a chained lookup. Approver locks are + // different. Re-running a lookup from an existing submitted value could + // overwrite persisted answers merely by opening the review, so only approver + // editable elements may run one. + const areLookupsDisallowed = + formIsReadOnly || isElementReadOnlyForAudience(element, audience) + const isNotStaticLookup = React.useMemo(() => { return ( (formElementDataLookup && formElementDataLookup.type !== 'STATIC_DATA') || @@ -186,15 +196,7 @@ function LookupNotificationComponent({ LookupNotificationContextValue['onLookup'] >( async ({ newValue, abortController, continueLookupOnAbort }) => { - // Definition-level `readOnly` must not suppress auto-lookups for a - // submitter: prefills and defaults still need to populate related - // fields. Approver locks are different. Re-running a lookup from an - // existing submitted value could overwrite persisted answers merely by - // opening the review, so only approver-editable elements may run one. - if ( - formIsReadOnly || - isElementReadOnlyForAudience(element, audience) - ) { + if (areLookupsDisallowed) { return } @@ -205,7 +207,7 @@ function LookupNotificationComponent({ return } - setIsDisabled(true) + setIsLookupRequestInFlight(true) setIsCancellable(false) setHasLookupFailed(false) setHasLookupSucceeded(false) @@ -290,19 +292,18 @@ function LookupNotificationComponent({ } finally { clearTimeout(isCancellableTimeout) if (isMounted.current) { - setIsDisabled(false) + setIsLookupRequestInFlight(false) setOnCancelLookup(undefined) } } }, [ - audience, definition, element, excludeDefinition, formElementDataLookup, formElementElementLookup, - formIsReadOnly, + areLookupsDisallowed, isMounted, isNotStaticLookup, isOffline, @@ -367,13 +368,21 @@ function LookupNotificationComponent({ const contextValue = React.useMemo( () => ({ isLookup: true, - isDisabled, + isLookupRequestInFlight, isLoading, onLookup: triggerLookup, allowLookupOnEmptyValue: runLookupOnClear, isLookingUp, + areLookupsDisallowed, }), - [isDisabled, isLoading, runLookupOnClear, triggerLookup, isLookingUp], + [ + isLookupRequestInFlight, + isLoading, + runLookupOnClear, + triggerLookup, + isLookingUp, + areLookupsDisallowed, + ], ) return ( @@ -444,7 +453,7 @@ function LookupNotificationComponent({ 'fade-in button is-primary ob-lookup__retry-button cypress-retry-lookup-button has-margin-top-8', )} onClick={() => retryLookup()} - disabled={isDisabled || isLoading} + disabled={isLookupRequestInFlight || isLoading} aria-label="lookup-retry-button" > diff --git a/src/form-elements/FormElementABN.tsx b/src/form-elements/FormElementABN.tsx index cd2bba12..39d74d19 100644 --- a/src/form-elements/FormElementABN.tsx +++ b/src/form-elements/FormElementABN.tsx @@ -268,7 +268,6 @@ function FormElementABN({ )} )} )} )} )} )} )} )} )} )} undefined, isLookingUp: false, }